diff --git a/.claude/decisions/2026-08-20-anti-memorization-and-search.md b/.claude/decisions/2026-08-20-anti-memorization-and-search.md new file mode 100644 index 00000000..48ca5119 --- /dev/null +++ b/.claude/decisions/2026-08-20-anti-memorization-and-search.md @@ -0,0 +1,40 @@ +# The anti-memorization preflight, and what `search_compare` is not + +Subject: `optimize/search.py::candidate_leaks`, `skill_text`, `search_compare`, +`leak_detection.py`. + +## Why the leak preflight must be handed a DIRECTORY, not a file + +`candidate_leaks` compares TEXT; `skill_text(skill_dir)` is the reader, and the CALLER owns which +of the two it passes. That split is why the obligation has to be stated rather than assumed: a skill +is a directory, and handed `SKILL.md` alone the preflight comes back CLEAN for a candidate that +bundled train-row content into `scripts/` or a reference file — byte-identical to a genuinely clean +result, which is the worst shape a preflight can have. + +## Why it shares its primitive with CE061 rather than reimplementing it + +`LEAK_LOCATOR_FIELDS`, `LEAK_MIN_CHARS`, `string_leaves` and `graded_strings` are ONE declaration +with TWO consumers pointing in opposite directions: CE061 asks whether a dataset row's PROMPT +contains a value a criterion grades it on; `candidate_leaks` asks whether a candidate SKILL.md newly +contains train-row content. A second copy would agree on ordinary input and diverge exactly where +either was written for. + +They differ in one behaviour, and it is a parameter rather than a fork: `graded_strings(drop_type=)`. +CE061 keeps the discriminator — a PROMPT saying "skill_triggered" is worth flagging — while +`candidate_leaks` drops it, because a skill BODY discussing eval criteria names types legitimately. + +The primitive lives in its own module rather than on `optimize.gate` because a task-lint rule +importing from the optimize gate inverts the dependency, the same separation `pricing.py` and +`path_utils.py` already have. + +## `search_compare` is emphatically not a gate + +It is an accept/revert decision inside a search, over one arm at a time, and it does not correct for +multiplicity. Calling it a gate invites a reader to promote on it, which is the one thing it cannot +support: the gates are `activation_gate` and `execution_gate`, and both go through Holm. + +## A hole is absent, never zero + +Every front and every comparison here treats a missing measurement as missing. Folding it to 0.0 +makes an arm that failed to produce a number look worse than one that produced a bad one, which +inverts the ranking rather than merely biasing it. diff --git a/.claude/decisions/2026-08-20-instrument-provenance.md b/.claude/decisions/2026-08-20-instrument-provenance.md new file mode 100644 index 00000000..774329eb --- /dev/null +++ b/.claude/decisions/2026-08-20-instrument-provenance.md @@ -0,0 +1,56 @@ +# Instrument provenance: what the two fingerprints cover, and why there are two + +Subject: `suite_fingerprint.py`, `models/optimize.py::RoundScores.suite_fingerprint` / +`grader_fingerprint`, `optimize/store.py::suite_changed` / `grader_changed`. + +## Why a second fingerprint was needed + +The grader fingerprint covers the outcome track's script and answer key. It cannot see a `weight` +change that re-blends `weighted_score` — the very number the execution gate's paired *t* compares — +and the activation track has no script grader at all, so it had NO instrument provenance of any kind. +A round's numbers were comparable across rounds only by assumption. + +## The ROWS are load-bearing, not decoration + +`activation.yaml` is `initial_prompt: ${row.prompt}` with `expected_skill: "${row.expected_skill}"`, +so every prompt AND every label lives in the rows file. A digest over row IDS alone — which is what +the first implementation did — is blind to a rewritten prompt and a flipped label: the commonest +suite edit there is, on the track this digest is the sole provenance for. + +## A DENYLIST, not an allowlist, and this is the one place that is the safe direction + +`scoring_dump` minus `_NOT_SCORING_RELEVANT` (a reason-carrying denylist of exactly one field, +`description`). An allowlist over `BaseSuccessCriterion`'s five fields silently omits every subclass +parameter — which is how the first draft could not see `run_command.command` move from `verify.py` to +`verify2.py`. + +## `run_limits` is hashed WHOLE + +Rather than through four curated caps: the three token caps abort a run exactly as `max_usd` does, +and `stop_early` is the kill switch for every armed criterion, so it moves `f1.yes` itself. + +## Length-prefixing and section tags are REDUNDANCY here + +Both are present, as in `verify.py::fingerprint`, but `_canonical` — canonical JSON per part — is +what actually stops a value forging a delimiter or a part migrating between sections. A mutation test +proved the obvious attribution wrong. + +## Order sensitivity is deliberate and asymmetric + +Order-SENSITIVE across criteria, because `criterion_index` is positional everywhere in this family. +Order-INSENSITIVE within a mapping and across rows. + +## What it excludes, and the one boundary that follows + +The TASK-LEVEL agent and sandbox blocks, so two checkouts agree. One stated consequence: an +`agent_judge` criterion embeds its own agent config and is hashed whole — which is right, since the +judge's model is part of what it measures, and which makes such a suite's digest machine-local. + +## Digest only, never the pre-image + +`measurements.json` is committed. + +## Three-valued, both of them + +`suite_changed` and `grader_changed` return "changed" / "unchanged" / "cannot tell" rather than a +bool. A round with no recorded fingerprint is not a round whose instrument matched. diff --git a/.claude/decisions/2026-08-20-stage-c-confirmation.md b/.claude/decisions/2026-08-20-stage-c-confirmation.md new file mode 100644 index 00000000..24a6b450 --- /dev/null +++ b/.claude/decisions/2026-08-20-stage-c-confirmation.md @@ -0,0 +1,51 @@ +# Stage C: did the Stage B effect reproduce? + +Subject: `optimize/gate.py::classify_confirm`, `build_confirm_verdict`, `confirm_split_check`, +`confirm_one_candidate`, `optimize/activation.py::confirm_gate`, +`optimize/execution.py::confirm_gate_execution`, `optimize/activation.py::gate_seed_stability`. + +## A family of ONE, and that is correct + +Only the Stage B winner is confirmed, so there is no multiplicity to correct. A reader who expects +Holm here is looking for a correction over hypotheses that were never tested. Holm is still applied +at `m = 1`, purely so the carried block is a DECIDED one rather than rendering as `UNDECIDED`. + +## The train effect is READ, never recomputed + +It comes off the Stage B verdict, so the two numbers the block compares cannot disagree with the +blocks they were reported in. + +## The margin is the confirm split's OWN MDE + +Which is what makes the rule per-track without a second declaration: each track passes its own +gate's floor, on its own metric. Picking a different multiple on one track would be a second +declaration of "how much shrinkage is real". A floor of `None` or 0.0 leaves the margin UNDEFINED +and the outcome is `undecided` rather than silently SHRANK — 0.000 means the floor could not be +priced, never that the suite can resolve anything. + +## Why the split check is shared and must not be an if/elif over the collapsed value + +`SplitProvenance.value` collapses to `UNRECORDED_SPLIT` when ANY pooled dir is unreadable. So a chain +reading `if unrecorded: note / elif value != "test": refuse` drops the refusal entirely for three +dirs recording `train` beside one unreadable `run.json` — and the confirm then classifies over TRAIN +rows carrying only a "provenance is missing from 1 of 4" note. That is precisely the failure the +refusal exists for. + +The execution twin takes ONE run dir and cannot reach that state, which is exactly why the rule may +not live on each track separately: the safe one would keep working while the other drifted. The +activation side had already gained a "not the Stage B winner" note the execution side lacked, while +its docstring claimed both worked "for the reasons the execution twin's docstring gives". + +## A recorded `train` is a REFUSAL; an unrecorded split is a NOTE + +A recorded `train` means Stage C silently re-ran the train rows, at full price, with no error +anywhere — an effect reproduces on its own training data by construction. An unrecorded split is a +run predating the field. + +## Seed stability carries no single `promoted` field + +Collapsing three disagreeing seeds into one verdict is the exact thing it exists to prevent: a +decision that flips with the seed is a coin flip, and reporting the majority's answer as *the* answer +hides that. Its `promote_agreement` counts promotions at a family of ONE, which is not the round's +decision when the round gated more than one candidate — stated because the number invites that +reading. diff --git a/.claude/decisions/2026-08-20-the-advisory-fronts.md b/.claude/decisions/2026-08-20-the-advisory-fronts.md new file mode 100644 index 00000000..e446bedd --- /dev/null +++ b/.claude/decisions/2026-08-20-the-advisory-fronts.md @@ -0,0 +1,53 @@ +# The three fronts, and why only one of them is a shortlist + +Subject: `optimize/fronts.py::arm_row_scores`, `pareto_front`, `instance_best_front`, +`cost_quality_front`, `cost_quality_points`, `headroom_ceiling`. + +## Three fronts, three different jobs + +- **Pareto** is a DISCARD rule. An arm dominated on every axis cannot be the answer, so dropping it + costs nothing. +- **Instance-best** is a MERGE shortlist: the arms that win at least one row. A candidate that wins + nowhere has nothing to contribute to a merge. +- **Cost/quality is ADVISORY only.** It is a 2-D Pareto filter over (quality, cost) — the arms + nothing beats on both — returned in INPUT order, with no ranking inside the front. It is NOT a + ratio: a ratio has no defensible threshold, and inventing an order here is exactly what would turn + an advisory into a shortlist, inviting promotion of the cheapest arm that happens to score. + +Reading the wrong front as the others is the failure this separation exists to prevent, and it is a +reading error rather than an arithmetic one, so the names carry the distinction. + +## A hole is absent, never zero — on all three + +An arm with no measurement on a row is missing there. Folding the hole to 0.0 makes an arm that +failed to produce a number rank below one that produced a bad number, which inverts the front rather +than biasing it. `is not None` rather than truthiness, because a free model is legitimately the +cheapest arm and `0.0` is a real cost. + +## Domination is gated on row-set COVERAGE, and that conjunct is load-bearing + +Without `row_ids <= other_ids`, an arm measured on a SUBSET can dominate one measured on more rows. +Measured: an arm that crashed 5 of 6 rows and scored 1.0 on the sixth knocked the incumbent off the +front. A count cannot express this — two arms on four disjoint rows each would both look entitled to +dominate the other. + +## `headroom_ceiling` bounds what is left to win, not what was won + +It answers "is another round worth paying for?" for ONE arm. Read as a score it is meaningless; read +as a ceiling it is the only number in the family that can say *stop*. + +Two things about it were deleted once by an over-eager docstring trim and are the reason this section +exists. The ceiling's denominator is the arm's FULL finite row count and never the selected subset — +a rule failing 3 of 15 rows has a ceiling of 0.1, and dividing by the subset overstates it 5x, which +makes every rule look promotable. And `rows=None` (every row) is a different question from +`rows=set()` (an empty selection), which matters because `rule_row_map` OMITS a rule that failed +nowhere: passing that missing entry as `None` reports the whole suite's ceiling under that rule's +name. + +## A contaminated tree WARNS here rather than refusing + +The return types are vectors and fronts with nowhere to put a refusal, and the caller is a human +reading a Stage A table rather than a gate deciding a promotion. So `arm_row_scores` reconciles and +logs. That is a deliberate asymmetry with the two gates, and CE053 is what keeps the reconciliation +from being dropped altogether: measured, without it `arm_row_scores` returned a stale row in its +vector and all three fronts were computed over it. diff --git a/.claude/decisions/2026-08-20-the-execution-gate-refusals.md b/.claude/decisions/2026-08-20-the-execution-gate-refusals.md new file mode 100644 index 00000000..ed842eb8 --- /dev/null +++ b/.claude/decisions/2026-08-20-the-execution-gate-refusals.md @@ -0,0 +1,98 @@ +# The execution gate's refusal causes, and why their ORDER is the rule + +Subject: `optimize/execution.py::execution_gate`, `_execution_diagnostics`, `_refuse_*`, +`_below_mde_findings`, `models/optimize.py::ExecutionGateVerdict.gate_refusal`. + +## First cause wins, and program order IS the precedence + +Every cause answers the same question — *is this a result?* — with the same consequence, so they +share one field, one headline and one prose token. They differ in REMEDY, and a later cause is +usually an earlier one's consequence: if there was no comparison to make, the rows are moot; if the +rows never loaded, whether their differences vary is moot. So the earliest cause is the one whose +remedy comes first, and routing every setter through one sink says that once instead of leaving +eleven `if gate_refusal is None` guards to be kept in agreement. + +The concrete case: a mistyped variant id makes that arm load ZERO rows as a consequence. Refusing on +the consequence replaces a message naming the two ids the experiment actually carries with one that +can only say "a wrong variant id, a wrong suite id or a wrong run directory". + +## Why an arm with no rows is a refusal rather than a note + +This track's statistic comes from `experiment.json`, not from the row tree — so it computes +perfectly well over rows that are not on disk, while every guardrail and integrity check reads green +over nothing. A valid experiment file beside a mistyped path renders as PROMOTED with every check a +green `— -> —`. + +## The below-MDE refusal is deliberately TWO-SIDED + +`mde` is the half-width of a bootstrap interval on a NULL difference, so a difference under it is +indistinguishable from the suite's own run-to-run noise however small the p is. But under the null a +candidate's difference is ALSO small: `abs(mean_diff) < mde` is true for nearly every candidate that +simply does not work — measured, 40 of 40 true-null candidates. Refusing all of them would retire +NOT PROMOTED almost entirely and send the reader to buy replicates for a candidate whose problem is +that it is null. + +So the refusal is conditioned on the interval EXCLUDING zero. An interval that contains zero is the +data agreeing the candidate is null: an ordinary negative result, and it stays one. What is left for +the refusal is the pathology — a confident claim, in either direction, about an effect the +instrument cannot see. + +## Zero variance splits into two messages + +At a constant difference of ZERO the arms behaved identically, which is a finding about the +candidate that no number of extra rows can change, and `paired_t_test` reports p = 1.0 there rather +than the 0.0 a non-zero constant shift gives. One message would state a p the block below it +contradicts. The same split, for the same reason, as `holm_promote`'s `p_floor >= 1.0` branch. + +## The interval-tighter-than-floor case is a caveat, NOT a refusal + +The paired *t*'s interval comes from the BETWEEN-ROW spread of the differences, which is tiny +whenever the arms differ by a similar amount on every row, while `mde` measures WITHIN-row noise the +*t* never sees. So a real, large, consistent win reports an absurd p. Refusing it would be worse +than the defect: measured, a genuine 8-row 0.30 win reports a half-width of 0.007, the same shape as +the 0.400-on-every-row case. What is wrong there is the reported PRECISION, not the decision. + +## Every note is suppressed under a refusal + +A refusal says the comparison decided nothing; a note beneath it is a second, contradictory claim on +a page a user pastes into a promotion ledger. The below-MDE note calls itself "an ordinary negative +result", and it was the one rung that fired regardless — reproduced through the real gate, a +zero-variance refusal printed it beneath `NOT A RESULT`. `promoted` was unaffected, so this was +prose only. + +`refused_already` is OR-ed with the local cause because "nothing has refused yet" has to include +what the diagnostics themselves decided three lines up. TWO paths arrive already refused and neither +returns early: the stale-tree cause and the primary-index cause. + +## The primary-index refusal must be recorded BEFORE the diagnostics run + +That ordering is load-bearing, not tidy. Recorded after, it produced a `NOT A RESULT — +primary_criterion_index=7 selected no usable row` headline above notes reading "this is an ordinary +negative result and not a measurement problem" and "the paired interval is tighter than this suite's +own noise floor" — measured. + +`require_valid_criterion_index` bounds only BELOW, deliberately, since rows may legitimately differ +in criteria count and an over-long index should skip a row rather than raise. That is the wrong +answer here: an over-long primary index makes `row_score` return `None` on every row, so the vector +is EMPTY and indistinguishable from a suite whose rows all errored on that criterion. + +## Dead weight is a READING and can never gate + +Measured rather than argued: a constant criterion scales the paired difference vector without +changing its shape, so it scales the mean AND the standard deviation by the same factor — the paired +*t* is identical to 1e-12 between the grader-only and blended scales while the mean difference scales +by 1/2.05. The bootstrap interval scales with the data, the MDE is measured on the same blended +scale, and the guardrails never touch the blend, so EVERY conjunct of `promoted` is invariant to it. +Wiring it into `integrity_checks` would force `promoted = False` on comparisons that are +statistically sound — strictly worse than the presentational problem it would be fixing. The one +case where dead weight genuinely invalidates a comparison is every criterion being constant, which +is already the zero-variance refusal. + +## A stale tree FLIPS the answer rather than merely being reported + +`run.json` is written per INVOCATION while the tree is APPEND-ONLY, so a re-used `--run-dir` leaves +an earlier call's rows — or, with a smaller `--repeats`, its replicates — on disk, and they are +pooled into the comparison and into the checks that gate it. Measured on an identical winning +candidate: four unrecorded incumbent replicates moved `completion_rate` from 1.0 to 0.667 and +`promoted` from True to False, with no refusal and no note. Contaminate the candidate arm instead +and the error runs the other way. diff --git a/.claude/decisions/2026-08-20-the-noise-floor.md b/.claude/decisions/2026-08-20-the-noise-floor.md new file mode 100644 index 00000000..164b9497 --- /dev/null +++ b/.claude/decisions/2026-08-20-the-noise-floor.md @@ -0,0 +1,59 @@ +# The noise floor: what it measures, and the four ways it read zero + +Subject: `optimize/gate.py::floor_preflight`, `no_floor`, `floor_from_clusters`, +`optimize/activation.py::measure_noise_floor`, `noise_floor_mde`, +`optimize/execution.py::measure_execution_noise_floor`. + +## A silent `None` is indistinguishable from a floor of zero + +Both floor functions return `None` for several distinct reasons, and the caller is an agent about to +decide whether to spend money. Verified against the shipped code: `noise_floor_mde` with a mistyped +run directory returned a bare `None` and printed nothing — on the one function whose job is to stop +a user spending. `no_floor` is now the single reporting channel, and an unconfigured +`logging.warning` reaches stderr through Python's last-resort handler, so the agent driving the +skill's inline snippet sees it without any logging setup. + +`reasons` is an out-parameter SINK rather than a widened return type because `noise_floor_mde` is +public and imported by those snippets: changing its `float | None` would break a user's terminal. + +## Why the preflight's order cannot be reversed + +Reconcile BEFORE load, so a contaminated tree costs no parse — and, more importantly, so a WRONG +path still wins its own case: a wrong path leaves nothing on disk to be unrecorded. Reversed, a +mistyped variant id reports a contaminated tree and sends the reader to check `--repeats` instead of +the path they mistyped. + +Measured on dirs `activation_gate` correctly refuses: `measure_noise_floor` returned a floor +computed over an extra pooled row, and `arm_row_scores` returned the stale row in its vector. The +floor decides whether a round runs at all. + +## An unrecorded dir is a NOTE, never a refusal + +The family's settled missing-provenance stance, so old run dirs stay measurable. `reconcile_arms` +logs it; neither floor has a `notes` channel to surface it in, which is why the count is unused +there. + +## A floor of exactly 0.000 is a real answer + +It means every row's replicates agreed exactly — a deterministic suite, or one whose rows all failed +the same way. `measured.mde if ... is not None`, never `measured.mde or None`: truthiness would +erase it. A reader who is not told this reads "Minimum detectable effect: 0.000" as "this suite can +resolve anything", which is the opposite of what an unmeasurable floor means. + +## Balancing before splitting, on both tracks + +`cluster_bootstrap_diff_ci` pools the drawn clusters' OBSERVATIONS before applying the statistic, so +an unbalanced row weighs 2:1 across the halves while a balanced one weighs 1:1 — and between-row +spread then leaks into a difference that is supposed to be zero by construction. Measured: 8 rows +with NO within-row variance report 0.000 at uniform counts and 0.056 when half of them carry 2 +replicates. + +## The execution floor measures `weighted_score`, not `f1.yes` + +Computing an F1 floor for a gate that never reads F1 is the bug that function replaced. On the +bundled outcome template it returned a confidently meaningless 0.000. + +## An odd count splits unevenly, and that is the safe direction + +Three replicates split 2/1, which widens the interval and therefore reports a CONSERVATIVE floor — +the same on both tracks. diff --git a/.claude/decisions/2026-08-20-the-promotion-decision.md b/.claude/decisions/2026-08-20-the-promotion-decision.md new file mode 100644 index 00000000..ee245d90 --- /dev/null +++ b/.claude/decisions/2026-08-20-the-promotion-decision.md @@ -0,0 +1,64 @@ +# What `promoted` means, and every way it has been wrong + +Subject: `optimize/gate.py::decide_family`, `optimize/activation.py::holm_promote`, +`optimize/execution.py::holm_promote_execution`, `models/optimize.py::GateVerdictBase.promoted`. + +## The field used to mean two different things + +`promoted` was computed in two places, 700 lines apart, and the two expressions were not the same +one. The activation track folded in its `sibling_checks` and left the cost/latency `guardrails` +advisory — for the skill's PROSE to gate on — so a candidate that materially raised what a row cost +came back `promoted=True` while the rendered block called it BLOCKED. A caller reading the field +could ship what the page said not to. + +Both tracks now go through one loop and one conjunction: Holm rejected AND `separated` AND no +refusal AND `failed_vetoes` empty. `failed_vetoes` is the single declaration of which lists veto. + +## Folding the veto in is only safe because the statistical half has its own name + +`separated` exists so the renderer can tell a candidate that LOST from one that WON AND WAS BLOCKED. +Read `promoted` for that second question and the BLOCKED rung becomes unsatisfiable the moment the +veto is folded in — a blocked winner degrades silently to the ordinary NOT PROMOTED headline, which +is the one thing a reader must not confuse it with, because the two call for opposite next actions. + +Measured: a candidate whose sibling check failed rendered as NOT PROMOTED, indistinguishable from +one that simply lost, until `failed_vetoes` was made the single declaration (`cab79de`). + +## `holm_rejected` is stored because it cannot be derived + +`holm_alpha` records the family-wide alpha, never the rank-dependent threshold, so a reader holding +`p_value` and `holm_alpha` cannot tell a rejection from a near miss — the family SIZE decides, and +only the function that saw the whole family knows it. Without the field, the BLOCKED headline also +fires on a candidate the family correction never rejected, sending the reader to fix cost when the +real problem is power. Measured: two candidates at p = 0.03 in a family of two, identical in every +statistic, rendered BLOCKED and NOT PROMOTED purely because one carried a failing cost check. + +## The refusal conjunct is load-bearing on both tracks + +Not belt-and-braces. On activation, `p_floor` bounds the p's EXPECTATION, so a realized p dips below +it on roughly half of all seeds — measured, 16 of 30 on the 6-row fixture at 20,000 draws. On +execution, a zero-variance verdict reports p = 0.0000 over a zero-width interval, so `separated` +holds on it too. Without the conjunct an undecidable comparison promotes AND carries a refusal: two +contradictory claims in one block, which is the defect `gate_refusal` exists to fix, reborn. + +## A refused verdict with a real p stays in the family + +Membership is `p_value is not None` and nothing else. Holm corrects for the hypotheses actually +tested, and dropping a measured-but-degenerate candidate shrinks `m` and LOOSENS `alpha/m` for its +siblings — the uncorrected-`p <= alpha` degeneration from the other side. Measured: three gate runs +with two below-MDE refusals promoted a p = 0.027 sibling that a family of three rejects. + +## Why the family size is `len(family)` and not `len(verdicts)` + +The two differ exactly when a member has no p. Mutating it to `len(verdicts)` passed the ENTIRE +suite, because every case in the sensor class used a family whose members were all measured. Three +assertions now cover it — the ladder rung, the trailing note and the activation refusal's own +sentence — and all three need a mixed-membership family to say anything. + +## One measurable difference from the two loops it replaced + +The execution wrapper omitted `gate_refusal` from its `copy_with`; the unified loop writes it on +every measured path, from the hook, which returns the verdict's own value. The VALUE is unchanged — +verified over a 10,982-state differential against both old loops — but the key now enters +`__pydantic_fields_set__`, so `model_dump(exclude_unset=True)` includes it on an execution verdict +built without it. Nothing reads a gate verdict that way today. diff --git a/.claude/decisions/2026-08-20-the-rendered-verdict-block.md b/.claude/decisions/2026-08-20-the-rendered-verdict-block.md new file mode 100644 index 00000000..cd05f3ed --- /dev/null +++ b/.claude/decisions/2026-08-20-the-rendered-verdict-block.md @@ -0,0 +1,48 @@ +# The rendered verdict block: five rungs, and the order is the contract + +Subject: `reports_optimize.py::_headline`, `render_markdown`, `render_execution_markdown`, +`render_confirm_markdown`. + +## It replaced two hand-written chains that drifted twice + +The activation `BLOCKED` rung once read `verdict.guardrails` alone while its twin unioned both veto +lists, so a candidate that separated, cleared Holm and was vetoed by a failing SIBLING check +rendered `NOT PROMOTED` — indistinguishable from one that simply lost. Before that it keyed on +`promoted`, which the guardrail veto had made unsatisfiable. + +## The three conjuncts of the BLOCKED rung, each load-bearing + +- **NEVER `promoted`.** Both Holm passes fold the veto into it, so a blocked candidate arrives with + `promoted is False` and keying on that field makes the rung unreachable — dropping a blocked + winner into `NOT PROMOTED`, the one rung it must never be confused with. +- **`holm_rejected`**, because `separated` alone is the trap on the other side: `separated` is a + property of ONE verdict and deliberately excludes the family decision, so at `m > 1` a p between + `alpha/m` and `alpha` leaves `ci_low > 0` while Holm rejects nothing. Measured: two candidates at + p = 0.03 in a family of two, identical in every statistic, rendered BLOCKED and NOT PROMOTED + purely because one carried a failing cost check — with the note ladder printing the contradicting + "did not clear the Holm threshold" line directly underneath. +- **`failed_vetoes` rather than `guardrails`**, which spans both of a track's veto lists. + +## Why one chain taking three arguments rather than a rung table + +The two tracks differ by exactly three strings. A table plus an evaluator would add indirection a +reader has to unwind to answer "what does this print?", for two call sites in one file. The +execution track passes `NOT A RESULT` as its `refusal_label`, which is why its ladder reads as four +rungs rather than five — it reaches rung 3 with the same text rung 2 produces. That is a property of +the argument, not a special case in the chain. + +## `render_confirm_markdown` is deliberately NOT folded in + +Its `REVERSED` rung is Stage-C-specific, there is exactly one confirm renderer, and generalizing for +one caller is the speculation YAGNI forbids. + +## Why the two body renderers were not merged + +`_headline` is shared and the remaining bodies are two genuinely different field lists. Merging them +would need a field-order table — indirection for two call sites in one file. + +## The presentation layer reads no disk and decides nothing + +Pinned by a test, and that boundary is what makes the split real. Its one runtime statistics import +is `reports_stats.bootstrap_p_floor`, which the same test REQUIRES: it makes the p-floor value +derived rather than respelled (CE040). diff --git a/.claude/decisions/2026-08-20-the-skill-facing-api.md b/.claude/decisions/2026-08-20-the-skill-facing-api.md new file mode 100644 index 00000000..da4f279d --- /dev/null +++ b/.claude/decisions/2026-08-20-the-skill-facing-api.md @@ -0,0 +1,99 @@ +# The skill-facing API: why `optimize/api.py` exists, and what it is not + +`/coder-eval:optimize-skill`'s `SKILL.md` carried **427 lines of python in fifteen fences**. They were +not import lines. They were guards (`if not arms[0].row_scores: raise SystemExit(…)`), fallbacks (a +suite-level ceiling when rule attribution is unavailable — the prose called it "the difference between +an answer and a missing row"), track branches (a commented-out half plus a `TRACK = "activation"` +string the user hand-edited), session continuity (Step 11's own prose warned "run it in a fresh +interpreter and it fails with `NameError`… after the round has already been paid for"), and one +hand-written row primitive reaching into `r.success_criteria_results[grader_index].score` with no +bounds check. + +Markdown does not execute, so none of it was reachable by a test. What that cost, measured rather +than argued: **two fences computed a reported number from a run tree they never reconciled** — the +ceilings table and the per-row replicates. CE053 exists to force exactly that reconcile and could not +see them, because the rule reads `.py` files. `grep -c reconcile SKILL.md` was `0`. + +`optimize/api.py` is rank 4 of the family: 18 composites over those fences, each returning the markdown +block the skill prints. Fence lines went **427 → 138**, and the fence count went 15 → 16 — Stage C's +activation twin gained one, because a composite shown only in prose is one a reader has to reconstruct +from a sentence. + +## Why composites and not a facade + +A module re-exporting the 46 primitives would have left every guard, fallback and branch in markdown. +The point was never to shorten the import lines; it was to put the *logic* somewhere a test can reach +it. `api.py` therefore exports composites **only**, and a fence that still needs a primitive is a +fence not finished — which **CE066** makes mechanically visible. + +This is also why `optimize/__init__.py`'s no-facade rule needed no amendment: its two sensors filter +by `__module__`, so a module's own composites satisfy "defines nothing" while its `from .load import x` +names are excluded. + +## Why every composite returns `str` + +The renderers already state each decision **in words inside the block** — +`reports_optimize.py` renders `"ACCEPT into the lineage"` / `"REVERT — the head stands"`, and +`"DO NOT ACCEPT"` / `"CANNOT COMPARE"`. A caller prints the string into its ledger, and a reader of +that ledger sees the same sentences the caller acted on. A wrapper model would add a second +representation of a decision that is already unambiguous (YAGNI), and each composite's regression test +is then a whole-string comparison against a direct library call. + +## Why the new blocks live in `reports_optimize` + +`CLAUDE.md` declares that module "the optimize gate's **PRESENTATION** half — every markdown block the +skill prints". Six blocks had no renderer because the fences hand-formatted them with `print(...)`, and +four more notes had no block at all — the staleness warning, the attribution fallback, the +family-shrink notice and Stage C's family size, each of which a composite would otherwise have written +itself. **Ten** new `render_*` functions landed there rather than in `api.py`; the plan predicted five, +and the gap is what the boundary actually cost once it was enforced rather than intended. **`api.py` authors no markdown**, +and that boundary was broken twice during implementation and caught both times in review — first by +`_staleness_note`'s bolded sentence, then by the Stage C family-size line. It is a harness gap: nothing +mechanically forbids a markdown literal in that one module. + +## Why Stage C recomputes instead of persisting a verdict + +`confirm_gate` needs the **Holm-corrected** Stage B verdict, because `promoted` is what Stage C +classifies against — and `measurements.json` is `extra="forbid"` with nowhere to put one. The +bootstrap is seeded, so re-gating the family and correcting again is bit-identical; the cost is CPU +over rows already on disk (measured: ~18 s on a five-candidate activation family, of which the +recomputation is ~85%). It also removes the `NameError`-after-payment failure the skill's own prose +warned about, because every input is an argument. + +**And it must NOT refuse a candidate that merely lost.** The first implementation did, and rank 1 says +otherwise in writing: `gate.confirm_train_note`'s docstring is *"A NOTE, not a refusal: a reader may +legitimately want to confirm a candidate that separated and was then vetoed by a guardrail."* A rank-4 +composite whose contract is that it decides nothing was deciding that the other way, and made both +rank-1 helpers unreachable from the only surface the skill uses. It now refuses only a verdict with no +statistic at all — a gate that could not measure is not a candidate that lost. + +## Why per-track functions and never a `track:` discriminator + +The library splits by track everywhere (`confirm_gate` vs `confirm_gate_execution`, `holm_promote` vs +`holm_promote_execution`), so Stage B, Stage C and the ledger each get two composites. One function +with a `track:` literal would carry mutually exclusive parameters, force an un-typeable signature, and +need a runtime assert for a combination the split makes unrepresentable — a grader fingerprint on the +activation track. `record_round_activation` simply has no grader parameter, which is the whole point. + +The test that pins this asserts the **two entry points and their disjoint parameter sets**, not a grep +for the string `track:`. A grep was the first attempt and it was decoration: the module legitimately +carries `_track_verdict(…, track_name)`, so renaming a parameter satisfied it without the design +holding. + +## The one fail-open case, and why it is stated rather than prevented + +A verdict with no p-value is not a Holm family member, so an arm that refused drops out and `m` falls. +Right for that arm, **wrong for its siblings**: they were predeclared against the larger family and are +decided against the smaller, looser threshold. Measured: two mapping keys pointing at one run dir +promoted the good arm "across a family of 1" while the round had predeclared two. Every other guard in +this area fails closed; this one fails open, and only a caller holding the predeclared count can see +it. `render_family_shrunk` says so on all four Stage B / Stage C surfaces. + +## Declined: generating `reference/run-layout.md` from `.claude/shared/run-layout.md` + +Recorded so it is not re-proposed. `tests/lint_tests/test_lint_plugin_skills.py` already considered +generation and rejected it: *"Generating one hand-written file from another would add machinery +without adding a source of truth, so this byte-equality assert is the sensor instead."* That is +correct. `reference/criteria.md` is generated because it is **derived from the models**; +`run-layout.md` is hand-written prose on both sides, so a generator would be a `cp` with a Makefile +target. diff --git a/.claude/decisions/2026-08-20-tree-reconciliation.md b/.claude/decisions/2026-08-20-tree-reconciliation.md new file mode 100644 index 00000000..1a66ae36 --- /dev/null +++ b/.claude/decisions/2026-08-20-tree-reconciliation.md @@ -0,0 +1,41 @@ +# Reconciling a run tree against its own `run.json` + +Subject: `optimize/load.py::reconcile_tree_against_run_json`, `reconcile_arms`, `stale_tree_reason`, +`wrong_path_reason`, `TASK_JSON_GLOB`, `_rules_verdicts`, `rule_row_map`. + +## The failure it exists for + +`run.json` is written per INVOCATION; the row tree is APPEND-ONLY. So a re-used `--run-dir` leaves an +earlier call's rows on disk — or, with a smaller `--repeats`, its extra replicates — while +`row_selection` is rewritten to describe only the latest invocation. Everything then loads, parses +and pools without error, and the numbers are computed over a row set no invocation ran. + +It is silent in every consumer: a contaminated tree produces a confident result. Measured on dirs the +activation gate correctly refuses, four other readers of the same trees returned numbers — the noise +floor over an extra pooled row, and the Stage A vectors with the stale row included. + +## Why the glob is declared once + +`TASK_JSON_GLOB` is this module's single declaration, and `task_json_pattern` derives all four +wrong-path messages from it. Changing the glob otherwise leaves three messages describing a tree the +code no longer searches — and those messages are the whole diagnostic for the family's documented +silent-zero failure mode. + +The replicate directory's NAME is spelled once too, in `path_utils.replicate_subdir_name` (CE042). +The day that padding widens, nothing raises: the globs simply match nothing, both gates load ZERO +rows, and the zero-row message then blames a wrong variant id, a wrong suite id or a wrong run +directory — the three things that would be correct. + +## Three-valued, not boolean + +`unrecorded` (results the run.json never wrote) and `unknown` (a dir whose provenance cannot be read) +are different findings with different remedies: the first is contamination and refuses, the second is +an old run dir and notes. Collapsing them to "not clean" would make every pre-`row_selection` run +unmeasurable. + +## The rules line is an attribution, not a score + +`_rules_verdicts` reads the grader's `RULES ` line — the LAST line of stdout, compact JSON of rule id +to `"pass" | "fail" | "na"`. It is what lets a reader see WHICH check moved, beside a +`weighted_score` that only says the blend moved. A malformed line costs no score: the float on line 1 +is the measurement and the attribution is optional by construction. diff --git a/.claude/decisions/README.md b/.claude/decisions/README.md new file mode 100644 index 00000000..42f61ff0 --- /dev/null +++ b/.claude/decisions/README.md @@ -0,0 +1,50 @@ +# Decision log + +Three registers for three different kinds of sentence. Putting them in one place is what made +`optimize/`'s docstrings 40-line essays, and an essay is not read: the contract a caller needs is +buried in the history of how the code got there. + +| Register | Lives in | Answers | +|---|---|---| +| **Contract** | the docstring | What does this do, what does it return, what invariants hold? | +| **Why not the obvious alternative** | a short comment at the decision site | Why is this line shaped like this and not the way you were about to change it to? | +| **Defect history** | a dated file here | What broke once, how was it measured, and what did the fix cost? | + +## The rule + +A docstring keeps what a CALLER needs to use the function correctly. Everything that is only +interesting once you are changing it — the measured failure, the rejected design, the commit that +introduced the bug — moves here, and the code keeps a one-line pointer: + +```python +# See .claude/decisions/.md +``` + +The pointer is not optional. `tests/lint_tests/test_lint_decision_log.py` asserts that every file +here is referenced from `src/`, because a decision nobody can find from the code is a decision +nobody will read — and it will be re-litigated. + +## What does NOT go here + +- **Deferred guardrails.** Those are `.claude/harness-candidates.md`: a candidate is work not yet + done, a decision here is work settled. Mixing them was considered and rejected — a reader + scanning for "what should I build next" would have to skip past history to find it. +- **Anything a sensor asserts.** If a test binds a sentence, that sentence stays where the test + reads it. Moving it is not a documentation change, it is deleting a check. +- **The contract itself.** If trimming a docstring leaves a caller unable to use the function, the + line was contract and belongs in the docstring. Say so rather than moving it; the ratchet in + `test_lint_decision_log.py` has slack for exactly that case. + +## Why this convention has no lint rule + +"Is this sentence a contract or a defect history" is a semantic judgement, and a heuristic for it +would be a rule policing wording. What IS checked is mechanical: that no decision file is orphaned, +and that the number of over-long docstrings does not grow. This is the one invariant in the +`optimize/` family with no computational guardrail, and that is stated here rather than left to be +discovered. + +## Naming + +`YYYY-MM-DD-.md`, dated when the decision was recorded rather than when the defect shipped. +One file per subject, not per defect: a reader arrives from a pointer in one function and usually +needs the neighbouring history too. diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index e067e9ad..2546ede9 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -384,8 +384,9 @@ with the two `action.yml` items above — one considered change to the action's `timeout is None`. Caught independently by two reviewers (`bai-uipath`, `uipreliga`) on the PR, both citing the exact same arithmetic mismatch. **Not promoted in this pass**, but a stronger candidate than most entries here: `uipreliga` proposed a - generic whole-tree rule (proposed as CE035, renumbered CE042 here — CE035 shipped as - the workflow-outputs resolver on the published-action branch) — for every sleep-loop under + generic whole-tree rule (proposed as CE035, renumbered CE065 here — CE035 shipped as + the workflow-outputs resolver on the published-action branch, and CE042 as the + replicate-padding seam on the optimize-skill branch) — for every sleep-loop under `src/coder_eval/agents/**`, assert its own cycle-count × interval either references a timeout-derived name or is provably below `experiments/default.yaml`'s baseline — that would catch this class of bug in ANY agent, not just this one (confirmed zero @@ -395,7 +396,7 @@ with the two `action.yml` items above — one considered change to the action's ## From 2026-08-04 published-action verification review -- [ ] **CE041 — `VAR=$(… | grep …)` under `set -e` followed by an emptiness check +- [ ] **CE061 — `VAR=$(… | grep …)` under `set -e` followed by an emptiness check is a dead diagnostic.** With `set -euo pipefail`, a pipeline whose `grep` matches nothing exits 1, so the assignment aborts the step *before* the `if [ -z "$VAR" ]; then echo "::error::…"` branch that was written to report it — @@ -407,7 +408,7 @@ with the two `action.yml` items above — one considered change to the action's `verify-published-action.yml`; **`actionlint` + shellcheck do NOT flag it** (verified against the exact snippet), so the actionlint candidate above does not subsume this one. -- [ ] **CE036 — ban the skipped-green job gate.** Fail a job-level `if:` in +- [ ] **CE062 — ban the skipped-green job gate.** Fail a job-level `if:` in `.github/workflows/**` whose only discriminator is an emptiness/equality test on `needs..outputs.`. A lost output on a partial "Re-run failed jobs" resolves the job to SKIPPED-**green**, so an operator sees a green re-run while nothing ran. @@ -415,14 +416,14 @@ with the two `action.yml` items above — one considered change to the action's `publish-pypi`'s `if: needs.release.outputs.version != ''` (dead *and* dangerous — a skipped publish also skipped `promote`) was removed in the follow-up review. CE035 catches the *typo* class; this catches the *shape*. Escape hatch: inline - `# noqa: CE036 — ` for value-driven gates that cannot strand a release. -- [ ] **CE037 — `if: failure()` is wrong in a job containing a `continue-on-error` + `# noqa: CE062 — ` for value-driven gates that cannot strand a release. +- [ ] **CE063 — `if: failure()` is wrong in a job containing a `continue-on-error` step.** Require `always()` (or a reference to the tolerated step's `steps..outcome`) on diagnostic/upload steps in such a job. Fixed by hand in `verify-published-action.yml`: the run dir was discarded in exactly the tolerated-red case the gate is designed around, because a tolerated red leaves the job green and `failure()` never fires. Pure YAML shape check, ~30 lines. -- [ ] **CE040 — cap inline `run:` bodies; oversized decision logic belongs in +- [ ] **CE064 — cap inline `run:` bodies; oversized decision logic belongs in `.github/scripts/`.** `verify-published-action.yml`'s parity step (~70 lines, 7 decision points) and its e2e gate (~66 lines, switching from bash to a `python3` heredoc mid-step) are 10-20-branch units invisible to `make check`, `make lint`, @@ -455,3 +456,847 @@ with the two `action.yml` items above — one considered change to the action's `final_status`, which does not exist in `run.json` and would have made a new assertion dead on arrival. Guard: assert the key set that non-Python consumers depend on, mirroring how CE030 pins doc/schema parity. +## From the split-field / optimize-skill plan (2026-08-12) + +- [ ] **A run whose every task is skipped exits 0 — a green run of zero tasks.** *(Narrow case + CLOSED 2026-08-13: a `--split` selector that matches no labelled row now raises + `SplitSelectorError` out of `expand_dataset`, which `resolve_all_tasks` re-raises and the CLI + turns into a `typer.BadParameter` — exit 2. The GENERAL case below stays open: `skip: true`, + load failures and tag filters that match nothing all keep today's exit-0 behaviour, because + making those fatal changes exit semantics for deliberate quarantine workflows and needs its own + decision plus tests per case.)* When + `resolve_all_tasks` demotes every task to `skipped_tasks` (a load failure or `skip: true` — + no longer a `--split` typo, see above), the run reports success: nothing + failed, so the exit gate in `cli/run_command.py` — which keys only on failed/errored tasks + and suite gates — passes. Verified directly before the narrow fix: `coder-eval run + --split holdou` printed one yellow "1 task file(s) skipped" line and exited 0. `--split` was + what made it reachable by a one-character CLI typo rather than a broken file, and + the whole point of a test confirmation is that you trust its verdict. Still unguarded for + the remaining paths, and + not a five-minute fix: making an all-skipped run non-green changes exit semantics for + every skipped-task path (including deliberate `skip: true` suites and tag filters that + match nothing), so it needs a decision about which of those should be fatal, plus tests + per case. A narrower option is to fail only when a CLI *selector* (`--split`, `--tags`) + eliminated everything, since that is unambiguously a user error rather than repo state. + +- [ ] **Semantic answer-leak in a task prompt** — a prompt that describes the graded behaviour in *different words* ("list the paths explicitly rather than with a recursive wildcard" while grading an explicit glob) scores well whether or not the behaviour happened, and in an A/B an arm that deleted the rule still passes. CE061 catches only the verbatim form; the semantic form needs an LLM judge or a `lint-tasks` pass over this repo's own `tasks/`, neither of which is cheap or deterministic. — caught in the final review of c/2026-08-13-optimize-skill-fixes.md, where 4 of 10 rows in a shipped worked example had it. +- [ ] **A doc claim that contradicts merge semantics** — `optimize-skill` told users to declare `allowed_tools` in an experiment's `defaults: agent:`, which is a silent no-op because those fields merge by `replace` and the task layer outranks experiment defaults. Detecting "this prose recommends a config location that the merge order makes ineffective" would need the rule to model the layer stack against prose, which no existing rule shape supports. — caught in the final review of c/2026-08-13-optimize-skill-fixes.md. +- [x] **`_normalized()` not used by every prose sensor** — CLOSED: all 9 sites converted, and `test_no_sensor_inlines_the_normalization_idiom` now forbids the raw form. Original note: — 8 sensors in `tests/test_custom_lint.py` still inline `" ".join(path.read_text().split())`, so a future one copied from the wrong neighbour is defeated by a line wrap (the bug that let a stale skill count ship past 91 green tests). A rule forbidding the raw idiom in that file is easy; the conversion sweep was out of scope. — caught in the final review of c/2026-08-13-optimize-skill-fixes.md. + +## From the optimize-skill review v2 plan (2026-08-13) + +- [ ] **"The ToolStart seam decides" is now a PER-CRITERION property, not a global invariant.** `command_executed`'s verdict is decidable from the tool call's inputs **only while `require_success` is unset** — with it set, `_matching_commands` drops the in-flight call, whose `result_status` is `None`, and the criterion decides at ToolEnd like any other (corrected in Plan D Phase 4; the entry originally stated the unconditional form, which is false for the configuration CE034 mandates). `skill_triggered`'s is never decidable there (for the `Skill` tool the body is delivered AS the result, so an in-flight call engaged nothing). A new `LiveSuccessCriterion` must state which seam its `live_verdict` is decidable at, and a criterion that decides at the ToolStart on information only the result carries silently diverges from its own frozen check. Not mechanically detectable today: the property is about what a `live_verdict` implementation *reads*, which no AST rule can infer — a rule would have to know that `result_status` is the field distinguishing the two seams. A cheaper partial guard would be a test-level convention (every live criterion has a "not decided before the result" or "decided on the call" test), which is a sweep rather than a rule. — caught implementing Phase 1 of c/2026-08-13-optimize-skill-review-v2-fixes.md. + +- [x] ~~A prose claim in `plugins/` about `src/` behaviour that no sensor checks — the token sensors + check PRESENCE, never TRUTH. Shipped false twice in one change: "the gate cannot be computed + from one run dir" (it can) and a halving cost saving that was arithmetically a premium. Only + `test_optimize_skill_snippet_names_the_public_gate_api` checks a claim against the code, and + each such sensor is bespoke — there is no general form. — caught in the optimize-skill gate + corrections review, 2026-08-13.~~ **CLOSED 2026-08-14 by CE064** (`tests/lint/computed_claims.py` + + `tests/lint_tests/test_lint_computed_claims.py::TestCE064ComputedClaims`). The general form is a `ComputedClaim` + registry whose entries *compute* the claim, plus the coverage rule that makes it a class + rather than N bespoke sensors: an arithmetic-bearing table in the two optimize surfaces that + no registered claim names **fails**. Three claims shipped with it — `cost-table` (asserts + invariants of the cost model: halved is never cheaper than flat at any N in 2..32, activation + Stage B is exactly 3x Stage A per arm, the control row's paired figure is 2x its unpaired one, + and Stage C alone is priced in `M_test`), `halving-premium` (recomputes every cell and the + standing never-saves claim), and `interval-from-one-run-dir` (**behavioural** — builds a + one-run-dir fixture and asserts `activation_gate` returns an interval but no MDE, which is + the exact claim whose false version shipped). Both self-tests are committed: one proves the + real matcher catches a wrong `premium` cell, one proves the real coverage matcher reports an + unregistered table — neither needs a shipped file edited to demonstrate. +- [ ] Changing a module-level statistical constant in `reports_stats.py` silently reddens + `tests/_fixtures/report_snapshots/`, which no phase's scoped tests run. A blast-radius check + ("these fixtures are downstream of these constants") is not obviously expressible without + hardcoding the pairing it is meant to discover. — caught in the same review. +- [ ] A note appended to a local list AFTER that list has been passed into a Pydantic model + constructor is silently discarded — pydantic COPIES the list during validation, so the + append mutates a detached object nobody reads. Cost a High finding in `execution_gate` + (the below-MDE warning and the zero-variance effect-size explanation never reached a + reader, on exactly the cases they exist for), and `activation_gate` avoids it only by + appending before its return. Mechanically detectable in principle — flag a `X.append(...)` + on a name previously passed as a constructor argument in the same function scope — but the + alias analysis to do it without false positives (the list may legitimately be rebuilt, + reassigned, or passed by `model_copy`) is not a 30-minute rule, and a noisy version of this + one would be ignored. — caught in the optimize-gate v8/v2/v3/v5/v1/v4/v6 review. +- [ ] A prose surface must not restate a formula `src/` owns. The two optimize surfaces already + have sensors forbidding rendered CONSTANTS (`MATERIALITY_FLOOR`, `GATE_RESAMPLES`, + `DEFAULT_ALPHA`), but nothing stops a closed form being retyped into a paragraph — and one + was, with a wrong factor, in the same change (`2*(1-R/M)^M` and its limit). It is the + CE062/CE040 class one level up, in prose. A narrow `^M`-shaped detector is cheap but would + claim more generality than it has; defining "a formula" precisely enough to gate on is the + part that is not cheap. CE064 does not reach it: that rule covers arithmetic-bearing + TABLES, and this was a sentence. — caught in the same review. +- [ ] A helper that takes caller-supplied keys must not index its own mappings directly. + `cost_latency_guardrails` did `rows[rid]`, which was safe while its only caller passed the + intersection of those maps — and became a `KeyError` the moment a second caller + (`execution_gate`) passed row ids derived from `experiment.json` instead. The general shape + is "a parameter documented as caller-supplied is used as a subscript into a parameter + documented as the callee's own data", which needs the two to be related by more than types; + an AST rule would either miss it or flag every legitimate lookup. — caught in the + optimize-gate v8/v2/v3/v5/v1/v4/v6 final review. +- [ ] Two code paths computing the SAME metric over the same rows must apply the same + normalization. `activation_gate` balances per-row replicate counts before pooling for the + primary criterion — with a six-line comment about why — while `_sibling_checks` pooled raw, + so byte-identical arms differed by 0.1 of recall on a check that gates `promoted`. Detecting + "these two call sites should share a preprocessing step" is a semantic claim about intent, + not a pattern; the realistic guard is a unit test per metric asserting invariance to + replicate imbalance, which is what was added here. — caught in the same review. +- [ ] A prose surface's claim about a NUMERIC CONSTANT in the code must be checked by reading the + constant. `SKILL.md` shipped "`failed_samples[]` is capped, so it will not hand you fifteen" + while `_FAILED_SAMPLE_LIMIT = 20` — the cap is *larger* than the number the sentence budgets + against, so the stated consequence was the reverse of the real one. CE064 cannot reach it: + that rule covers arithmetic-bearing TABLES, and this was a sentence naming a bare number. + A `ComputedClaim` could bind this one instance, but the general rule ("every number in these + surfaces that shadows a constant is derived from it") needs a way to know WHICH constant a + given number refers to, which is the part that is not cheap. — caught in the ReAPO + optimize-skill final review, by a fact-checker that ran the code rather than read it. +- [ ] The exempt-locator list is criterion-agnostic and is now read by a second consumer pointing + the other way. `LEAK_LOCATOR_FIELDS` omits `llm_judge.files` / `agent_judge.files`, + `cli_called.log` and `uipath_eval.eval_set`, all locators by the module's own definition. + For CE061 that is pre-existing scope; for `candidate_leaks` it is a NEW false-positive + channel in a checker whose whole design rationale is not firing more than it has to (a body + that names its own output path gets flagged). Not done here because widening the list also + weakens the shipped CE061 rule and changes its derived CLAUDE.md sentence — a separate + decision, not a refactor. The mechanical half is easy: derive the list from every criterion + field whose name matches a locator vocabulary, and fail when a criterion grows a + location-shaped field nobody classified. — caught in the same review. +- [x] **CLOSED.** The search loop's accept/revert arithmetic lived in a markdown snippet rather + than a tested function, against `models/optimize.py`'s own stated principle ("the gate's + verdict is a typed value the skill prints, instead of arithmetic an agent performs by hand"). + Now `optimize.search.search_compare` + `lineage_head_scores` + `render_search_comparison`, with + 18 unit tests: the four guards (shared-row intersection, no-overlap-before-holes, refuse on a + hole, corpus regression blocks an accept) are asserted rather than copied. The deferral + reasoning — that every snippet in this skill is hand-written the same way — was right about + the general case and wrong about this one: these four guards are the only ones whose omission + is *silent and score-changing*, which is what makes them worth the API. — caught in the ReAPO + optimize-skill Phase 1 quality review, closed the same day. +- [x] **CLOSED.** `model_copy(update={...})` is not validated by pydantic even under + `extra="forbid"`, so a mistyped key is set as a bare instance attribute and dropped from + `model_dump()` entirely — + no raise, no log, and the field it was meant to set stays at its default. This is the exact + hole CE041 closed for *construction*, still open for *update*, and it matters most where it + is worst: `ActivationGateVerdict.promoted` / `holm_alpha` and their execution-track twins are + written ONLY this way (`optimize.activation.holm_promote`, `holm_promote_execution`), so the two + fields that ARE the promotion decision are the two the runtime backstop does not cover. Not + done with CE041 because the fix is not a matching rule: `update=` legitimately takes a dict + in every one of this repo's ~9 call sites, so a rule that flags the CALL is wrong and one + that validates the KEYS has to resolve the receiver's model type, which an AST walk cannot do + in general. The tractable shapes are a runtime helper (`copy_with(model, **kwargs)` taking + literal keywords, plus a rule forbidding bare `model_copy(update=)`) or a + `model_validate(instance.model_dump() | update)` convention — a design decision, not a + bolt-on. — caught in the Plan A Phase 3 quality review; CE041's docstring points here. + **Resolution (Plan D Phase 3):** the helper, not the convention. `models/copy_with.py` + takes literal keywords and raises on an unknown field name; **CE048** forbids the bare call + shape in `src/`; all 21 call sites converted, with one documented exemption + (`criteria/agent_judge.py` — a dict VARIABLE from user YAML, plus `deep=True`; **that + exemption was retired in Plan A Phase 2** and `src/` now holds exactly one live + `model_copy(update=)`, the helper's own). The + re-validating convention was rejected: it re-runs every validator on every field on paths + that run once per candidate per round. Note the inventory here said "~9 call sites" and the + plan's single-line grep said 17 — the AST count is **22**, because four are wrapped across + lines, one of them `holm_promote`'s main promotion write. The reasoning above that "a rule + that flags the CALL is wrong" did not survive contact either: 21 of the 22 sites needed no + dict at all, so flagging the call shape and routing every author to a validating helper is + exactly what CE048 does, with one documented exemption. +- [x] **REFUTED, not built — a SHA-pinning rule over `templates/**/.github/workflows/`.** A review + raised it on the premise that *"users copy this template into their own repos"*, which would + make a floating action tag a supply-chain exposure. The premise is false, and the file's own + header says so: `templates/ci-outcome-fixture/.github/workflows/lint.yml` is a **graded eval + fixture** mounted into a sandbox for the `ci` skill's outcome suite — deliberately + "unrelated to evaluation", present only so `ci` does not take its no-`.github/` branch. + GitHub never runs it, no user copies it, and a floating tag therefore has no effect at all. + Worse, editing it perturbs the fixture the `ci` skill is *scored on*, so the "fix" would move + an A/B baseline. One file, zero exposure. Recorded here so the next review does not re-raise + it. — checked and refuted in the Plan D scoping pass, confirmed against the file at + implementation time. +- [ ] **`estimator_ledger.WATCHED_CONSTANTS` is one-directional — nothing forces a statistical + constant onto the list.** It shipped with the gap already LIVE, not merely prospective: the + final review found `optimize_execution.FLOOR_RESOLUTION` and `NEAR_FLOOR_MULTIPLE` unwatched in a + file four of whose constants were, and both move rendered output (the first decides whether + an MDE is measurable at all, and therefore whether the execution gate REFUSES). They are + watched now; the class is not closed. Both watch lists have anti-rename parity tests (a renamed constant + or a moved fixture directory would make the job match nothing and pass silently), but a + newly introduced resample count, alpha or tolerance is unwatched by default, and deleting a + tuple entry passes both `make test` and the job. CE064 solved exactly this rot for prose + tables with a COVERAGE check — a table no claim names is a failure — and the same shape + belongs here: "a module-level constant that looks statistical and is not watched is a + failure". Not built with the protocol because "looks statistical" needs a design pass (a + name heuristic? a `Final[float]` in two named modules? an explicit opt-out list?), and a + noisy version of this rule in a merge-blocking job is worse than none. — caught in the + Plan D Phase 6 quality review. +- [x] **`agent_judge` silently drops an off-kind judge config's fields.** `_build_agent_config` + copies the user's `criterion.agent` dump onto a `ClaudeCodeAgentConfig`, but + `AgentJudgeCriterion.agent` is the four-way `AgentConfig` union — so `type: antigravity` + supplies `thinking_level`, which `ClaudeCodeAgentConfig` does not declare. Verified: it lands + as a bare instance attribute, absent from `model_dump()`, and the same call writes + `type="antigravity"` onto a claude-code model unvalidated. This is exactly the key hole + `copy_with` closes, at the one site CE048 exempts — the exemption is legitimate (a dict + variable plus `deep=True`) but it is NOT harmless, and the comment there now says so instead + of claiming the hole cannot open. Not fixed with CE048 because the fix is a behaviour + decision — reject an off-kind judge config, or coerce it to the judge's own kind — rather + than a mechanical conversion, and either answer changes what a currently-accepted task YAML + does. — caught in the Plan D Phase 3 quality review. + **Resolution (Plan A Phase 2): REJECT.** `AgentJudgeCriterion.agent` is narrowed from the + four-way union to `ClaudeCodeAgentConfig`, so an off-kind block is a `ValidationError` at + task load rather than a silent coercion. Coercion was rejected because the offending field + is meaningless on the target model — there is nothing to coerce `thinking_level` INTO, and + `model: gemini-3` reaching the Claude SDK is not a config to repair. Blast radius measured + as zero: every in-tree `agent:` block under an `agent_judge` criterion already spells + `type: "claude-code"`, and all 46 task YAMLs load under the narrowed schema. Out-of-tree + YAML carrying a non-Claude judge block was already silently broken; it now fails loudly. + The overlay at the same site moved to + `ClaudeCodeAgentConfig.model_validate({**defaults.model_dump(), **user_overrides})`, + retiring the tree's last `# noqa: CE048`. +- [ ] A no-op "absence" assertion: `assert "X" not in text.replace("NOT X", "")` is vacuous whenever + the fixture cannot contain `X` at all, and reads as a strong guard. The named instance — + `tests/test_reports_optimize.py`'s `render_search_comparison` blocked-path test — **is FIXED** + (Plan D Phase 4: it now asserts `block.splitlines()[0] == "### Search round — CANNOT + COMPARE"`, and a sibling pins `DO NOT ACCEPT` as PRESENT on the input that produces it), so + do not go looking for a live example; the general RULE is what remains deferred. The correct form is to read the discriminating + LINE — `_headline()` in that file is the worked example, and it proves itself non-vacuous by + rendering a fixture where the forbidden string DOES appear. Not done here because catching it + mechanically means an AST rule over `tests/` matching a `Compare(NotIn)` whose right operand is + a `.replace()` call whose first argument CONTAINS the left operand, plus a repo-wide sweep of + the hits before it can land green. — caught in the Plan A Phase 1 review. +- [ ] An `if` whose body is only `notes.append(...)` sitting beside a structurally identical `if` + that ends in `return` — the shape of the incumbent-variant fall-through this plan fixed, where + one validation branch failed open while its sibling three lines above failed closed. AST- + detectable within a single function body (same-parent `If` nodes, one terminating in `Return`, + one not, both guarding a comparable predicate). Not done with the fix because landing it green + needs a sweep of every multi-branch validator in `src/` to separate the real fall-throughs + from the deliberate accumulate-then-continue ones — the same reason the + OSError-in-a-`try` rule was deferred. (That entry once reserved the number "CE046"; Plan D + Phase 2 spent CE046 on the CLI-flag documentation rule, so the number here would now point at + a shipped, unrelated rule. Numbers reserved in this file are enforced nowhere — + `runner.py`'s uniqueness assert covers `ALL_RULES` only.) — caught in the Plan A Phase 2 + review. +- [ ] A duplicated long prose literal (≥60 chars) across two functions. Phase 3 of the optimize-gate + module split collapsed four such strings — the notes both Holm wrappers emit, which sat 600 + lines apart as byte-identical copies, 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 interim guard is + `test_neither_wrapper_respells_a_shared_note` in `tests/test_optimize_layering.py`, which pins + those four strings only. The general rule is **Plan D's proposed CE049** and is deliberately + not built here: unlike CE042 (a one-allowed-site seam rule copied wholesale from CE040), this + is a heuristic whole-tree rule needing its own design pass — a length threshold, a + normalisation for source wrapping, and an allowlist sweep before it can land green. Recorded + here because Plan D lives in an untracked planning file and this is the committed surface. — + caught in the optimize-gate module-split run, Phase 3. **A second real instance, Plan D + Phase 4:** `early_stop.py` stated the ToolStart decidable-seam claim in BOTH its module + docstring and `_on_event_impl`'s, so correcting one left the other contradicting it — and the + module copy survived the grep that found the method copy only because it wrapped mid-phrase. + A length-and-wrapping-tolerant rule would have caught exactly that. +- [ ] A raw-substring prose sensor firing on its own documentation. `test_module_imports_no_cli_machinery` + scans module source for banned tokens (`import typer`, `coder_eval.cli`, …); the new + `reports_optimize.py` tripped it by *documenting* that it imports no such module. The instance + was fixed by rewording (and saying why in the docstring), but the class is live for every + substring-scanning sensor in `tests/test_custom_lint.py` — the same fragility CE064 exists to + discourage for arithmetic claims. A real guard means parsing rather than scanning: check + `ast.Import`/`ast.ImportFrom` nodes instead of text, which is a sweep of every such sensor and + a decision about the ones that legitimately scan prose. — caught in the optimize-gate + module-split run, Phase 6. + +- [ ] **`fingerprint_diff` cannot see a config key that MOVED, only one that changed value.** + `compute_run_fingerprint` dumps the whole `BatchRunConfig`, and `fingerprint_diff` compares + only keys present in BOTH stamps — so when Phase 2 collapsed the three flat selector fields + into one nested `row_selection`, a `--resume` into a run dir stamped before the change + silently skipped the config-drift warning instead of reporting it. Verified: a prior stamp + with `"split": "train"` against a current stamp with `row_selection: {"split": "test"}` + yields `{}`. Not a correctness break (resume matches on `id_field`-derived row ids, and the + warning is informational) and it self-heals after one run, so it was not worth blocking on. + A real guard is a rule that flags a key present in exactly one of the two fingerprint + schemas — which needs a notion of the PREVIOUS schema that nothing in the tree currently + carries, so it is a design question rather than a test. Note also that nothing pins that a + changed `row_selection` surfaces in the diff at all. — caught in the row-selection + integrity run, Phase 2 review. + +- [ ] **The cross-split gate refusal compares `--split` only, not the samplers.** `run.json` + records `max_rows` and `sample_per_stratum` beside `split`, and `read_split_provenance` + reads none of them. A `--sample` draw is fixed-seed, so two arms run at DIFFERENT counts + score largely disjoint rows (`random.sample` with a different `k` is not a prefix) and the + preflight passes silently — the same failure the refusal exists for, one field over. It is + narrower than the split case: a sampler mismatch surfaces downstream as a small + `rows_paired` beside a large `rows_excluded`, which the verdict already reports, whereas a + split mismatch can leave both arms fully paired on rows that merely share ids. Widening the + comparison is a behaviour change beyond what the preflight was scoped to, and it needs a + decision about whether an intentional `--sample` difference should ever be gateable at all. + The message and a comment beside the check now state the scope so it is at least not a + false claim. — caught in the row-selection integrity run, final cross-phase review. + +- [ ] **Nothing PREVENTS a run dir from accumulating rows across invocations — the gate only + refuses afterwards.** `coder-eval run --run-dir ` writes into the tree without + noticing that a previous invocation's `//task.json` are still there, and rewrites + `run.json`'s `row_selection` to describe only the current call. `activation_gate` now + reconciles the tree against `run.json` and refuses (`reconcile_tree_against_run_json`), but + that is detection at the far end: the user has already paid for both runs, and a run dir + *already* contaminated before the check landed stays unusable rather than being repaired. + The prevention half is a behaviour change to the primary entry point, which is why it was + left out: either `run` refuses a non-empty `--run-dir` unless resuming, or every `task.json` + stamps the `--split` (and sampler) that produced it, so a row carries its own provenance and + no reconciliation against a per-invocation artifact is needed at all. The second is the + better shape and the larger change — `EvaluationResult` would gain a field, and every + reader of a run dir could then answer "which selection produced this row?" directly. + — caught in the top-10 review-fixes run, Phase 4. + +- [ ] **The eight CE041 splat sites are exempted, not converted.** CE041 now fires (it never had + — it reported 0 against 8 real model-constructor splats until `resolved_module` landed), and + all eight carry a reasoned `# noqa: CE041` rather than the `Model.model_validate(payload)` + the rule's message asks for. That was a deliberate scope call: converting them changes the + raised exception type on YAML-parsing paths, so each call site's `except` clause and + user-facing message has to be traced — `task_loader.py:85` in particular is wrapped in a + handler producing `ValueError: Invalid task definition: ...` that other code and tests + depend on. What makes the exemptions honest rather than a dodge, and what a converter must + re-check: `ExperimentDefinition`, `SimulationConfig`, `RunLimits`, `SandboxConfig` and the + agent configs all declare `extra="forbid"`, so a mistyped key RAISES there today rather than + landing at a default; `TaskDefinition` is the one that does not (its top-level schema is in + soft launch) but it emits `UnknownTaskFieldWarning` via `_warn_on_unknown_fields`, which + `coder-eval plan` renders inline. So no site is silently wrong — the noqas give up the + STATIC half of the guard only. Sites: `cli/evaluate_command.py`, `orchestration/config_merge.py` + (x3), `orchestration/experiment.py` (x2), `orchestration/task_loader.py` (x2). + — caught in the top-10 review-fixes run, Phase 6. + +- [ ] **Ratchet ruff `C90` down from 30 toward 20, one function at a time.** `C90` is now enabled + at `max-complexity = 30` — one above the worst function in the tree — so it costs no + refactor and still fails a NEW god-function. It is a floor under the debt, not a fix for it. + Four functions sit above 20 and each needs its own decomposition and review: + `isolation/docker_runner.py::_build_argv` (29), `reports_experiment.py::generate_variant_report` + (24), `orchestrator.py::_simulation_dialog_loop` (22), + `agents/claude_code_agent.py::communicate` (21). Lower the ceiling by one step per landed + refactor; do NOT add a `per-file-ignores` entry instead — a second entry in that list means + the ceiling is wrong, not that a file is special. Note the plan that introduced `C90` assumed + `optimize_gate.py` would need the single exemption. It does not — but NOT because the module + split landed: that is still undone (3250 lines, `radon cc` reports one E-grade and seven + D-grade functions). Its mccabe peak is 17 (`execution_gate`), unchanged since `fa69cdf`, so + mccabe and radon disagree about that file and only the ceiling of 30 is why no exemption + exists. The module split is still owed. + — caught in the top-10 review-fixes run, Phase 7. + +- [x] **CE050, CE051 and CE052 are TAKEN** by the top-10 review-fixes run: CE050 (escape untrusted text in a Rich-markup `console.print` under `cli/`), CE051 (a lint rule matching an import's module string must route through `tests/lint/import_resolution.py::resolved_module`), CE052 (every task YAML under `templates/` must load through the real `load_task`). CE049 remains reserved by the backlog entry above. The next free id is **CE053** — but re-run `grep -rhoE "CE0[0-9][0-9]" tests/ .claude/ src/ docs/ pyproject.toml | sort -u` before claiming it: `tests/lint/runner.py`'s uniqueness assert covers `ALL_RULES` only, so a class-wired id (CE052 is one) can collide with a `BaseRule`'s without failing anything. + +- [ ] **CE025 pins that a live criterion DECLARES its polarities, not that its checker AGREES + with the declaration.** `TestCE025LiveVerdictConsistency` asserts a `LiveSuccessCriterion` + model has a checker overriding `live_verdict` and vice versa; nothing checks that a verdict + the checker actually RETURNS is one `live_decidable_polarities()` declared. A checker + returning `"fail"` while its model declares `{"pass"}` produces a verdict that + `EarlyStopWatcher._evaluate_impl` classifies as neither a native fail (needs + `_fail_trigger[i]`, which is False) nor a budget fail (needs `_budget_expired[i]`) — so a + definitively-failed armed criterion whose ceiling is below threshold fires **no stop at + all**. Reproduced during the Phase 1 review. Pre-existing, not a regression: the + `_budget_drove` arm deleted in Phase 1 never rescued the latched case either, and the + unlatched case it did rescue is unreachable (an armed budget and a declarable live-fail are + mutually exclusive). Recorded in `_budget_drove`'s docstring rather than papered over. + NOT cheap: agreement is a BEHAVIOURAL property — you have to exercise each checker over + inputs and compare the returned polarity against the declaration — so it is a property test + over the criterion registry, not an AST rule, and it needs a decision about what input + corpus is representative. — caught in the top-10 review-fixes run, Phase 1 review. + +- [ ] **Nothing requires a new CExxx rule to ship with a test proving it FIRES.** Three guards + written during this run were themselves broken in the fail-open direction and only found by + review: CE051's docstring exclusion compared `ast.get_docstring`'s *normalised* text against + raw `Constant.value` (so it excluded nothing), CE051's GAP violation was anchored on the + Module node (line 0, which no `# noqa` can ever suppress), and `resolved_module` FABRICATED + module paths for files outside `src/coder_eval` instead of returning `None`. All three + looked green. The existing convention — a positive fixture per rule — is real but + unenforced, and a rule shipped with only negative fixtures is indistinguishable from a + working one. A mechanical version ("every `ALL_RULES` member is constructed somewhere in + `tests/test_custom_lint.py`") is cheap but would not have caught any of the three, since all + were constructed. The version that WOULD catch them — "at least one test per rule asserts a + NON-EMPTY violation list" — needs a reliable way to tie a test function to the rule it + exercises and to recognise a non-emptiness assertion, which is heuristic enough to deserve + its own design pass. — caught in the top-10 review-fixes run, Phase 6 review. + +- [x] **The Stage A / floor surfaces read the run-dir tree without reconciling it.** + `activation_gate` and `execution_gate` both refuse a run dir holding results its own + `run.json` never wrote (`reconcile_tree_against_run_json`). `measure_noise_floor`, + `noise_floor_mde`, `arm_row_scores` and `cost_quality_points` do not — measured, on + contaminated dirs that `activation_gate` correctly refuses, `measure_noise_floor` returned a + floor computed over an extra pooled row and `arm_row_scores` returned the stale row in its + vector. They are not gates, which is why this was left: each returns a float or a list with + nowhere to put a refusal, so closing it needs a decision about the return contract (log and + degrade? an optional strict flag? a `Reconciliation` alongside the value?) rather than a + copy of the preflight. The floors feed the MDE a gate reports and the vectors feed the three + Pareto fronts, so a wrong number here is not cosmetic. — caught in the top-10 review-fixes + run, final adversarial review. + + **Resolution (Plan B+C Phase 1):** closed, and the return-contract question resolved by + SPLITTING on it rather than picking one answer. The DETECTION is shared — + `optimize.load.reconcile_arms` is the one sweep every whole-arm reader routes through + (`execution_gate` still calls `reconcile_tree_against_run_json` directly — it works one run + dir per variant and needs the per-dir result), and `stale_tree_reason` is the one message + the readers share — while the RESPONSE follows the return type: + `measure_noise_floor` and `measure_execution_noise_floor` return `None` through the existing + `no_floor` channel, and `arm_row_scores` logs a WARNING and returns its vector, because + `ArmRowScores` has no field a refusal could live in. A shared `_refuse_or_warn` helper was + considered and rejected — it would take a mode flag, which is two functions in a trench coat. + Three readers deliberately do NOT reconcile and carry a reasoned `# noqa: CE053`. Two name who + reconciles for them: `load_and_pair` (its only caller `activation_gate` sweeps both arms + first) and `cost_quality_points` (it reaches the tree through `arm_row_scores`, so a second + sweep would read every `run.json` twice per arm and warn twice about one fault). The third, + `resolve_arm_model`, argues from its RETURN VALUE instead: contamination can only flip a model + id to `None`, which bars the cache rather than borrowing another model's floor, and every + consumer's own `floor_preflight` refuses the tree first. The + correction the entry itself needed: the four readers are `measure_noise_floor`, + **`measure_execution_noise_floor`**, `arm_row_scores` and `cost_quality_points` — + `noise_floor_mde` reaches the tree only through `measure_noise_floor`. **CE053** is the + standing guard, and its path scope is the `optimize/` DIRECTORY rather than a list of module + names (it was the `optimize_*` filename prefix until the family became a package), so moving a + reader — or adding a module — cannot take it out of reach. + +- [ ] **A dotted `coder_eval..` reference in PROSE is unchecked.** The snippet + sensor (`tests/lint_tests/shared.py::_snippet_binding_failures`) resolves imports inside + ` ```python ` fences only. The six-module split (Plan B+C Phase 7) left **fourteen** files + naming a moved symbol by its old dotted path — including + `plugins/coder-eval/reference/proposal-prompt.md`, which told the proposer to call + `coder_eval.optimize_gate.candidate_leaks(...)` (now `optimize.search`, so the import + raises), and two Pydantic FIELD DESCRIPTIONS, which are public model documentation. All + were found by a reviewer reading files, not by any sensor. The guard is a scan for + `coder_eval\.[\w.]+\.[A-Za-z_]\w*` across `src/`, `docs/`, `plugins/` and `.claude/`, + resolving each through `importlib` — the same existence check the fence sensor already + does, one syntax over. Not built here because the reference forms vary (`:func:` roles, + backticked prose, bare dotted paths) and deciding which are claims about the API versus + incidental mentions needs a design pass. — caught in the Plan B+C final cross-phase review. + +- [ ] **A contaminated `ArmRowScores` vector is persisted with no marker.** `arm_row_scores` warns + to stderr and returns its vector (Plan B+C Phase 1 — `ArmRowScores` has nowhere to put a + refusal). `record_round_scores` then writes that vector into `measurements.json`, and + `lineage_head_scores` reads it back rounds later, when the run dirs may be gone and the + warning is long out of scrollback. Nothing marks the stored vector and `render_row_matrix` + prints no footnote, so the only trace of contamination is a stderr line from the snippet that + produced it. The plan's B1 table asserts "no field can carry a refusal"; that is true only + because no field was added — a defaulted `stale: bool = False` on `ArmRowScores` is additive + under `extra="forbid"`. Not done here because it ripples into `RoundScores`, + `record_round_scores` and the matrix footnotes, which is a scoped change rather than a guard. + — caught in the Plan B+C Phase 1 quality review. + +- [ ] **CE055: a module-size / complexity ratchet over the optimize family.** The six-module split + landed (Plan B+C Phase 7), and this is the baseline it would ratchet against, measured at + that commit: + + | module | lines | D-grade functions | + |---|---|---| + | `optimize_load.py` | 713 | `_load_and_pair` D(30) | + | `optimize_gate.py` | 374 | `cost_latency_guardrails` D(24) | + | `optimize_activation.py` | 1159 | `_sibling_checks` D(23), `activation_gate` D(23) | + | `optimize_execution.py` | 1033 | `execution_gate` D(30), `_execution_diagnostics` D(29), `holm_promote_execution` D(21) | + | `optimize_fronts.py` | 327 | none | + | `optimize_search.py` | 286 | none | + + **No E-grade function anywhere in the family**, down from one file of 3,664 lines with one + E-grade and seven D-grades. Not built here because a ratchet needs a checked-in baseline + (this table) plus a design pass on what cap a NEW function gets — the existing `C90` + ceiling is a mccabe number and radon disagrees with it (mccabe peaks at 17 on + `execution_gate` where radon reports D(30)), so a ratchet has to pick one metric and say + why. Deferred to Plan D, which is where the decision belongs. + +- [ ] **The execution track's "the floor came back unavailable" advisory names no cause.** + `activation_gate` now threads the real reason out of `measure_noise_floor` through a + `reasons` sink, so its MDE note says WHICH of five causes fired instead of naming one + unconditionally (Plan B+C Phase 3). `_execution_diagnostics`' twin advisory + ("this suite's minimum detectable effect came back unavailable / 0.000") still names none, + because `measure_execution_noise_floor` does not thread the sink — the parameter was added + and then removed, since nothing called it and speculative surface is worse than a recorded + gap. Closing it is the same shape as the activation side: forward `reasons` from + `measure_execution_noise_floor`, collect it in `execution_gate`, and pass it into + `_execution_diagnostics` beside `mde`. Not done there because that advisory's text is pinned + in `optimize_verdicts/execution_gate.json`, so it needs a fixture regeneration and an + estimator-ledger row of its own. — caught in the Plan B+C Phase 3 quality review. + +## From 2026-08-16 xlsx execution-track dogfood run + +First real `/coder-eval:optimize-skill` execution round against a THIRD-PARTY skill +(Anthropic's `xlsx`, 24-row outcome suite). Working tree: `tmp/xlsx-opt/`, plan and run +log in `c/2026-08-16-optimize-public-skill-blog.md`. Findings 1, 2 and 4 are defects in +**shipped guidance**, not in the experiment. + +- [ ] **The engagement criterion halves every effect the execution gate measures.** The + bundled `reference/templates/outcome.yaml` stacks `skill_triggered` on every row at the + default `weight: 1.0`. That criterion scores exactly 1.0 on every row *by design* — it is a + gate, and the same skill requires `recall.yes: 1.0`. `weighted_score` averages it with the + real grader, so a measured 0.16 difference reaches `execution_gate`'s paired *t* as 0.08. + Nothing in `optimize-skill` or the template mentions it, and the template's own recommended + shape is what causes it. Measured on the xlsx suite: a row grading 0.857 reports 0.929. + Fix is guidance, not code: the template should set a near-zero weight and say why. +- [ ] **...and the obvious fix is rejected by a validator, so two shipped instructions + conflict.** `weight: 0` raises "weight=0 makes the criterion informational (non-gating), so + it cannot also set suite_thresholds". The template tells you to gate on `recall.yes` AND the + method tells you the gate compares `weighted_score` — you cannot satisfy both cleanly. + Worked around with `weight: 0.05` (~5% shrinkage rather than ~50%). Decide which of the two + gives: either the validator learns that a zero-weight criterion may still carry + `suite_thresholds`, or the template stops stacking a constant-scoring criterion into the + compared statistic and documents the near-zero weight instead. +- [x] **DONE (2026-08-17, outcome-suite-mode plan Phase 4): No preflight on whether the INSTRUMENT + is fair.** Shipped as `/coder-eval:task` **Step 6.5** — build a known-good and a known-bad + artifact, grade both, report the SEPARATION MARGIN, before any stage is paid for — with the three + fairness questions consolidated into `reference/task-rubric.md` § "Grader fairness" (one + declaration; `task` and `optimize-skill` both point at it) and guarded by + `test_task_skill_has_discrimination_gate` / `test_grader_fairness_is_declared_once`. The original + entry follows. `optimize-skill` hard-gates + engagement but has nothing on "is the grader discriminating and unbiased". Two checks in + this run's grader were wrong in ways only the baseline could reveal: one penalised a + legitimate alternative implementation (nested `IF` where the body never mandates `IFS`), one + double-charged a single mistake (a workbook with no formulas failed both "use formulas" and + "recalculate"). **Both biased every arm equally**, so no cross-arm comparison could ever have + surfaced them — the same blind spot `skill-creator`'s analyzer calls a "non-discriminating + assertion". Candidate: a Step 6.5 that requires grading a known-good and known-bad artifact + and asserting the scores separate, before any stage is paid for. +- [ ] **Answer-key leakage into the fixture is unguarded, and measurably inflates scores.** + `candidate_leaks` checks whether a CANDIDATE reproduces train-row text; nothing checks + whether the FIXTURE ships the grader's expectations into the sandbox. Reproduced + accidentally here (a row generator wrote `expectations/*.json` under the fixture root, which + is copied into every sandbox). Measured against a clean run on the same 11 rows: + **mean 0.9158 clean -> 0.9461 leaked**, two rows flipping partial->perfect. That is larger + than most effects the gate exists to detect. Candidate: a lint/preflight that fails when a + `run_command` criterion's script or data lives under a `template_dir` the sandbox mounts. + **RESERVED AS CE056 (see the entry at the end of this file), and partially closed:** the shipped + layout is asserted by `TestPluginArtifacts::test_outcome_grader_lives_outside_any_mounted_fixture` + and the grader template now addresses its script through `$TASK_DIR`, beside the suite rather than + inside the fixture. What remains un-guarded — and what CE056's promotion trigger waits for — is + the general tree-walking rule, which today has one discoverable subject and would pass vacuously. +- [ ] **`--sample 1` is the missing cheap preflight.** One row for ~$0.50 proved the whole + pipeline (skill engages, artifact lands, grader scores) before any stage. `optimize-skill` + Step 6 goes straight to a full baseline. Pure guidance fix, one sentence. +- [ ] **`plan --split` is a capability the version string does not carry.** The PATH binary and + the tree-local editable install BOTH report `0.9.6`; only the second accepts `plan --split`. + Step 1 already warns about this in prose and it still cost a cycle — the check it describes + should be a copy-pasteable command in the skill rather than a paragraph. + +## From Plan A (scoring-correctness fixes, 2026-08-16) + +- [ ] **`agent_judge`'s `allowed_tools` and `ignore_patterns` are order-nondeterministic across + processes.** `criteria/agent_judge.py` builds both through a set literal + (`list({*config.allowed_tools, SUBMIT_VERDICT_MCP_TOOL_NAME})`, and the same shape for the + ignore-patterns floor), and Python's string hash randomization reorders a set per interpreter + run. Measured three consecutive runs of the same config: `['mcp__…', 'Glob', 'Bash', 'Grep', + 'Read']`, `['Glob', 'Grep', 'mcp__…', 'Bash', 'Read']`, `['Glob', 'Bash', 'Grep', 'mcp__…', + 'Read']`. Behaviourally harmless — both are membership sets downstream, and neither affects + cost or scoring — but it has two real costs: **any test asserting LIST equality on either is + flaky in CI** (Plan A's own prescribed test would have been, and now compares as a `set`), and + the persisted `agent_config` dump in `task.json` differs run-to-run for byte-identical config, + which is noise in any artifact diff. The fix is one `sorted(...)` per line. Not taken in Plan A + because it changes the bytes of a persisted artifact, which is a different decision from a + scoring fix and does not belong in the same commit. A sensor is also available and is the + cheaper half: a test asserting no test in the tree compares `allowed_tools`/`ignore_patterns` + by list equality. — found by the Plan A spike (S6), recorded rather than smuggled in. +- [ ] **A `result_status` membership DENYLIST has no guard — CE018's shape, one field over.** + `CommandTelemetry.result_status` classification is the `FinalStatus.name` problem in a + different field, and CE018 already forbids the denylist form there. Not built alongside + **CE054** (which confines the comparison to one site per criterion module, and IS built) + because the two catch different things and only one of them was the bug: the drift that + produced the false positive was between an ALLOWLIST (`!= "success"`) and a DENYLIST + (`in ("error", None)`), so a denylist rule catches one of the two sites and the seam rule + catches the pair. Independently, `result_status` is a closed + `Literal["success","error","unknown"] | None`, so CE018's actual motivating failure — a new + enum member silently falling through a stale denylist — requires a model edit a reviewer + sees, which is a much weaker case than CE018's own. It would nonetheless be cheap and + NON-NOISY: after Phase 1 there is no `result_status` membership test left anywhere in `src/`, + and the adjacent shapes are safe by construction (`command_executed.py` and + `reports_html.py` use `!= "success"` / `== "success"`, `analysis.py` uses `==`/`is None` + chains to bucket report counts). The rule must match the `in` / `not in` form ONLY — a + broader "any comparison against a non-`success` literal" version fires on `analysis.py` x3 + and `codex_agent.py` and should not be built. — raised by the Plan A final review + (multi-model + Opus, both independently). + +- [ ] **CE056 (RESERVED): a grader's answer key must never ship inside a mounted fixture.** + (The same rule as the "Answer-key leakage into the fixture is unguarded" entry above, which + states the measurement in its original context; this entry is the ID reservation and the + promotion trigger. Do not treat them as two candidates.) + Everything under a `template_dir` is copied into every sandbox, so a `run_command` grader's + `expectations/` placed there hands the agent exactly what it is being marked against — and + the run looks completely normal. Measured on a real suite when it happened by accident: on + the same 11 rows the mean went **0.9158 clean -> 0.9461 leaked**, two rows flipping from + partial to perfect. That is larger than most effects an optimization round exists to detect, + and it inflates **every arm**, so no cross-arm comparison can reveal it. + `optimize.search.candidate_leaks` does not cover this — it asks whether a CANDIDATE + reproduces train-row text, not whether the FIXTURE ships the marking scheme. + + **Not built now, deliberately: it would pass vacuously.** The only discoverable subject in + the tree is the bundled `reference/templates/outcome.yaml`, whose mounted + `./outcome-fixture` is a one-file placeholder — so a tree-walking rule would report clean + whether or not it worked, which is the exact CE044/CE045 failure. The assertion exists + instead as `TestPluginArtifacts::test_outcome_grader_lives_outside_any_mounted_fixture`, + scoped to that one file by construction and carrying a non-empty-mount-set GAP check. + + **Promotion trigger:** a SECOND outcome suite with a `run_command` grader appears (in + `tasks/`, `templates/`, or the plugin). Fold in the sibling rule rejected for the same + one-subject reason at the same time — **a grader criterion must set + `score_from_stdout: true`**, since a binary grader over a dozen-odd rows manufactures the + execution gate's zero-variance refusal (today's single subject is pinned by + `test_outcome_template_grader_slot_is_continuous`). — caught in the + 2026-08-17 outcome-suite-mode plan, Phase 5. + +- [ ] **Nothing binds a shipped surface's PROSE cross-reference to the step it names.** + `test_bundled_plugin_root_references_resolve` checks `${CLAUDE_PLUGIN_ROOT}` FILE paths in + `.md` files only — it cannot see "see `/coder-eval:task` step 6.5", and it does not scan `.py` + or `.json` surfaces at all. Four shipped files cited step 6.5 while it did not yet exist, and + an author following the pointer would have found nothing at the one moment the safeguard + matters. Closed for THIS reference by `test_shipped_surfaces_cite_a_step_that_exists`, which + is bespoke: it hardcodes the four citing files and the string "step 6.5". The general rule — + extract ` step N.N` / `§ ` references from every bundled surface and assert + the target exists — is a Markdown-structure walk over headings, which is why it is deferred + rather than written here. — caught in the 2026-08-17 outcome-suite-mode plan, Phase 3 review. + +- [ ] **Nothing binds a CLI command's DESCRIPTION to what it does.** CE046 pins that every visible + long flag appears in `docs/USER_GUIDE.md`, but nothing checks the prose around it. `plan` + gained filesystem validation while the guide still called it "task syntax, required CLI + tools, API keys, and schema validity" — and `check-skill/SKILL.md` has said "`plan` is a + schema check only — it does not read the dataset file" since before the dataset preview + landed, which was already false. Both were fixed by hand, twice, by reading. A rule would + have to compare a docstring's claims with a command's behaviour, which is not mechanically + decidable; the tractable subset is a sensor per claim, on the CE064 `ComputedClaim` model. + — caught in the 2026-08-17 outcome-suite-mode plan, Phase 1 and final reviews. + +- [ ] **Nothing checks that a CE064 `ComputedClaim` still fails on a MUTATED table.** The + `headroom-ceiling` claim shipped able to pass over a table trimmed to a single row: every + remaining cell recomputed correctly, so the check returned `[]` while the table said the + opposite of what the claim exists to assert (deleting the one non-gap rule leaves "three of + four were unpromotable" describing three rows that are all gaps). Fixed for that claim by + asserting the rule set against the fixture and deriving the headline count from the cells — + both bespoke. The general rule is a MUTATION test over the registry: for each claim, perturb + each covered table (drop a body row, bump a numeric cell) and assert the check now fails. + Deferred rather than written because the perturbation has to be claim-shaped to be fair — a + dropped row is caught by the cost table's exact-label lookup and by the sizing table's + per-row recompute, but the halving table iterates whatever rows it finds, so a naive + row-drop mutation would report a gap in a pre-existing claim rather than in the harness, and + deciding whether that gap is real is the actual work. `covers` already gives the rule its + table set, so the registry half is free. — caught in the 2026-08-17 outcome-suite + measurement-quality plan, Phase 3 review. + +- [ ] **Nothing runs a shipped SKILL.md snippet, so a track-specific one can crash on the other + track.** `_snippet_binding_failures` binds keyword arguments against the real signatures and + the import sensor asserts every name exists — but neither executes anything, so Step 11's + ledger snippet shipped calling `subprocess.run` on a grader the ACTIVATION track has no such + thing as, in a file whose own prose calls the snippet a runnable continuation. Fixed by hand + (the fingerprint half is commented out per track, as the floor already was). A real guard + would execute each fence against a fixture run directory per track, which needs a fixture + builder per snippet and a way to neutralise the paid calls — well past the promote threshold. + The cheap subset, "a fence that names one track must not call anything unconditionally", is + a heuristic that would fire on every correct block too. — caught in the 2026-08-17 + outcome-suite measurement-quality plan, Phase 5 review. + +- [ ] **A CLI mode that bypasses a protocol can still fall through that protocol's error handler.** + `verify.py --fingerprint` prints only a hash, but an exception inside it reached the + always-exit-0 guard that exists to protect a computed SCORE — printing `0.0000\ngrader + failed: …`, which `subprocess.run(check=True)` cannot catch and a caller records AS the + fingerprint, so every later round reports a changed instrument from a permissions error. + Fixed and regression-tested (`test_a_fingerprint_that_cannot_be_computed_exits_non_zero`). + The general rule — every declared output mode has its own failure protocol, asserted — needs + a machine-readable declaration of the modes, which this scaffold does not have and should not + grow for one rule. — caught in the 2026-08-17 outcome-suite measurement-quality plan, Phase 5 + review. + +- [ ] **A python fence in a shipped SKILL.md must be executable as written, not merely + import-resolvable.** Step 11's ledger snippet shipped with `suite = suite_fingerprint(suite_task, …)` + whose only assignment of `suite_task` was inside a COMMENT — a `NameError` in the user's + terminal after the round is already paid for. The existing snippet sensor asserts every + imported NAME resolves, which is satisfied here, and the prose-token sensors assert strings + are present; neither can see an undefined local. The cheap version — `ast.parse` each fence + and check every load is bound by an earlier store, an import, or a builtin — is defeated by + the placeholders these snippets legitimately carry (``, ``, + `gate_dirs` carried from an earlier step), so it needs a per-fence declaration of what the + step inherits before it can tell a placeholder from a bug. That declaration does not exist + and should not be grown for one rule. Note this is the SECOND time a fence defect shipped + past every sensor (see the entry above about executing fences per track), so the pair is + the argument for building it properly rather than cheaply. — caught in the 2026-08-17 + optimize measurement-unit / confirm-gate plan, Phase 3 review. + +- [ ] **A moved WATCHED constant leaves `docs/REPORT_SCHEMA.md`'s boundary paragraph attributing it + to the old module, with every assertion green.** `FLOOR_RESOLUTION` moved from + `optimize_execution` to `optimize_gate`; `WATCHED_CONSTANTS` was updated and both anti-rename + parity tests passed, because `test_the_documented_watch_list_matches_the_code` matches the + constant NAME only. A module check was written and then REMOVED for being unfailable: the + section legitimately names an old module inside a ledger row describing the move, and every + watched module is named somewhere in the section anyway, so neither a subset rule nor a + proximity rule distinguishes a correct mention from a stale one. Doing it properly means + parsing the boundary paragraph's `'s / ` structure — a one-paragraph grammar, + which is more machinery than the defect (a stale attribution in prose) justifies today. The + limitation is stated in the test itself so the next reader is not misled. — caught in the + 2026-08-17 optimize measurement-unit / confirm-gate plan, Phase 6 review. + +- [ ] **A public function returning a PRIVATE `NamedTuple` is a cross-module signature CE059 cannot + see.** CE059 matches an imported NAME, so it fires on `from .load import _PairedRows` and is + structurally blind to the same type reached as a return value. `optimize/load.py`'s public + `load_and_pair` returns `_PairedRows`, whose fields `optimize/activation.py` reads at eleven + sites, and `optimize/gate.py`'s public `holm_family` returns `_HolmFamily`, destructured in + both track modules. Neither is IMPORTED by a sibling, which is why the package-internal + convention left both private — but a sibling that wanted to annotate either could not without + tripping the rule, and the underscore tells a reader "safe to change this signature" about a + field set four modules depend on, which is the exact error CE059's docstring calls the + expensive direction. Not cheap to guard: the honest predicate is "a public function's return + ANNOTATION names a private type defined in this package", which means resolving annotations + (string-ified under `from __future__ import annotations`) rather than matching import lines — + a different kind of check from every rule under `tests/lint/rules/`, and it would want a + companion pass over parameter annotations to be worth having. Renaming the two types is the + one-line alternative and was deliberately NOT taken mid-phase, since the plan had verified and + recorded both as single-module. — caught in the 2026-08-18 optimize subsystem architecture + plan, Phase 3 quality review. + +- [ ] **The `optimize-skill` snippet binder is structurally blind to an ATTRIBUTE read, which is 13 + of the skill's live bindings.** `tests/lint_tests/shared.py::_snippet_binding_failures` resolves + the skill's `import` lines and binds keyword arguments, so a renamed FUNCTION or PARAMETER is + caught. It never resolves an attribute read off a returned object, and the skill has thirteen: + `floor.mde`, `attribution.failed` / `.unattributed`, `arm.row_scores` / `.variant_id`, + `measurements.regression_corpus` / `.round_scores`, `confirm.test_verdict`, + `promoted_verdict.candidate_variant`, `r.success_criteria_results[i].score`. Proven both + directions: renaming `NoiseFloor.mde` -> `mde_value` in `src/` leaves all 15 binder tests + GREEN, and the in-tree tests that do go red point at `src/`, so the repair path never reaches + `SKILL.md`. This is the seam that matters most in this subsystem, because it has ZERO in-tree + callers by design — the binder is the only thing between a rename and a skill that fails in the + user's terminal after they have paid for the runs. Not cheap: doing it properly means resolving + each snippet expression's TYPE (the return annotation of the function it came from, then the + field on that model) rather than matching a name, which is a small type resolver, not a regex. + — caught in the 2026-08-18 optimize subsystem architecture plan, final cross-phase review. + +- [ ] **A commented-out snippet in a skill is import-sensed but not signature-sensed.** The activation + track's whole Stage C call (`plugins/coder-eval/skills/optimize-skill/SKILL.md:1841-1852`) is a + commented block. The binder's raw-text import regex matches `# from coder_eval... import + confirm_gate`, so the NAME is checked, but `ast.parse` discards comments, so its seven keyword + arguments are never bound — mutating one reports zero failures. It is the one procedure step + with no signature sensor. Two possible fixes and neither is a one-liner: uncomment the block + (a change to the shipped skill's content, needing its own review of whether the step should be + live), or teach the binder to strip a leading `# ` from a fenced block's lines before parsing, + which risks parsing genuine prose comments as code. — caught in the same review. + +- [ ] **A test-file split can silently drop a module-level statement that defines no NAME.** Measured: + splitting `tests/test_optimize_gate.py` into eight files lost exactly two statements — both bare + module-scope `assert`s guarding the optimize rank ladder's coverage. Nothing went red, because + an assert is not a collected test and the node-id parity check that guarded the split (803 ids, + identical modulo the file component) cannot see a statement that collects nothing. A specific + sensor now exists (`TestTheLayeringCoverageAssertRunsAtCollection`), but the general rule — "a + refactor must preserve every top-level statement, not every top-level NAME" — has no guard. Not + cheap as a lint rule: it is a property of a DIFF across a file boundary, not of one file's AST, + so it belongs with `estimator_ledger.py`'s diff-based protocol rather than under + `tests/lint/rules/`. — caught in the same review. + +- [ ] **CE044 (RETIRED): the restricted evaluator's whitelist and its dispatch can no longer drift.** + The rule pinned parity between `computed_claims.py`'s `_ALLOWED_OPS` tuple and the `match` arms + of `_compute`, because a wildcard arm returning a value would have computed an unhandled + operator as something else — `ast.Mod` in the whitelist reported as *division*, by the one + sensor class whose whole purpose is catching arithmetic that lies. It was retired by **designing + the parity out** rather than by dropping the check: the whitelist IS the dispatch now + (`_BINARY_OPS` / `_UNARY_OPS` map an operator type to the function that computes it), so + admitting an operator and implementing it are a single edit. What replaced it is behavioural — + `7 % 2` and `2 ** 3` must raise naming the operator, every admitted operator must compute + correctly, and the two tables are pinned non-empty and disjoint by arity, which is the + anti-vacuity guard for the other two. `tests/lint/evaluator_dispatch.py` (144 lines) is deleted. + **The id stays reserved**: `tests/lint_tests/test_lint_task_surfaces.py::test_a_reserved_or_retired_id_is_not_live` + parametrizes over CE044 and CE056 together, because `runner.py`'s uniqueness assert covers + `ALL_RULES` only and a class-wired rule could claim either number with nothing failing. + Re-using CE044 would make `make lint` report findings under a number whose documented meaning + is something else. + +- [ ] **A `NamedTuple` field written at every construction site and read by nobody.** Two instances + in one plan — `FamilyFacts.family_resamples` and `_GateExperiment.rows` — and both were + invisible to `ruff` and `pyright`, because a written-never-read tuple field is + indistinguishable from a used one at the type level. **Measured why it is not cheap:** the + naive detector (collect a `NamedTuple`'s fields, grep `.field` across the package) reports 15 + "unread" fields in `optimize/` against those 2 real historical hits. Every false positive is a + field read by TUPLE UNPACKING (`members, rejected_at = holm_family(...)`) or by a consumer + OUTSIDE the package — tests and the skill's inline snippets legitimately read + `SearchComparison.head_score`, `RuleCeiling.ceiling` and the rest. Getting it right needs + unpacking analysis plus a cross-package read set, which is a different order of work from an + AST shape match. — caught twice in the optimize-family architecture plan. + +- [ ] **One rendered fact derived TWICE in a block by independent expressions.** `facts.family_size` + in the note ladder and `len(family)` in the trailing note: making them disagree passed the + whole suite until a test was written for the one state where they differ (a family with an + unmeasured member). This is CE062/CE040's shape one level up — two DERIVATIONS of one number + rather than two spellings of one formula — and the hard part is that neither expression is + wrong in isolation. — caught in the same plan. + +- [ ] **A conditional-expression fallback arm that is unreachable.** `_dead_weight_notes` carried an + `else` on a ternary that could not fire; replacing it with a `raise` left every test green. + `coverage.py` does not treat a ternary as a branch, so branch coverage reads 100% over a dead + arm — which is how it shipped. Detecting it needs a reachability argument over the values, not + an AST shape. — caught in the same plan. + +- [ ] **A `See .claude/decisions/X.md` pointer whose target does not discuss the subject.** The + orphan and dangling-pointer directions are both guarded + (`tests/lint_tests/test_lint_decision_log.py`); what is not is whether the file a docstring + points AT actually covers what the docstring stopped saying. That is the judgement + `.claude/decisions/README.md` states outright no rule can make, and the plan's own regression — + three specifications replaced by descriptions written from the function names — is what a + heuristic here would have to catch. Left to review deliberately. — caught in the same plan. + +- [ ] **A scripted multi-file edit whose match count or end boundary is not asserted.** Four + instances in one plan, each of which silently edited something it was not aimed at: a + `str.replace` without a count hit two unrelated test classes; a class-span heuristic stopped at + a decorator and rewrote a helper into infinite recursion; a `description=(` end-marker search + overran a block ending `)\n )` and DELETED two model fields; and `git checkout ` to + undo a temporary mutation discarded a whole phase's uncommitted work. Every one was caught by + the suite within seconds, so the harness already covers the OUTCOME — what has no guard is the + technique, which is a property of how an edit was made rather than of the tree. The cheap + discipline, recorded here because it is what actually works: assert the match count before + writing, prefer line-indexed edits over pattern matching, and restore a mutation from a `cp` + backup rather than from git. — caught in the same plan. + +## From `c/2026-08-20-optimize-skill-api-surface.md` (the optimize family's declared API) + +**CE066 is TAKEN** by that plan: `optimize-skill`'s `SKILL.md` imports from `coder_eval.optimize.api` +and nothing else. Reader at `tests/lint/skill_api_imports.py`, wired as +`tests/lint_tests/test_lint_plugin_optimize.py::TestCE066SkillImportsOnlyTheApi`. **Next free id: +CE067** (CE049, CE055, CE056 and CE061-CE065 remain reserved below; CE044 is retired). + +- [ ] **`optimize/api.py` must author NO markdown.** Every block it returns comes from a + `reports_optimize.render_*`, which is what keeps `CLAUDE.md`'s "every markdown block the skill + prints" true of that module. Nothing guards it — `tests/test_optimize_layering.py` pins only + WHICH module may import the renderer, not that the composites never format. The boundary was + broken TWICE during that plan and caught both times only by review: `_staleness_note`'s bolded + sentence, then Stage C's family-size line. An AST check for a string literal containing `**`, + `###`, `| ` or a leading `_` in that one module is the cheap shape; the hard part is exempting + docstrings, which is why it is not a five-minute job. — caught in review, twice. +- [ ] **A ledger writer that PERSISTS numbers derived from a run tree needs the CE053 discipline, and + CE053 cannot see it.** The rule keys on `load_suite_rows`/`load_arm_rows`; `record_round_*` reach + the tree through `arm_row_scores`, so they were outside it. A leftover row then lands in the + persisted vectors, both fronts, the lineage head AND the suite digest — after which a later + CLEAN round reports "The SUITE CHANGED" for a suite nobody touched. Fixed by hand in both + writers; what has no guard is the CLASS. Widening CE053's `_TREE_READERS` to include + `arm_row_scores` would cover it, at the cost of firing on every current caller (each of which + does reconcile, so each would need routing or a suppression) — which is why it is a candidate + rather than a one-line change. — caught in the final cross-phase review. + +- [ ] **`_REPORTERS` in `tests/test_optimize_api.py` is still hand-maintained** with a hardcoded + `if name == ...` dispatch ladder, where its sibling `_ENTRY_POINTS` now derives its completeness + from the module. The derivable property is "every composite that calls `_staleness_note` appears + here", which needs an AST walk of `api.py` rather than `inspect`. — caught in the final review. + +- [ ] **No `SKILL.md` python fence may reference a name no fence binds.** The snippet binder checks + that imports RESOLVE and that keywords BIND; neither sees a free variable. That is how Step 11's + fence came to use an `arms` binding Stage A had stopped providing — a `NameError` after the round + was paid for, which is the exact failure that fence's own paragraph warned about. An `ast` + free-variable audit over every fence is ~20 lines and found three such fences when run by hand + (two were already scheduled for repair, one was a live regression). — caught in review. +- [ ] **A per-replicate score reduction is declared FOUR times**: `load.row_replicate_scores`, + `fronts.arm_row_scores`, and twice inside `optimize/execution.py`. Converging them is a + behaviour change rather than a refactor — `arm_row_scores` renders an explained empty matrix + where the new primitive raises on an out-of-range index — so what is wanted is a sensor of the + shape `test_the_trim_is_declared_once`, not a rewrite. — caught in review. +- [x] **CLOSED.** `_ENTRY_POINTS` in `tests/test_optimize_api.py` was hand-maintained, claimed to + cover every composite taking a run-dir sequence, and silently fell behind TWICE — the second + time in the very phase whose comment justified widening `_require_run_dirs` by pointing at the + ledger writers' two sequences. `test_every_run_dir_parameter_has_a_boundary_entry` now derives + the set from the module by inspecting for a `Sequence[Path]` annotation, and checks it BOTH + ways (a missing entry and a stale one). It found one real gap on its first run — + `record_round_execution(run_dirs)` — and names the exact pair when mutated. +- [ ] **`measure_execution_noise_floor` threads no `reasons` sink**, so the execution track's no-floor + block cannot name which of its four preconditions failed while the activation track's can. This + entry already existed above for `_execution_diagnostics`; the composite surface makes it + user-visible. — re-confirmed. +- [ ] **`headroom_report` reads the run tree THREE times** (its own reconcile sweep, `arm_row_scores`', + and `floor_preflight`'s), re-validating every `task.json` each time — ~135 validations for 45 + files on a 15-row, 3-replicate suite. Threading preloaded rows into `arm_row_scores` would remove + two, but that is a signature change on a rank-3 primitive. Performance, not correctness. +- [ ] **`resolve_model` and `resolve_arm_model` live in `execution.py` but BOTH tracks need them.** The + activation composites import them across the track boundary, which `activation.py` itself + correctly does not. `load.py` (rank 0) is the cleaner home and would let `activation.py` use them + directly — but it is a rename across the family under + `test_a_moved_name_lives_in_exactly_one_module`, unrelated to that plan's goal. Recorded so the + next reader finds the reasoning instead of rediscovering the smell. +- [ ] **`headroom_ceiling`'s docstring contradicts `rule_row_map`'s** about whether a never-failing + rule is a key: the first says `rule_row_map` "OMITS a rule that failed nowhere", the second says + "Every rule SEEN is a key". One is stale prose. Pre-existing, unrelated to that plan. +- [ ] **Do not run a mutation-testing reviewer against a tree you are still editing.** A review agent + applied and reverted source mutations from its own snapshot while the implementer was writing to + the same file, and warned that its restore could have discarded concurrent edits. It did not, + verified by grep — but the technique and the concurrency are individually fine and jointly + dangerous, and nothing prevents them being combined. Process, not code. — caught in that plan. diff --git a/.claude/shared/run-layout.md b/.claude/shared/run-layout.md index 57edc4a6..a0f33221 100644 --- a/.claude/shared/run-layout.md +++ b/.claude/shared/run-layout.md @@ -14,6 +14,82 @@ runs/////{task.json, task.log, artifacts/} - `task.json.malformed` — present only on the docker degrade path: when an existing `task.json` fails to parse (schema skew from a stale `:latest` image, or a truncated/torn write), the docker runner moves the unparseable original aside to this sidecar and writes a synthetic `final_status=ERROR` `task.json` in its place. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `task.log` — the human-readable task log; `artifacts/` — files the agent produced. +## Suite rollups (dataset-backed tasks only) + +A task carrying `dataset:` fans out into one row-task per row and additionally writes a +per-suite rollup: + +``` +runs////{suite.json, suite.md} +``` + +`` is the original (pre-fan-out) `task_id`. Nothing is written for a task +without `dataset:`. + +`suite.json` carries the suite's pass counts plus `criterion_aggregates[]` — one entry per +criterion that opted into across-row aggregation, each with: + +- `criterion_type`, and `description` (set when a task stacks several criteria of the same + type, e.g. one `skill_triggered` per skill — that is what distinguishes them); +- `rows_total` and `rows_excluded` — the denominator and what was dropped from it. A row + that errored before criteria ran (a timeout, say) is **excluded** rather than scored, so + metrics are computed over `rows_total - rows_excluded`; +- `metrics` — a **flat** name → float map. Classification-style criteria emit + `accuracy`, `macro_f1`, and per-label `precision.