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>

The discreteness requirement was only discoverable at Stage B, as a refusal, after three invocations had been paid for. It is knowable at Step 6: how many rows the two arms must DISAGREE on is three to five, essentially regardless of suite size, and no row count fixes a suite whose candidate changes two verdicts. The table states its Holm threshold as `alpha/S` rather than a retyped 0.05, which is both what keeps the constant declared once and what puts the table inside CE039's arithmetic-table predicate — so the new `sizing-table` claim recomputes every cell from `min_discordant_rows`, and the coverage rule is satisfied rather than side-stepped. The no-number sensor now covers DEFAULT_ALPHA too, which immediately found an illustrative `0.05` in the method file that reads as alpha in a document using both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Both found by an independent review of 2/6 and verified by computing them. 1. When `min_discordant_rows` returns None the refusal said the suite needs "more rows AND more disagreement". It returns None only when the floor at R == M — which collapses to the estimator's own 2/(m+1), a function of the DRAW COUNT and of nothing about the suite — is still above the bar. So rows cannot help at any size; the levers are n_resamples and family size, and the message now says that. It was the one refusal branch with no test. 2. `ActivationGateVerdict.n_discordant` claimed a RATE binds rather than a count, with a figure to match. Measured: the required count is 3 at 8 rows and 4 at 10, 20 and 100 — flat — while the rate falls from 0.4 to 0.04. The description asserted the opposite of what the skill's own sizing table now pins, so two shipped surfaces disagreed about the same quantity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Every promotion appended rows to `regression_corpus`, and nothing read them except the writer's own de-duplication. `regression_check` is the read: given the corpus and an arm's row vector it returns the rows that arm did not fully score, in corpus order, with each row's stated reason still attached. A hole comes back as None rather than being skipped — the same rule `_dominates` applies to the row vector, since not measuring a row is not passing it. The two causes (the row errored, or it belongs to the skill's other suite) are indistinguishable from the corpus alone, and the docstring says so rather than letting the caller guess. Step 10 reads it before shortlisting; Step 11 now points forward at it, so the write and the read are named together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…verdict The execution gate was a procedure a human performed: read the reporter's paired block, work out which direction its subtraction ran, call the guardrails separately, and eyeball completion_rate and engagement recall. The method file warns twice that a reversed reading promotes the arm that lost. `execution_gate` reuses `reports_stats.paired_comparison` — the statistic this repo already owns — and resolves the sign itself, so `mean_diff` is always candidate minus incumbent and the interval bounds stay ordered whichever way the experiment declared its variants. The two integrity readings come from the rows the gate already loaded, NOT from `suite.json`: `criterion_aggregates` is a filtered list, so a positional read there silently reports a different criterion the moment an earlier one produces no aggregate. `holm_promote_execution` is the second and only other `holm_rejections` call site; on this track the family lives across run dirs, one two-variant gate per candidate. A test counts call NODES so a third one inside the module fails it — a file-set assertion was already true and proved nothing. Guardrails and integrity checks stay out of `promoted` and gate in the render, which is what keeps BLOCKED BY A GUARDRAIL reachable rather than dead code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`sibling_indices` now defaults to None, which DERIVES every other classification criterion from the run itself; `()` is how you turn the check off deliberately, so the two states differ. The shipped snippet passed `()` and told the reader to leave it that way, which meant a suite stacking sibling criteria was silently ungated against a candidate winning by annexing them — a guardrail nobody remembers to arm is a guardrail the tool does not have. Derivation takes both arms' row maps as varargs: merging them would drop the incumbent's results for every shared row id, which is every row in the common case. Each sibling check also reports a `GuardrailCheck.rate` — the annexation rate, the fraction of that sibling's true-yes rows this candidate alone turned to no. A reading beside the recall, never a second gate. Also carries the review fixes for 5/6, which land in the same files: - notes appended AFTER the verdict was constructed were silently discarded (pydantic copies the list), so the below-MDE warning and the zero-variance effect-size explanation never reached a reader. The MDE is now measured once, up front, and `_verdict` is the last thing every return path does. - a mistyped variant/suite/run-dir rendered as PROMOTED with four green `— -> —` checks, because the statistic comes from experiment.json and the checks come from the row tree. It now carries the same ZERO-rows note activation_gate has. - the same variant id on both arms produced a significant, sign-flipped verdict comparing an arm to itself. Guarded. - completion_rate was computed over the paired intersection, so it could not see a row that vanished from one arm — the exact erosion it exists to catch. It is now over the union, with a shared per-row denominator. - guardrails and integrity checks ran over the on-disk intersection rather than the rows the statistic actually paired, contradicting cost_latency_guardrails' own documented contract. - the verdict records `n_resamples`; the confinement sensor now attributes a call to the INNERMOST enclosing function, so a third call hidden in a closure no longer inherits an allowed name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Neither is a 30-minute guard: one needs alias analysis to avoid false positives, the other needs a definition of "a formula" precise enough to gate on. Recorded rather than half-built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Cross-phase findings — both High ones are integration bugs no per-phase review could have seen, and one of them I introduced while fixing an earlier finding. - `cost_latency_guardrails` indexed `rows[rid]` directly. That was safe while its only caller passed the intersection of the two maps, but `execution_gate` now passes the rows `paired_comparison` paired, which come from `experiment.json` — so a row named there and missing on disk (a skipped malformed task.json, or an unfanned single-task suite) raised a KeyError out of the skill's inline snippet, discarding the wrong-path note composed just above it and contradicting the function's own "never an exception" contract. - The sibling check read UNBALANCED replicate pools while the primary criterion balanced its own. Two arms with byte-identical labels on every row and every replicate read recall 0.5 against 0.6 from one row's extra replicate — and unlike the guardrails, the sibling check is folded into `promoted`. Dormant until 6/6 armed it by default, which is what makes it worth fixing now. - The annexation rate zipped the two arms over one FLATTENED list, so an unbalanced row shifted every later row's alignment: a candidate that annexed half the sibling's true rows rendered 0.000, "took nothing". Pairing is now per row, which the balancing above makes exact rather than best-effort. - `ActivationGateVerdict.promoted` now says it folds the sibling checks in, and `render_markdown`'s docstring no longer claims it is the primary statistic alone — that sentence was true of the execution gate and false of the one it documents. The engagement check's `tolerance` is an absolute FLOOR, not a permitted drop, and both the value and the field description now say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…view Both are semantic claims about intent rather than patterns, so neither is a cheap AST rule. The second is guarded here by a per-metric invariance test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ters Rounds 2+ may run a single lineage before Stage A: one candidate, one train split, accepted if it beats the score the lineage head recorded. It promotes nothing — the comparison is across invocations, unpaired and uncorrected — so the loop needed a pointer that cannot be misread as a promotion. `RoundScores.lineage_head` is that pointer. It stores no score: the number to beat is derived from the named arm's `row_scores`, and a validator rejects the two states that would otherwise be silent at write time and fatal at read time (a head naming an absent arm, and a head with no rows). Two things the surfaces now say that the first draft got wrong: - the activation search compares **accuracy**, not `f1.yes`. A mean over a binary criterion's per-row scores credits true negatives; F1 does not, so a merely-more-conservative arm can win the search and lose the gate. - two invocations' vectors are not automatically over the same rows, and every way they diverge favours the candidate — so the comparison is taken over the shared rows and a hole is refused rather than averaged around. Step 13's stop rule now counts *gated* rounds. A search round promotes nothing by construction, so the un-amended rule would have ended every session after two of the cheapest rounds available. CE039 gains invariant 5 with a committed self-test: a search round must be strictly cheaper than the smallest possible Stage A. The plausible wrong edit is to co-run the incumbent "for a fair comparison", which doubles the phase and deletes its reason to exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A candidate that reproduces a train row's graded content verbatim scores well on that row whether or not the behaviour under test happened, and an A/B arm that DELETED the behaviour would pass it too. That is CE036's rule pointed at a skill body instead of a prompt, so the primitive moves to `src/coder_eval/leak_detection.py` and both consumers share it — a second copy would agree on ordinary input and diverge exactly where either was written for. `optimize_gate.candidate_leaks` is the reader. Three things it is deliberately not, each measured rather than reasoned: - **not an absolute scan.** Measured on this repo's own ci-outcome suite, an absolute scan flags the SHIPPED `ci` skill on five train-split strings that are simply the output contract its suite grades. A checker that fires on the shipped skill on its first run is one users learn to ignore. - **not keyed on the incumbent.** The baseline is what the candidate was DERIVED from — the lineage head on a search round — or every span the head added is re-reported every round, which is the same wolf-crying one level up. - **not a proof.** The docstring states all three boundaries: verbatim-only, substring-not-token containment, and that a span already in the baseline is invisible from then on. That last one is why proposal-prompt.md puts the rule on the proposer as well; no diff can catch it later. `graded_strings(drop_type=)` is the one behavioural difference between the two consumers. CE036 keeps the discriminator — a row PROMPT saying "skill_triggered" is worth flagging — and `candidate_leaks` drops it, because measured without that, `optimize-skill`'s own body flags on `skill_triggered` and nothing else. All 12 existing CE036 tests pass unchanged; that is the extraction's regression check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two of the four activation categories were already in Step 7 as prose examples. This promotes them in place rather than adding a parallel list — a second taxonomy beside the first would leave two overlapping sets and two sensors guarding near-identical prose. The four are named for the edit each implies, as the execution list already was, and the fourth carries the distinction the sibling guardrail is built on: annexation is a false positive in THIS skill's matrix and a false negative in the sibling's, so it surfaces as the sibling's `recall.yes` dropping — the number Stage B actually gates. Two matrices, one row, opposite signs. The gold-solution rule answers a question expected-vs-observed cannot: not that the row failed, but how it was MEANT to be solved. Its hazard is sharper than ordinary memorization because the content is known-correct and sitting right there, so the rule is stated with "extract the procedure, never the answer" attached. Scoped: most activation suites carry no reference and it is a no-op. Two field-name claims were wrong on the first write and are corrected here against the code rather than against memory — `llm_judge` has no `rubric` field and `reference_comparison` carries no reference file; the one real source is `TaskDefinition.reference`. Its own description then supplied a distinction worth keeping: that block is hidden from the AGENT by design, which is why handing it to the proposer is legitimate and why doing so is not a relaxation of the test-split blinding. The token sensors move to `_PROPOSAL_TOKENS` / `_SKILL_PROCEDURE_TOKENS` and a committed self-test runs the REAL matcher over mutated text for every entry in both. Adding self-tested tokens to an un-self-tested inline loop would have reproduced the failure class the helper exists to prevent, so the whole loop moved — ~40 pre-existing deletion sensors are now proven able to fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Cross-phase findings, from two Opus reviewers and a fact-checker that RAN every claim the new prose makes about the code. Integration between the phases — the defects only the whole diff shows: - `proposal-prompt.md` never received Phase 1's two-pointer edit. It still told the proposer to build from "the incumbent", which on a search round discards everything the lineage accumulated and leaves the leak check — which diffs against what the candidate was DERIVED from — nothing coherent to compare. - the search loop advanced the lineage head without consulting the regression corpus, though `regression_check` takes exactly the `ArmRowScores` the snippet had already computed. A re-lost row could ride forward until the next multi-arm round. Verified by executing the shipped snippet: the candidate wins the aggregate 0.75 to 0.50 while re-losing a corpus row, and now says so. - Phase 3 introduced a leak source Phase 2's checker cannot see. `reference:` is a TASK-level field, so `candidate_leaks` never scans it — while the prose called copying it "especially tempting". Now a stated fourth boundary on the function and a plain warning in the proposal shape. Three false claims, each caught by computing it rather than reading it: - "`failed_samples[]` is capped, so it will not hand you fifteen" — the cap is 20 (`reports.py:32`), which is MORE than fifteen. The real hazard is the opposite of the one stated: the list is a PREFIX in row order, so it costs spread, not count, which is what the paragraph above it asks for. - the validator's docstring promised a `ZeroDivisionError` that the snippet's guards preempt; the real symptom is a no-shared-rows exit blaming an unpinned `dataset.sample_seed` — the wrong problem entirely. - "flagged `optimize-skill`'s own body and nothing else" — `skill_triggered` appears verbatim in FOUR shipped bodies. Also: guard order (no overlap is a WIRING fault and must not be reported as a hole), "best-scoring arm" now names its metric (the mean of `row_scores`, not the f1.yes ranking Stage A uses — two rankings that diverge), and the gate-API sensor now covers every `coder_eval` module a snippet imports rather than `optimize_gate` alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Both are real and neither is a ~30-minute guard: one needs a way to know which constant a bare number in prose shadows, the other changes a shipped lint rule's exemption list and its derived CLAUDE.md sentence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Deferred rather than skipped: every snippet in this skill is hand-written the same way, so fixing one alone leaves the inconsistency it is meant to remove. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The accept/revert logic lived in a markdown block an agent copies and adapts. Four guards were embedded in it, and every one is silent when dropped: - compare over the rows BOTH arms scored (the two vectors come from different invocations, and every way they diverge favours the candidate); - report no-overlap BEFORE holes, because that one is a wiring fault and calling it a hole sends the reader hunting a flaky row; - refuse on a hole rather than averaging around it — a candidate that errored on the hardest rows scores higher over the survivors; - block an otherwise-winning candidate that re-loses a regression-corpus row, since a search accept advances the lineage and an aggregate cannot show it. `search_compare` asserts them instead. `beats` and `accepted` are two fields because a corpus block and a plain loss call for opposite next actions, and a refused comparison reports None for both scores rather than a number nobody should read. A tie does not win: advancing the head on a tie moves the bar every later round is judged against, on an accident. `lineage_head_scores` resolves the head by highest `round` rather than list position — the sidecar replaces per round, so order is a write-order artefact — and skips a later round that accepted nothing, so a quiet round leaves the head where it was. Not a gate, and the rendered block says so: unpaired, unreplicated, uncorrected, so an accept advances the lineage head and never the incumbent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Checked every doc and tutorial against the current code. Most were fine — no surface referenced the constants that moved to `leak_detection`, none lists the src tree, the "three stages" framing is still correct, and the docs indexes regenerate byte-identically. Four things were stale, one of them materially. **Tutorial 08's Reference section stated a RETIRED promotion rule as current.** Its Stage B row said "promote only on `min(candidate) > max(incumbent)`" — range non-overlap, which threw away the pairing and had very little power at 8-12 rows per polarity. That has been a reported diagnostic and not the gate for a while; the rule is the paired cluster-bootstrap interval excluding zero AND the Holm-corrected test rejecting. The page's NARRATIVE sections already carried "the gate has since been replaced" caveats — it was only the section framed as a reference summary that did not, which is the one a reader quotes. Its "what Stage B proves" paragraph understated the same thing: resampling rows bounds row-sampling variation as well as run noise, and Holm bounds multiplicity. Neither corrects for the survivors having been chosen on those rows at Stage A, which is still exactly why Stage C is not optional. - `plugins/coder-eval/README.md` omitted `proposal-prompt.md` from its reference list entirely — a shipped file no reader could discover from that page. - both reference lists now mention the search loop (and say why it is not a fourth stage) and the gold-solution input. - tutorial 09 pointed at the reporter's raw paired block for the verdict; it now also names `execution_gate`, which resolves the sign the paragraph warns about. And its "four arms tying exactly is a bug report" take-away now names the control arm — the stage added because of that round, which would have caught it on the first comparison rather than the fourth. No new headings, so no anchors moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`holm_promote_execution` promoted at p = 0.0000 with a zero-width interval whenever the per-row differences had zero variance, at any row count. Per-row `weighted_score` is a weighted mean over a handful of discrete criterion scores, so identical per-row differences are ordinary: `paired_t_test` then returns 0.0 for a constant non-zero shift and `paired_t_ci` collapses to a point, and `rejected_at`, `favours_candidate` and `excludes_zero` all hold at once on a sample that separated nothing. The shipped `outcome-rows.jsonl` train split hits exactly this shape. `execution_gate` now sets `ExecutionGateVerdict.gate_refusal` where the condition is ALREADY computed — `mean_diff is not None and effect_size is None` is exactly `stddev(diffs) == 0`, since `cohens_d` is the paired d. The note that described the same finding is replaced, not printed beside it. `holm_promote_execution` only READS the field: it forces `promoted=False` and suppresses the three negative-result notes, which would otherwise contradict the headline. `render_execution_markdown` gains `NOT A RESULT` between UNDECIDED and BLOCKED BY A GUARDRAIL — the module's own existing phrase for this concept, and deliberately not the activation track's CANNOT SEPARATE AT THIS SIZE, which reports a discreteness floor the paired t does not have. Two messages, split at `mean_diff == 0.0` the way `holm_promote` splits its own refusal at `p_floor >= 1.0`: a constant difference of exactly zero reports p = 1.0, not 0.0, and it is a finding about the CANDIDATE (it behaved identically to the incumbent — a wrong `plugins:` path gives this shape) rather than about the suite, so "add rows the arms disagree on" is the wrong remedy there. The four surfaces claiming this track has no refusal state are corrected, each keeping its p_floor half. The zero-row cause those surfaces also name is set in 2/4; the two commits land together and nothing ships between them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… arm Two wiring faults reached PROMOTED. The incumbent-variant mismatch annotated and FELL THROUGH while its candidate-side sibling three lines above returned. The sign had already been resolved from the candidate, so the block reported a real, significant difference against whichever arm `experiment.json` actually carried, under a header naming the arm the caller asked for. It now returns a statistics-free verdict, byte-symmetric with that sibling. The zero-row case never reaches that line at all: `load_arm_rows` returns `{}` for a mistyped variant/suite/run dir while the experiment file stays perfectly valid, so the statistic computes and every check reads green over rows that are not there. It now sets the SAME `gate_refusal` field 1/4 added — one refusal naming every empty arm, replacing a loop that appended the same finding twice when both arms were empty. Wiring outranks variance by an explicit `if gate_refusal is None:` guard, because if the rows never loaded, whether their differences vary is moot. `except ValueError` on the experiment read becomes `except (ValueError, OSError)`: the docstring promises "Never an exception", and a permission error or a file that vanished between the `is_file()` and the read is as much a wiring fault as a malformed one, with the same right answer. No new field, no new headline, no new prose token — Step 10's block from 1/4 already names both causes and both remedies. The fail-closed branch is pinned by a test in which it is the ONLY thing acting: a mistyped id also empties the arm, so a test using one could not tell the two halves apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…shing Both verdict models were built by splatting a string-keyed dict, and neither they nor `GuardrailCheck` declared `extra="forbid"`. So `mean_dif=0.42` landed in no field at all: pydantic ignores extras by default, the model was constructed with `mean_diff` at its default, every other number stayed intact, and nothing raised or logged — on models whose entire job is to say what a promotion decision rests on. The fix is two-sided, and both halves are here. `extra="forbid"` catches it at runtime; literal-keyword construction catches it statically. `activation_gate`'s `verdict_kwargs` becomes four named locals — no new closure, since Plan C owns that restructuring — which also retires the `p_floor`/`n_discordant` overwrite the two splats forced and the `isinstance(mde, float)` recovery that existed only to get a type back out of a `dict[str, object]`. `execution_gate` keeps its `_verdict` closure but takes explicit keyword-only parameters. CE041 ships with the fix rather than waiting for Plan D, because the rule is the sensor for the defect this phase fixes and the invariant is otherwise prose. It matches ANY `**` splat into a constructor whose name came from `coder_eval.models` — narrowing the operand to a name or a dict display would miss `**other.model_dump()`, which is how this arrives in the wild — and its docstring states what it does not reach: aliases, factories, and `model_copy(update=)`, which is a different hole now filed as a harness candidate. That one matters: `promoted` and `holm_alpha` are written only that way, so the two fields that ARE the promotion decision are the two the runtime backstop still does not cover. The rewrite is behaviour-preserving by construction, which is the kind of claim that needs a witness. Four verdict dumps are pinned as committed fixtures beside `report_snapshots` and `golden_streams`, captured from the pre-rewrite code in a worktree at 2/4. The fixtures are shaped so they can SEE a dropped keyword: a sibling criterion so `sibling_checks` is non-empty, an early-return arm whose incumbent still yields a real `mde`, and a refused execution verdict so `gate_refusal` is not None. Verified by deleting those three keywords — three of the four pins go red; with a single-criterion suite and no refused pin, none did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…s had Five parity gaps, each with a sibling in the same file to copy from. `_dominates` and `pareto_front` had no non-finite guard while both other fronts did. Every `>=` against NaN is False, so a NaN cell makes its arm undominatable AND unable to dominate anyone: it takes the front by incomparability and renders in bold beside arms that earned it. `_finite_scores` treats such a cell as a hole, which routes it through the coverage rule already agreed for a hole, and an arm whose every cell is non-finite then drops out by the same rule that excludes an arm scoring no rows. Nothing produces one today — `_row_score` returns means bounded [0, 1] — which is exactly why it would be silent. The claim that all three fronts guard this is now made in `cost_quality_front`'s docstring and in CLAUDE.md, and a parametrized test over nan/inf/-inf asserts the three agree rather than leaving it to be believed. `measure_noise_floor` lacked the wrong-path guard its execution twin has, so a mistyped variant, suite or run dir was reported as "only 0 row(s) scored a classification result at criterion N" — sending the reader to check the criterion index. The predicate is `if not any(per_dir):` rather than the twin's `if not rows:` because this one splits per-invocation maps instead of pooling once; the twin's "no run dirs were given" fallback is omitted because the invocation-count guard above already returned. `NEAR_FLOOR_MULTIPLE` names the bare `5.0` that silently decides whether the resolution-floor warning fires. `load_suite_rows`' warning said "for the activation gate" while both gates load through it. And `render_search_comparison` formatted two `float | None` fields with a bare `:.3f` in four places, which raises; they route through `_fmt`, the module's one declaration of how it prints an optional float. That path is unreachable through `search_compare` today — stated in the docstring rather than left as a dead branch — but the function is public and the alternative is a TypeError out of the skill's inline snippet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Cross-phase defects the per-phase reviews could not see, all in how the refusal this change introduced interacts with everything around it. **The refusal was routed through a headline-only channel.** `UNDECIDED` outranks it — correctly, since a verdict Holm never saw has no decision to refuse — but the messages the refusal SUBSUMED used to live in `notes`, which every render path prints. So a pre-Holm block over a mis-wired arm rendered a confident interval and four green checks with nothing anywhere saying the rows are not there: exactly the silent zero this module promises never happens, reintroduced by the fix for it. The refusal text now prints on its own line whenever the headline could not carry it — which is precisely `promoted is None`, so it still appears exactly once. **A refused verdict stayed in the Holm family.** The zero-row cause is set before `experiment.json` is opened, so a refused verdict routinely carries a real p — and Holm's step-down breaks at the first failure, so a refused sibling could flip a genuine candidate to NOT PROMOTED. It is now excluded from the vector for the same reason a p-less verdict already was: it cannot be allowed to tighten the correction for the others. **The `p_value is None` note was unguarded**, and the comment saying that rung was "unreachable with a refusal today" was false — this repo's own mistyped-incumbent test produces refused-and-p-less, rendering "the sample could not support a p-value" directly under a NOT A RESULT headline. Guarded, and the comment corrected. Two findings resolved as claims rather than code. A non-finite score would make `paired_t_ci` decline and produce an all-`None` statistic over a real row count with no note — but it cannot arrive here, because pydantic's JSON validator rejects `NaN`, so the file never parses and the read's own note is what the reader gets. Recorded as a comment plus the test that will fail if that stops being true, rather than as an unreachable guard claiming otherwise. And CE041's boundary list gains the two import shapes it does not track. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ils loudly `_exec_run_dir` writes a FIXED `<tmp>/round1-gate` and never clears it, so two calls under one `tmp_path` merge: the second arm's rows land beside the first's while `experiment.json` is overwritten. Silent, and it drifts a fixture without failing anything — measured during this plan's Phase 1 review, where a test comparing a refused fixture against a clean control ran the control over the refused arm's leftover rows and moved its `mde` from 2.8e-17 to 0.030, assertion still green. The guard is one assertion at the top of the helper, which turns the merge into a test error naming the fix. Two further candidates from this plan's reviews are deferred to .claude/harness-candidates.md: the vacuous `"X" not in text.replace("NOT X", "")` absence assertion, and the annotate-and-fall-through validation branch. Both need a repo-wide sweep before a rule could land green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

**1. Near-zero variance promoted.** The zero-variance refusal caught only the exact-zero point of a continuum: 4 rows differing by 0.400, 0.400, 0.400, 0.401 promote at p = 5.4e-10. Two different things go wrong there and they need opposite responses, so they are two checks and only one refuses. The effect being below the suite's own MDE — the half-width of a null comparison over the incumbent's own replicates, where the true difference is zero by construction — is a claim about an effect the instrument cannot see. That was already DETECTED and reported as a note the reader could promote past; it is a refusal now, conditioned on the interval EXCLUDING zero. That conjunct is load-bearing: under a true null nearly every candidate is below the floor, so refusing on the floor alone retires NOT PROMOTED almost entirely and sends the reader to buy replicates for a candidate whose problem is that it does not work. Measured over 40 true-null candidates: 37 NOT PROMOTED, 2 refused, 1 promoted; large effects promote 40/40. The interval being tighter than the floor is a CAVEAT, not a refusal — the t reads between-row spread while the MDE measures within-row noise, so a genuine 8-row 0.30 win has the same shape, and refusing it would discard real wins. And where there is no floor at all, the block says so. A null split returns exactly 0.000 whenever every row's replicates agree, which two replicates on a deterministic suite produce routinely; rendered alone, "minimum detectable effect: 0.000" reads as "this suite can resolve anything". `FLOOR_RESOLUTION` exists because that floor is often 2.8e-17 rather than 0.0, which is not zero — so an `== 0.0` test missed it while `abs(diff) < mde` could never fire, and both floor checks went silently inert on exactly the suites they exist for. **2. Five validation branches rendered NOT PROMOTED for wiring faults** — same variant on both arms, a missing/unreadable/malformed experiment file, an experiment with other than two variants, either variant id absent from it, and fewer than two rows paired. Each now refuses, through a `_refuse` closure that keeps the FIRST cause so precedence is stated once rather than in eleven guards. The zero-row refusal moved BELOW them: a mistyped variant id also empties that arm, so refusing on the consequence replaced a message naming the two ids the experiment carries with a vaguer one. All 16 fault combinations were enumerated and each now yields its most actionable message. **3.** `reference/optimize-method.md` gained the execution track's refusals, and two claims that were false there are corrected. Also reverts this branch's Holm-family exclusion, which was wrong: membership is `p_value is not None` and nothing else. Holm corrects for the hypotheses actually tested, and a candidate that was gated and measured was tested however degenerate its sample turned out to be — dropping it shrinks m and LOOSENS alpha/m for its siblings. Measured: two below-MDE refusals promoted a p = 0.027 sibling that a family of three rejects. A cause meaning there was no comparison at all already has no p, so it leaves by the ordinary rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Plan A pinned both verdicts' `model_dump`; nothing pinned the STRING the skill actually prints. `TestRenderMarkdown` and its siblings assert substrings, so a reordered or dropped line stays green — and the next five phases split `render_row_matrix` into section helpers and move every renderer to a new module, which is exactly the change that reorders a line without moving a number. Six whole-block pins, compared with `==`: the activation block (with a sibling regression, so the sibling-check rendering is covered), the discreteness refusal (the one refused verdict carrying no filesystem path), the execution block, the row matrix, the cost/quality table and the search comparison. The row-matrix fixture is tuned so every section renders something: the two fronts disagree in BOTH directions, one row is all-zero, one row is a hole and one arm scored nothing. The cost/quality fixture carries a thin arm and an arm missing a coordinate. Fixtures were generated by running the six test bodies with the comparison helper swapped for a writer, so a fixture cannot differ from what the test renders. Determinism re-checked across independent tmp_path trees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`path_utils.replicate_subdir_name` owns the two-digit replicate directory name and its docstring already says callers MUST use it. Four sites hand-rolled the padding anyway: two f-strings in reports_junit, the optimize gate's suite glob and reports_stats' task glob. None of them RAISES the day that padding widens — the globs just match nothing, both optimize gates load ZERO rows, and the zero-row note blames a wrong variant id, a wrong suite id or a wrong run directory, which are the three things that would be correct. `TASK_JSON_GLOB` is now the gate's one declaration of its glob and `_task_json_pattern` derives all four wrong-path messages from it, so changing the glob can no longer leave three of them describing a tree the code does not search. The two junit sites call `replicate_subdir_name`; reports_stats globs padding-agnostically. The glob change is behaviour-identical, measured rather than argued: both patterns were run over every suite and task directory in this repo's real runs/ tree — 207 suite-level and 2,454 task-level matches, identical sets, zero differing directories. CE042 is the sensor, copied from CE040's one-allowed-site shape and equally narrow: an `02d` format spec fires only when the formatted value NAMES a replicate, so a clock formatter is not told to call a replicate helper. Its docstring states what it does not catch (zfill, %-format, a `??` glob, a renamed local) and that shape 2 is unconditional in both directions — prose pinning the old glob fires deliberately, an unrelated `[0-9][0-9]` regex is the one false positive a suppression is for. Not a shared glob constant: the three readers glob at different depths, so one constant would be concatenated at two of three sites. That rationale is in the rule's docstring so it is not re-litigated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…m notes The per-row replicate trim was spelled three times in three shapes — a nested `min` inside a comprehension in the guardrail, a `keep` local in the sibling pairing, and a `keep` plus dropped-observation accounting in the gate — and only the third surfaced the trim to the user. `_balance_pair` is the one declaration, generic over the element type for the same reason `_floor_from_clusters` is: the guardrail trims floats, the F1 and sibling paths trim label pairs. `measure_execution_noise_floor`'s row-wise minimum stays separate. It is a minimum ACROSS rows rather than between two arms of one row — a different computation — and it is the one exemption `test_the_trim_is_declared_once` carries. That test counts call nodes rather than matching text, and it is mutation-verified: injecting a trim into `_pool` makes it fail naming `_pool`. The four notes both Holm wrappers emit were byte-identical copies 600 lines apart, two of them wrapped differently in source while producing the same string. A wording fix applied to one would have left the two tracks describing the same decision differently in a ledger read back weeks later. The `dropped` accounting now derives from the returned lengths, which is algebraically identical to the old `- 2 * keep`. Every note GUARD is untouched — the activation track still appends the outside-the-family note unconditionally while the execution track guards it on `gate_refusal is None`, and that asymmetry is deliberate. The zero-row and below-MDE notes are NOT collapsed, and a comment says why so a later reader does not "finish the job". Witnessed by Phase 1's six render pins and Plan A's four verdict pins, all byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Four extractions, all behaviour-preserving. No E or F block remains in the module: execution_gate F(50) -> D(25), activation_gate F(45) -> below D, render_row_matrix E(34) -> below D, holm_promote E(33) -> D(27). `_load_and_pair` takes the six concerns that were interleaved in `activation_gate`'s first hundred lines — loading, pairing, the zero-row note, hollow-row exclusion, replicate balancing and the wrong-index note. None of them touches a statistic and every one appends to `notes`, which is why they read as one step. `_PairedRows` carries the clusters and the flattened pairs rather than the mapping they come from, so the caller does not re-flatten what the helper just flattened. The returned `notes` list is THE list, not a copy: pydantic copies it at model construction, so a note appended afterwards is silently discarded. A test pins the identity. `_refusal_message` lifts a ~60-line message builder out of a boolean decision. Its early return is not a plain De Morgan of the guard it replaced, and that mattered: every comparison against NaN is False, so a NaN floor refused nothing before, while `p_floor <= threshold` would fall through to `math.floor(alpha / nan)` and raise out of the skill's inline snippet. Spelled `not p_floor > threshold`, which is exact on NaN and on inf. Pydantic rejects non-finite floats so no validated verdict can carry one — the test builds it with `model_construct` and says as much. `_execution_diagnostics` takes the tail of five independent post-statistic checks. It returns the first cause rather than calling `_refuse` itself, so first-cause-wins stays in one place; `mean_diff`, `effect_size` and `bounds` are hoisted above the call because the final `_verdict` reads them, and hoisting a pure computation above the `empty_arms` CHECK reorders no refusal. The gate's `_verdict` and `_refuse` closures are byte-identical. Witnessed by Plan A's four verdict pins, Phase 1's six render pins, and CE039's behavioural claim, which builds a run directory and calls `activation_gate`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…perty Every construction site set `accepted` to exactly `beats and blocker is None`, so the tuple carried two spellings of one rule that a caller could set inconsistently with nothing to notice. It is now a property. `beats` and `blocker` stay two fields, which is what the docstring's defence was actually arguing for: a corpus block and a plain loss call for opposite next actions — "look at the row and decide" against "write the next hypothesis" — and that distinction lives in those two, not in their conjunction. The CLAUDE.md sentence claiming `beats`/`accepted` were the pair is corrected. `_refused` constructed positionally, so dropping a field would have shifted every later argument silently. It moves to keyword form in the same edit — that is the point of the change, not tidying alongside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…f the gate `optimize_gate.py` was ~2,960 lines doing three jobs. It now does one. `reports_optimize.py` takes the seven render functions plus their helpers and `COST_FRONT_ADVISORY`, joining the reports.py / reports_experiment.py / reports_junit.py family. `optimize_store.py` takes the measurements.json sidecar — a cache (noise floors, replaced per key) and a corpus (regression rows, append-only) in one file, which is why its two writers differ. Both are carved out on the precedent leak_detection.py already set: the gate DECIDES, one sibling RENDERS the decision and the other PERSISTS it. The dependency runs one way and a test pins it rather than a comment claiming it. `reports_optimize` has zero runtime `coder_eval` imports beyond `models` and `reports_stats.bootstrap_p_floor` (a display value CE040 requires be derived there), makes no filesystem call, and takes the two NamedTuples it renders under `if TYPE_CHECKING` only — a renderer that reached back for a run directory or an estimator would be a gate with a table on it. `optimize_store` imports `models` and nothing else; `UNRESOLVED_MODEL` lives there rather than with the gate because it is a cache-key sentinel, and keeping it in the gate would close a cycle. The gate imports exactly two names back, which is what Spike D measured and what ruff's F401 keeps minimal. `test_a_moved_name_is_gone_from_the_gate` is derived on BOTH sides: each module's contents by enumeration with a `__module__` filter, the permitted survivors by parsing the gate's own import statements. A hand-written list would be a second declaration of each module's contents — the exact defect this plan removed three of. Everything moved is byte-identical: the blocks were cut and re-emitted by a script that asserts each is found before deleting it. Plan A's four verdict pins and Phase 1's six render pins are unchanged, which is what makes that claim checkable rather than asserted. The plugin's SKILL.md snippets are re-pointed at the module that exports each name, alphabetised within each block and isort-ordered across them, because a reader copies them verbatim. The derived snippet sensor follows automatically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…split CE049 (a duplicated long prose literal across two functions) is Plan D's, and Plan D lives in an untracked planning file — so it is recorded on the committed surface rather than left to a document nobody else can see. The interim guard covering the four strings this run collapsed is named alongside it. The second is a class the run surfaced rather than one it fixed: a prose sensor that scans source as raw text fires on a module that DOCUMENTS the token it bans, which is what the new presentation module did to the CLI-freedom assertion. Fixed at the instance; the general form needs every such sensor re-pointed at the AST. 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 30 commits August 14, 2026 13:51

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 98 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!

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!

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