From 5ae75dedebd07e060d02325b20e3b5de2d233e93 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Tue, 28 Jul 2026 13:58:50 -0700 Subject: [PATCH 1/4] docs: classify the frozen v1 baseline and preregister the v2 gate manifest ## Summary - Add `review-suite/evals/v2/FAILURE-TAXONOMY.md`, classifying every material outcome across the three frozen v1 scored reports by the smallest evidenced cause, independently re-verified against each report's `per_case` data. - Add `review-suite/evals/v2/DECISION-RECORD.md`, giving #51-#57 each an explicit disposition (retain, narrow, defer) with baseline evidence, the smallest proposed schema delta, fixtures, scored corpus slice, and owner-confirmation-flagged thresholds. - Add `review-suite/evals/v2/gate-manifest.json`, preregistering the v2 corpus/grader versions, runtime/model stratum, run count/timeout/cost, deterministic invariants, proposed quality/stability thresholds, mechanism ablations, and threshold-change rules before any scored v2 output exists. - Point `review-suite/evals/README.md` at the new `v2/` directory. ## Why - #59 requires this taxonomy, decision record, and gate manifest as one reviewable candidate before any live issue body or dependency edge changes. - Every threshold is marked as a proposal requiring owner confirmation, per #59's own instruction not to assert a gate as settled from a 5-attempt sample. - This candidate changes no review prompt, schema, rubric, orchestration, or caller runtime behavior; it only classifies evidence and proposes what #51-#57 should implement. --- review-suite/evals/README.md | 13 + review-suite/evals/v2/DECISION-RECORD.md | 405 ++++++++++++++++++++++ review-suite/evals/v2/FAILURE-TAXONOMY.md | 222 ++++++++++++ review-suite/evals/v2/README.md | 35 ++ review-suite/evals/v2/gate-manifest.json | 171 +++++++++ 5 files changed, 846 insertions(+) create mode 100644 review-suite/evals/v2/DECISION-RECORD.md create mode 100644 review-suite/evals/v2/FAILURE-TAXONOMY.md create mode 100644 review-suite/evals/v2/README.md create mode 100644 review-suite/evals/v2/gate-manifest.json diff --git a/review-suite/evals/README.md b/review-suite/evals/README.md index d9c04bc..de89843 100644 --- a/review-suite/evals/README.md +++ b/review-suite/evals/README.md @@ -40,6 +40,11 @@ review-suite/evals/ │ ├── SOURCING.md ground truth, sanitization, batches │ ├── LIMITATIONS.md explicit inputs to interpretation │ └── pilot/.report.json the unscored pilot's compact reports +├── v2/ the v2 planning/definition record (#59) +│ ├── FAILURE-TAXONOMY.md every v1 material outcome, classified +│ ├── DECISION-RECORD.md per-mechanism evidence and disposition +│ ├── gate-manifest.json preregistered v2 gates, before scoring +│ └── audits/ the three independent graph audits └── artifacts/ opt-in captured output, not in git review-suite/scripts/evals/ @@ -70,6 +75,14 @@ figure. In particular: **the connector stratum is deferred, not satisfied.** Connector-escape recall has never been measured here, and no human-review figure may be reported as a connector figure. +## v2 planning + +[`v2/README.md`](v2/README.md) records the planning/definition gate that reads +this frozen v1 baseline and turns it into an implementation-ready decision for +the review suite's next mechanisms, before any scored v2 output exists. It +changes no review prompt, schema, rubric, orchestration, or caller runtime +behavior by itself. + ## Calibration An uncalibrated grader does not report a conservative score; it reports a diff --git a/review-suite/evals/v2/DECISION-RECORD.md b/review-suite/evals/v2/DECISION-RECORD.md new file mode 100644 index 0000000..ecf4f0d --- /dev/null +++ b/review-suite/evals/v2/DECISION-RECORD.md @@ -0,0 +1,405 @@ +# v2 mechanism decision record + +`decision_record_version: 1.0`. Reads `FAILURE-TAXONOMY.md` as its evidence base +and turns each classified cause into an explicit disposition for every mechanism +proposed in #51–#57: retain, narrow, split, merge, defer, or close. Every +threshold below is a **proposal** requiring owner confirmation, not a settled +fact — see the "Why these are proposals, not settled numbers" note at the end of +each entry that carries one. + +This record changes no review prompt, schema, rubric, orchestration, or caller +runtime behavior by itself. It specifies what #51–#57 must implement; the child +tickets carry out the actual change. + +## #51 — Make clean verdicts require passing validation and current-head lens evidence + +**Disposition: retain, unchanged.** + +- **Baseline cases and metrics demonstrating the problem.** None from the scored + stochastic corpus — this is a deterministic protocol/validator defect, + independently evidenced by the reproduction already documented in #51's own + body: (1) a schema-valid `clean` result can pair with every focused/full + validation entry set to `failed`, because pair validation never cross-checks + packet validation status against aggregate verdict; (2) the orchestrator's + re-review matrix allows a correctness-fix or code-simplicity-fix path to reach + a new-head aggregate without a fresh solution-simplicity execution for that + exact head. Per #59's own text, a known deterministic contradiction may be + retained without a stochastic recall delta, and this is exactly that case. +- **Why existing behavior or a smaller change is insufficient.** The hole is + structural: nothing in `review-result.schema.json` v1.0 or + `review-suite/scripts/validate.py` ties `verdict: clean` to the packet's own + validation evidence or to a completed, candidate-bound lens execution. + Documentation alone cannot close a schema-level gap that a well-intentioned + caller can trip on by construction. +- **Smallest proposed intervention and canonical owner.** Exactly the behavior + already specified in #51's "Required contract behavior" section: cross-check + packet validation status against verdict (no `clean` may pair with a `failed` + or `unavailable` required validation entry), and add a minimal + `lens_executions` evidence array to the aggregate result recording, for each + of the three required lenses, its name, head SHA, comparison-base SHA, + verdict, and freshness. Canonical owner: + `review-suite/contracts/review-result.schema.json`, + `review-suite/scripts/validate.py`, and `review-suite/CONTRACT.md`'s + verdict-semantics section; `review-code-change/SKILL.md`'s orchestration + matrix. +- **Exact contract/version and compatibility impact.** + `review-result.schema.json`: `1.0 → 1.1` (additive `lens_executions` array + plus the packet/verdict cross-check rule; no packet schema change). Atomic + migration across the bundled skill copies via `just sync-contracts`; no + dual-schema layer; stale v1.0 aggregate evidence is rejected with a clear + migration error, exactly as #51 already specifies. +- **Deterministic positive and negative fixtures.** The 11 fixtures already + enumerated in #51's "Required tests" section (failed focused/full/unavailable + validation paired with an otherwise-clean result; missing, duplicate, or + stale-head/stale-base lens execution; the two head-changing-fix restart cases; + unchanged-head base-drift retention; stale v1.0 rejection). +- **Scored corpus slice expected to change.** None. This is a deterministic + invariant validated by canonical fixtures, not a stochastic recall target; + replaying the frozen v1 baseline's 15 scored cases at v1.1 must produce + byte-identical verdicts and findings for every case, because none of them + exercises a failed/unavailable validation entry or a stale lens execution — + the new validator rule only rejects previously-invalid pairs that no v1 case + contains. +- **Preregistered quality/stability/non-regression target.** Non-regression + only: the 15 frozen v1 cases must replay identically at v1.1 (see above). No + quality or stability target applies — there is nothing stochastic to measure. +- **Maximum acceptable latency and cost impact.** Negligible — no new reasoning + pass, only a structural evidence field and a validator rule. *Proposal:* cap + any measured latency/cost delta on the `s1` stratum at 5%, as a sanity ceiling + rather than an expected effect; if a re-run of `s1` at v1.1 exceeds it, treat + that as a signal the orchestration matrix change did more than intended, not + as an acceptable cost of the fix. +- **Required ablation and removal rule.** None in the normal sense — a + deterministic invariant closing a known contradiction cannot be "ablated" + without reintroducing that contradiction. The only removal condition is proof + that the contradiction is unreachable through some other structural mechanism, + which no evidence here supports; #57 verifies the invariant holds rather than + testing whether removing it changes anything. + +## #52 — Evaluate compact review coverage, impact, and risk evidence + +**Disposition: narrow.** The full changed-surface ledger, acceptance-trace, +risk-profile, and aggregate-coverage apparatus in #52's current body is **not** +justified by the baseline and is dropped. Only one baseline case demonstrates a +consumer/impact-traversal gap. + +- **Baseline cases and metrics demonstrating the problem.** + `dependency-strictness-propagation` (`s1`): `mean_recall: 0.0`, + `ever_referred_root_cause_ids: []` across all 5 attempts — a confident, + zero-ambiguity miss. Root cause `rc.sibling-call-site-keeps-permissive-mode`: + a changed shared helper (`dependency_finalized`) has two call sites, one + hardened by the diff and one left permissive, and every attempt missed the + second one. + +- **Why existing behavior or a smaller change is insufficient.** The current + packet gives a complete diff and repository access but has no structural + requirement that a reviewer account for other call sites of a symbol whose + behavior changed. Five for five attempts omitted the same sibling call site, + which is a systematic, repeatable gap, not incidental variance a prompt-only + nudge would reliably fix without something for a validator to check. + +- **The smallest proposed intervention and canonical owner.** Add exactly one + small, additive evidence array to the review result — + `consumer_impact_evidence` — instead of #52's original changed-surface ledger, + acceptance trace, and risk-profile triad: + + ```jsonc + "consumer_impact_evidence": [ + { + "changed_symbol": "string, the changed shared symbol/contract", + "location": "string, its defining location", + "consumer_search_evidence": [ + { "location": "string", "detail": "string, what was inspected and found" } + ], + "disposition": "all_consumers_consistent | inconsistency_found | no_other_consumers" + } + ] + ``` + + #52 owns only the **schema and validator**: the structural shape of this array + and the rule that `clean` requires it to be non-empty, or explicitly justified + empty, whenever the correctness lens's own traversal pass (owned by #53) + determines a changed symbol has other call sites. #52 does **not** own the + lens behavior that populates it — that ownership boundary is what keeps this + ticket and #53 from duplicating each other, per the scope/completeness audit + requirement that no sibling duplicate an owner. No packet schema change is + needed: the packet already ships the complete diff and live repository access + is available to the reviewing agent, so no new packet-side ledger is required + to enable consumer search. + +- **Exact contract/version and compatibility impact.** + `review-result.schema.json`: `1.1 → 1.2` (additive `consumer_impact_evidence` + array; no packet schema change). Same atomic-migration, no-dual-layer + precedent as #51. + +- **Deterministic positive and negative fixtures.** A sanitized fixture pair + shaped like `dependency-strictness-propagation`: a changed shared + permissive/strict helper with a sibling call site. Positive: the result's + `consumer_impact_evidence` identifies and inspects both call sites → validator + accepts `clean`. Negative: the same fixture with the evidence array omitted, + or naming only the hardened call site → validator rejects `clean`. + +- **Scored corpus slice expected to change.** None directly from #52 alone — #52 + adds only the structural evidence capability; it ships no new reviewer + behavior and therefore does not by itself move any case's recall. The slice + this schema addition exists to enable — `dependency-strictness-propagation` — + only changes once #53's traversal pass (built on this schema) is implemented + and re-scored. Recording this plainly here rather than overclaiming #52's own + effect. + +- **Preregistered quality/stability/non-regression target.** Non-regression + only, and only deterministic: the schema addition must not change any lens's + verdict on any of the 15 frozen v1 cases, because none of them has a `clean` + result whose validity should depend on this new field being present (no other + v1 case involves a changed symbol with sibling call sites per its expectation + record). No quality/stability target applies to #52 itself. + +- **Maximum acceptable latency and cost impact.** Negligible on its own — no new + model reasoning, only a schema/validator change. *Proposal:* no measurable + per-attempt cost delta expected from #52 in isolation; any observed delta + belongs to #53's passes, not this ticket's schema addition. + +- **Required ablation and removal rule.** Tied to #53 and #57: if, once #53's + traversal pass is implemented and `s1` is re-scored, + `dependency-strictness-propagation` (or an equivalent v2 case) does not move + from miss toward matched or a genuinely improved referral, and no other case + benefits from this evidence array, remove it as unjustified single-case + structural complexity. This is the explicit removal condition #57's ablation + protocol must check. + +## #53 — Evaluate correctness traversal, verification sufficiency, and specialist routing + +**Disposition: retain the traversal and verification-sufficiency work; drop +specialist routing entirely.** Nothing in the baseline supports specialist +modules; inventing them now would be exactly the unevidenced expansion #59's own +non-goals forbid. + +- **Baseline cases and metrics demonstrating the problem.** Both confident + misses in `s1`: + + - `dependency-strictness-propagation` (traversal) — see #52's entry above. + - `stale-claim-release-guard` — `mean_recall: 0.0`, + `ever_referred_root_cause_ids: []` across all 5 attempts. Root cause + `rc.guard-skipped-when-snapshot-owner-absent`: the ownership check in + `_release` is conditional on the snapshot having had an owner, so the exact + scan-then-claim interleaving the change exists to guard against skips the + comparison. The added test "starts from an owned entry, so it exercises the + branch that was already safe and passes" — a happy-path test accepted as + sufficient evidence for an untested concurrency edge case. + + No baseline case demonstrates a need for the specialist modules (security, + concurrency-as-a-separate-routed-module, compatibility/migration, operations, + or UI) as originally drafted in #53. The concurrency miss above is fully + explained by a verification-sufficiency gap, not by the absence of a routed + concurrency specialist — the reviewer never needed a separate specialist + context; it needed to ask whether the existing test could fail for the stated + trigger. + +- **Why existing behavior or a smaller change is insufficient.** Both misses + were 5-for-5 across independent fresh processes with no partial/referred + attempts, which rules out incidental prompt variance as the explanation — the + omission is systematic. Neither omission is addressed by #52's schema addition + alone; #52 gives the evidence *shape*, but nothing in the current + `review-correctness` rubric instructs the lens to actually walk to sibling + call sites or to interrogate whether a passing test could fail for the claimed + risk. + +- **The smallest proposed intervention and canonical owner.** Add exactly two + required passes to `review-correctness`'s rubric, replacing #53's original + four-pass draft: + + 1. **Impact/consumer-traversal pass** — for each changed shared symbol or + contract, search for other call sites/consumers and populate #52's + `consumer_impact_evidence`. This is #53's only claim on #52's schema; #53 + does not add a second schema for the same evidence. + + 2. **Verification-sufficiency pass** — for each claimed validation command or + test touching a materially risky change, ask whether it would actually fail + for the specific trigger the change addresses, and record the answer in a + new, small evidence array owned entirely by #53: + + ```jsonc + "verification_sufficiency_evidence": [ + { + "claimed_test_or_command": "string", + "exercises_material_risk": "yes | no | not_applicable", + "reasoning": "string, what triggering condition was or was not exercised" + } + ] + ``` + + **Drop entirely:** the "risk-routed specialist pass" and the "Specialist + modules" section (security/authorization, concurrency, contracts/migration, + operations, UI). No case in the baseline demonstrates that a routed specialist + context — as opposed to the two general passes above — is needed to catch + either confident miss. Reintroducing specialist routing would need its own + baseline evidence and its own decision-record entry; it does not get one from + this ticket's evidence. + + Canonical owner: `skills/review-correctness/SKILL.md` and its references; + `review-suite/contracts/review-result.schema.json` for the new evidence array. + +- **Exact contract/version and compatibility impact.** + `review-result.schema.json`: `1.2 → 1.3` (additive + `verification_sufficiency_evidence` array on correctness-lens results; no + packet schema change). Same atomic-migration precedent. + +- **Deterministic positive and negative fixtures.** Two pairs: + + 1. The `dependency-strictness-propagation`-shaped pair already specified under + #52 (owned jointly at the schema level by #52, populated by #53's traversal + pass). + 2. A new pair shaped like `stale-claim-release-guard`: a guard-release + function with an owner-absent interleaving edge case. Positive: the + added/existing test exercises the owner-absent branch, and + `verification_sufficiency_evidence` records `exercises_material_risk: yes` + → clean. Negative: the test only exercises the already-safe owned-entry + branch, and the evidence correctly records `exercises_material_risk: no` → + the missing coverage produces a gating finding, not a silent `clean`. + +- **Scored corpus slice expected to change.** `s1-correctness-orchestrator`'s + `dependency-strictness-propagation` and `stale-claim-release-guard` — both are + the direct target of this change and are expected to move from + `mean_recall: 0.0` at re-scoring. + +- **Preregistered quality/stability/non-regression target (proposal).** Given a + 5-run-per-case sample (`LIMITATIONS.md` item 36 — a single attempt moves a + per-case `mean_recall` by a fifth), propose: at v2 re-scoring, each of the two + target cases reaches `mean_recall ≥ 0.6` (at least 3 of 5 attempts matched), + or the combined matched+referred rate reaches `≥ 0.8` with the referred + attempts showing real content overlap (not a flat miss). This threshold is a + genuine judgment call given the tiny sample and is offered as a proposal, not + a settled number — the owner should confirm or replace it before it gates + anything. Non-regression: neither target case's `verdict_stability` nor + `finding_stability` may fall below its v1 floor (both are `1.0` today); the + three clean controls (`dependency-hint-parser-coverage`, + `post-bootstrap-module-load`, `session-continuation-summary`) must remain at + zero false positives; and `process-isolation-assertion`'s false-alarm rate + must not get worse than its observed 2-of-5. + +- **Maximum acceptable latency and cost impact (proposal).** Two added reasoning + passes will increase output tokens and likely latency on `s1`. *Proposal:* + raise `s1`'s cost ceiling from $9.00 to $12.00 for the v2 re-score, and be + prepared to raise the 300 s timeout to 450 s if a pilot run approaches it — + both flagged explicitly as proposals needing owner confirmation, sized from + the v1 pilot's worst observed cold cost ($0.2021/attempt) and worst latency + (51.2 s) rather than invented. + +- **Required ablation and removal rule.** #57 must run three configurations + against `s1`: traversal pass only, verification-sufficiency pass only, and + both together, each scored against the same two target cases plus the full + stratum for non-regression. Remove either pass if, in isolation, it does not + move its target case and does not measurably help any other case — this + directly implements #57's "unique contribution beyond earlier mechanisms" + requirement and must not be skipped in favor of only testing the combined + configuration. + +## #54 — Evaluate independent correctness discovery and per-finding validation + +**Disposition: defer, unchanged.** #54's own gate text — "a remaining recall or +stability failure that one-pass changes do not solve" — is only measurable after +#53 ships and is scored. No baseline evidence exists yet for or against +independent explorers or a validator topology; inventing one now would violate +#59's own non-goal against choosing mechanisms before reading evidence that does +not yet exist. No schema delta, fixture, or threshold is drafted here. #54 stays +blocked on #53's eventual v2 scoring evidence, not on #59 itself — #59's own +closure does not by itself unblock #54. + +## #55 — Migrate caller contracts and prove current-head review integration + +**Disposition: defer, unchanged.** Downstream of #54's outcome (its own +"Definition status" already says so); nothing in this decision record changes +that. No schema delta, fixture, or threshold drafted. + +## #56 — Operationalize adjudicated connector outcomes as review regressions + +**Disposition: retain, now evidence-backed.** #56's existing intake-record and +promotion-workflow design (already specified in its own body) is not +architecturally changed by this decision record. What changes is its evidence +basis and two small guardrails. + +- **Baseline cases and metrics demonstrating the problem.** A real + connector-outcome dataset exists and the repository owner has authorized its + use for v2-cycle analysis. Per the owner's explicit instruction, **the source + repository is described here only generically — "a private, owner-authorized + connector-review source" — and is never named in this record, in #56's revised + body, or in any other public artifact.** This record does not attempt to + characterize the dataset's contents beyond that description; doing so is out + of scope for this ticket and, per the owner's own boundary, would require + stopping and returning `blocked` rather than searching for or inferring the + source. +- **Explicit non-retroactivity note.** This source is a v2-cycle addition. It + does **not** retroactively modify, extend, or rescore + `review-suite/evals/baseline/v1/`, which stays exactly as merged and frozen in + #58. `frozen-configuration.json`'s `connector-escape` stratum entry remains + `state: deferred_not_satisfied` with zero attempts; nothing in this decision + record changes that entry, and any future connector stratum built from this + new source is separate v2-only work with its own preregistered cost ceiling, + not a retroactive fill of the v1 gap. +- **Why existing behavior or a smaller change is insufficient.** Before this + authorization, #56 had no evidence base at all beyond the deferred (and + explicitly zero-data) connector stratum, which is why it was previously + provisional. The workflow it already specifies (intake record, promotion + steps, optional scoped-rule escalation gated on demonstrated need) does not + need architectural change now that evidence exists — it needs the sourcing + boundary stated explicitly and one mechanical guardrail added. +- **The smallest proposed intervention and canonical owner.** No schema + redesign: #56 continues to reuse the corpus/expectation/provenance contracts + already owned by #50/#58 (`review-suite/evals/contracts/*.schema.json`). Add + one small, mechanical guardrail to #56's intake validator: when a curation + record's `provenance.source_class` is `private_authorized`, its public-facing + `source_description` field must match an allow-listed generic phrase (e.g. "a + private, owner-authorized connector-review source") and must fail closed if it + contains a path-like token (`/`), a bare hostname, or any string matching a + configured deny-list of known-sensitive identifiers. This is a cheap, + mechanical backstop for a disclosure boundary that is otherwise only enforced + by curation discipline — consistent with this repository's own observation + (`LIMITATIONS.md` items 21–24, 29–31) that curation discipline alone does not + reliably catch every leak. +- **Exact contract/version and compatibility impact.** No packet/result schema + version change. The corpus/expectation/provenance contracts are unchanged; + only #56's own intake validator (its own code, not a shared contract) gains + the guardrail above. +- **Deterministic positive and negative fixtures.** #56's already-specified + fixture list (positive, negative, duplicate, unresolved, restricted-data, + promotion-decision) plus one new negative fixture: a `private_authorized` + curation record whose `source_description` contains a disallowed token → + intake validation fails closed. +- **Scored corpus slice expected to change.** None directly — #56 is a + workflow/tooling ticket, not itself scored against `s1`/`s2`/`s3`. Any future + regression case sourced from the new connector data is separate, later work + with its own preregistered cost ceiling; #56 does not commit to running one + now. +- **Preregistered quality/stability/non-regression target.** Unchanged from + #56's own body: any rubric/instruction change promoted through this workflow + must independently pass #56's already-specified "measure before changing + guidance" step and #57's preregistered gates. No new target is added here. +- **Maximum acceptable latency and cost impact.** Not applicable — #56 changes + no reviewer runtime behavior by itself. +- **Required ablation and removal rule.** Not applicable to #56 itself; any + rubric change it later promotes is subject to #57's ablation/removal rule, as + #56's own body already states. + +## #57 — Run the preregistered review v2 ablation and integration closeout + +**Disposition: defer, unchanged.** Downstream of #55 and #56; #57's own +"Definition status" already names #59 as its gate for the exact scored-v2 +configuration, which this record and `gate-manifest.json` provide. One prose +correction is in scope: #57's current body reads "Run the evidence-backed v2 +experiment approved by #58" in its Goal section. #58 curated the frozen v1 +corpus and baseline; it is #59 — this ticket — that approves the v2 mechanism +set and gate manifest #57 executes against. That single stale cross-reference is +corrected in #57's revised body; #57's disposition, required inputs, and +acceptance criteria are otherwise unchanged. + +## Net effect on the dependency graph + +Applying the dispositions above to the native graph: `blockedBy #59` is removed +from #51 once #59 closes, leaving #51 with no open `blockedBy` edge. Every other +child (#52 through #57) keeps its existing `blockedBy` edge onto its current +predecessor (#52→#51, #53→#52, #54→#53, #55→#54, #56→#52, #57→#55,#56), all of +which stay open. **#51 is therefore the only open issue in this epic with zero +open blockers once #59 closes** — the expected net effect #59's own text names. +See `audits/dependency-sequencing-audit.md` for the verified read-back +confirming this against the live graph after the edge and body mutations are +applied. diff --git a/review-suite/evals/v2/FAILURE-TAXONOMY.md b/review-suite/evals/v2/FAILURE-TAXONOMY.md new file mode 100644 index 0000000..8f0ba51 --- /dev/null +++ b/review-suite/evals/v2/FAILURE-TAXONOMY.md @@ -0,0 +1,222 @@ +# v1 failure taxonomy + +`taxonomy_version: 1.0`. Classifies every material outcome in the frozen v1 +baseline (`review-suite/evals/baseline/v1/*.report.json`, suite commit +`2644c12`, `v1_review_behaviour_commit` +`16560d807c66076fcbf3f00d3a87f543c6ae2458`) by the smallest evidenced cause. +Read [`LIMITATIONS.md`](../baseline/v1/LIMITATIONS.md) beside this document; +every classification below cites the exact `per_case` field it rests on, and +every cited figure was independently re-read from the committed report JSON +while writing this document, not copied from an earlier summary. + +This taxonomy answers one question per case: is the outcome reviewer, +evidence/rubric, grader/corpus, or runtime/tooling behavior? It does not +prescribe a mechanism; that is [`DECISION-RECORD.md`](DECISION-RECORD.md)'s job, +and it only proposes a mechanism for a cause classified here as reviewer +behavior with a demonstrated, repeatable (`mean_recall: 0.0`, +`ever_referred_root_cause_ids: []`) shape. + +## Category vocabulary + +Each case is tagged with one primary category from #59's own list, then rolled +up into one of the five acceptance-criterion buckets: **reviewer**, +**evidence/rubric**, **grader/corpus**, or **runtime/tooling**. A case with no +expected root cause and no false-positive/false-alarm attempts is recorded as a +**correct control**, not a failure, but is still listed so every material +outcome in the frozen reports is accounted for. + +## s1-correctness-orchestrator (7 cases, 5 attempts each, 35 attempts total) + +| case | outcome | primary category | bucket | +| ----------------------------------- | ----------------------- | --------------------------------------------------------------------- | ----------------------------------------- | +| `dependency-hint-parser-coverage` | correct control | — | reviewer (correct) | +| `dependency-strictness-propagation` | confident miss | weak change/consumer/negative-space traversal | reviewer | +| `optional-tool-probe` | mixed partial | domain-specific reasoning miss, compounded by grader/corpus ambiguity | reviewer + grader/corpus (undisentangled) | +| `post-bootstrap-module-load` | correct control | — | reviewer (correct) | +| `process-isolation-assertion` | false-alarm instability | stochastic search-order or anchoring failure | reviewer | +| `session-continuation-summary` | correct control | — | reviewer (correct) | +| `stale-claim-release-guard` | confident miss | failure to test verification sufficiency | reviewer | + +**`dependency-strictness-propagation`** — `mean_recall: 0.0`, +`ever_referred_root_cause_ids: []`, `false_positive_attempts: 5/5`, +`finding_stability`/`verdict_stability: 1.0`. Root cause +`rc.sibling-call-site-keeps-permissive-mode`: the change hardens one caller of +`dependency_finalized` and leaves a sibling caller on the permissive path; the +added tests exercise only the hardened caller. Confirmed independently against +`review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-strictness-propagation.json`. +Zero ambiguity across all 5 attempts: a repeatable, demonstrated failure to +traverse to a sibling consumer of a changed shared contract. Classified **weak +change/consumer/negative-space traversal**, not verification-sufficiency, +because the reviewer never reasoned about the second call site at all — the gap +is discovery, not test-adequacy judgment (the added test's narrowness is a +compounding symptom of the same missed traversal, not an independent cause). + +**`stale-claim-release-guard`** — `mean_recall: 0.0`, +`ever_referred_root_cause_ids: []`, `false_positive_attempts: 5/5`, stability +1.0/1.0. Root cause `rc.guard-skipped-when-snapshot-owner-absent`: the ownership +check in `_release` is conditional on the snapshot having had an owner, so the +exact scan-then-claim interleaving the change exists to guard against skips the +comparison and releases a live claim. Confirmed against the same expectations +directory. The added test "starts from an owned entry, so it exercises the +branch that was already safe and passes" — the reviewer accepted a passing, +happy-path test as sufficient evidence for a concurrency/ownership edge case it +never exercised. Classified **failure to test verification sufficiency**, not +traversal — the miss is single-file and single-function; no consumer walk would +have found it, only asking whether the test could actually fail for this trigger +would have. + +**`optional-tool-probe`** — `mean_recall: 0.2` (1 of 5 attempts fully matched), +`ever_referred_root_cause_ids: ['rc.probe-checks-status-not-absence']`, +`finding_stability: 0.8`, `verdict_stability: 1.0`. Root cause: the probe checks +the tool's status rather than its absence, so a missing executable raises +instead of skipping, and the failure only reproduces on the one environment (CI) +the skip exists for. Unlike the two confident misses, this case is **not** +zero-ambiguity: one attempt matched fully, and it is a genuine domain-specific +reasoning question (does a status check detect absence?) rather than a pure +traversal or verification-sufficiency gap. Because 4 of 5 attempts landed in the +referred bucket rather than a flat miss, and because `LIMITATIONS.md` items 3–4 +document that the reference grader's containment matching and one-token surface +rule systematically under-recognize a correctly identified finding phrased a +fifth way, this case's true reviewer-behavior component cannot be cleanly +separated from a grader/corpus confound with only 5 attempts. **This case does +not, by itself, justify a new mechanism** and is not cited as evidence for any +#51–#57 disposition; it is recorded as an open question for whoever next +calibrates or recalibrates this expectation, per #59's own instruction not to +prescribe a change until a failure is separated from grader and corpus defects. + +**`process-isolation-assertion`** — a negative control +(`expected_root_cause_ids: []`), re-raised in 2 of 5 attempts +(`false_alarm_attempts: 2`, `verdict_stability: 0.6`), while every other clean +control in this stratum scored `verdict_stability: 1.0`. The underlying +observation (a test named for process freshness actually asserts executor +identity) was adjudicated not material because the packet states no process +identifier is observable at all and the assertion is the strongest available +proxy — but LIMITATIONS.md item 15 also records that this case's first +adjudication came from the review suite under evaluation itself, which is weaker +evidence than an independent human disposition. Classified **stochastic +search-order or anchoring failure**: the same packet and prompt produced a +stable `blocking`/`strong_recommendation` re-raise on 2 of 5 independent fresh +processes and a correct pass on the other 3, with no change in evidence between +attempts. This is real, repeatable verdict instability on an already-adjudicated +finding, not a corpus or grader artifact — the case is not evidence for a new +mechanism, but it is evidence that a v2 gate must keep measuring false-alarm +stability on adjudicated-rejected findings rather than assuming the current +suite already has zero false-alarm risk. + +**Correct controls** (`dependency-hint-parser-coverage`, +`post-bootstrap-module-load`, `session-continuation-summary`): all three have +`expected_root_cause_ids: []`, `false_positive_attempts: 0`, and +`verdict_stability: 1.0`. Each pairs a real adjudicated-not-material observation +(recorded as an `accepted_non_finding` in its expectations file) with a reviewer +that correctly declined to gate on it in all 5 attempts. No failure of any kind; +recorded here only so every case in the stratum is accounted for. + +## s2-solution-simplicity-lens (4 cases, 5 attempts each, 20 attempts total) + +| case | outcome | primary category | bucket | +| -------------------------------- | -------------------- | ----------------------- | ------------------ | +| `reconciliation-outcome-type` | correct control | — | reviewer (correct) | +| `record-status-transition-guard` | correct control | — | reviewer (correct) | +| `registry-client-layering` | referred, not missed | grader/corpus ambiguity | grader/corpus | +| `setup-service-path-gateway` | referred, not missed | grader/corpus ambiguity | grader/corpus | + +Both material cases show `mean_recall: 0.0` with a non-empty +`ever_referred_root_cause_ids` that names the expected root cause exactly +(`rc.three-client-concepts-duplicate-binding` for `registry-client-layering`, +`rc.gateway-abstracts-a-pure-function-with-one-implementation` for +`setup-service-path-gateway`). Per the owner-settled three-way grading method +(`LIMITATIONS.md` item 14) and the deliberate-non-calibration policy for scored +cases, a referral this consistent — the grader recognizes the finding's surface +and root cause every time but the shipped formulation never matches the +reviewer's actual prose — is the corpus/grader behaving exactly as designed for +an intentionally uncalibrated scored expectation, not a demonstrated reviewer +defect. **Neither case is treated as evidence of a solution-simplicity lens +failure.** `setup-service-path-gateway` additionally shows +`false_positive_attempts: 4/5`; this is not independently confirmed as a +distinct false-alarm defect — `LIMITATIONS.md` items 3–4 document the same +containment/one-token confound producing exactly this shape — and is recorded +here as an open measurement question for the corpus/grader owner, not folded +into any #51–#57 disposition. + +The two correct controls (`reconciliation-outcome-type`, +`record-status-transition-guard`) both show `expected_root_cause_ids: []`, zero +false positives, and `verdict_stability: 1.0` across all 5 attempts: +requirement-justified near-misses the lens correctly left clean. + +## s3-code-simplicity-lens (4 cases, 5 attempts each, 20 attempts total) + +| case | outcome | primary category | bucket | +| -------------------------------------- | ----------------------------------------- | ---------------------------------------------------- | ------------------ | +| `compat-accessor-boundary-duplication` | correct control | — | reviewer (correct) | +| `env-inventory-bullet-format` | correct control, low discriminating power | rubric ambiguity or overbreadth (measurement design) | evidence/rubric | +| `metrics-label-formatting-duplication` | referred, not missed | grader/corpus ambiguity | grader/corpus | +| `watcher-check-policy-duplication` | referred, not missed | grader/corpus ambiguity | grader/corpus | + +`metrics-label-formatting-duplication` and `watcher-check-policy-duplication` +both show `mean_recall: 0.0` with `ever_referred_root_cause_ids` matching their +expected root cause exactly (`rc.label-expression-copied-into-two-new-functions` +and `rc.local-policy-duplicated-across-call-sites`), zero false positives. Same +disposition as the s2 pair: intentionally uncalibrated scored expectations +behaving as designed, not a demonstrated code-simplicity lens defect. + +`metrics-label-formatting-duplication` (this stratum's case 7) and s2's +`registry-client-layering` (case 3) trace to the same source PR comment read at +two levels (`LIMITATIONS.md` item 32, confirmed by the owner). **Their recall +figures are correlated, not independent, and must never be combined or presented +as two independent samples in any aggregate figure this decision record or the +v2 gate manifest reports.** + +`env-inventory-bullet-format` is a correct control +(`expected_root_cause_ids: []`, zero false positives), but `LIMITATIONS.md` item +28 records that it is a pure Markdown-formatting diff and +`review-code-simplicity`'s own rubric instructs unconditional omission of +formatting concerns — so a fully compliant reviewer returns clean regardless of +whether it ever reasoned about the near-miss judgment the case was built to +test. Classified **rubric ambiguity or overbreadth**, specifically a +measurement-design limitation in the control itself, not a reviewer defect: the +case cannot currently discriminate a reviewer that reasons about justification +from one that reflexively ignores all formatting. Recorded as an open item for +whoever next curates this stratum, per item 28's own recommendation; no #51–#57 +disposition depends on it. + +`compat-accessor-boundary-duplication` is a correct control with zero false +positives and `verdict_stability: 1.0` — no failure. + +## Runtime/tooling behavior + +Zero attempts across all 75 graded attempts (35 + 20 + 20) recorded any +`spawn_failure`, `timeout`, `runtime_failure`, `output_too_large`, +`malformed_output`, or `protocol_mismatch` status; every `statuses` block in +every `per_case` entry across all three reports reads `{'review_result': 5}`. +**No runtime or tooling failure occurred anywhere in this baseline.** This +bucket is closed for v1 with zero material findings, and no #51–#57 mechanism is +justified by a runtime/tooling cause. + +## The deferred connector stratum + +`frozen-configuration.json`'s `connector-escape` stratum entry is +`state: deferred_not_satisfied` with `target_skill: null` and no cost ceiling. +Zero connector-escape attempts exist anywhere in this baseline. This is not +classified as a correct control, a miss, or any other outcome — it is simply +absent evidence, and per `LIMITATIONS.md` item 1 must never be read as zero +connector-escape risk. See `DECISION-RECORD.md`'s entry for #56 for how a +separate, owner-authorized connector-outcome source is used in this v2 cycle +without retroactively touching this deferred stratum. + +## Summary rollup (acceptance criterion 1) + +| bucket | cases | +| ------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| reviewer (demonstrated failure) | `dependency-strictness-propagation`, `stale-claim-release-guard`, `process-isolation-assertion` | +| reviewer (correct, no failure) | `dependency-hint-parser-coverage`, `post-bootstrap-module-load`, `session-continuation-summary`, `reconciliation-outcome-type`, `record-status-transition-guard`, `compat-accessor-boundary-duplication` | +| evidence/rubric (measurement design) | `env-inventory-bullet-format` | +| grader/corpus ambiguity | `registry-client-layering`, `setup-service-path-gateway`, `metrics-label-formatting-duplication`, `watcher-check-policy-duplication` | +| reviewer/grader, undisentangled | `optional-tool-probe` | +| runtime/tooling | none observed | + +Every one of the 15 scored cases across the three strata appears in exactly one +row above. Only three cases — the two confident misses and the false-alarm +control — are used as evidence in `DECISION-RECORD.md`. The rest are either +correct behavior, a corpus/grader confound explicitly out of this ticket's +non-goals to fix, or an open question recorded for a future corpus-curation +ticket rather than resolved here. diff --git a/review-suite/evals/v2/README.md b/review-suite/evals/v2/README.md new file mode 100644 index 0000000..9406d05 --- /dev/null +++ b/review-suite/evals/v2/README.md @@ -0,0 +1,35 @@ +# v2 planning record + +This directory is the planning/definition gate #59 produces. It changes no +review prompt, schema, rubric, orchestration, or caller runtime behavior by +itself — it interprets the frozen `../baseline/v1/` record and turns it into an +implementation-ready decision for #51–#57. + +- [`FAILURE-TAXONOMY.md`](FAILURE-TAXONOMY.md) — classifies every material + outcome in the frozen v1 baseline by the smallest evidenced cause. +- [`DECISION-RECORD.md`](DECISION-RECORD.md) — for each proposed mechanism in + #51–#57, the baseline evidence, why a smaller change is insufficient, the + smallest intervention, exact contract/version, fixtures, scored corpus slice, + proposed thresholds, efficiency bounds, ablation/removal rule, and disposition + (retain, narrow, split, merge, defer, or close). +- [`gate-manifest.json`](gate-manifest.json) — the preregistered v2 gate + manifest: corpus/grader versions, runtime/model stratum, executor and suite + configuration, run count/timeout/retry/cost, deterministic invariants, + proposed quality/stability/non-regression thresholds, mechanism ablations, + efficiency bounds, and the rules for invalid runs, missing data, runtime + drift, and threshold changes. Committed before any scored v2 output exists; + every numeric threshold is marked as a proposal requiring owner confirmation. +- [`audits/`](audits) — the three independent graph audits #59 requires + (scope/completeness, dependency/sequencing, shovel-readiness), each recorded + as its own pass with its own findings, run against the live issue graph after + the #51–#57 body and native-edge mutations below. + +## What this is not + +This record does not run any scored v2 evaluation. `gate-manifest.json` +preregisters the configuration a future scored run (owned by #57) must use; no +case in this directory has been executed against a real reviewer. Every +threshold here is explicitly a proposal, not a measured result. + +It also does not modify `../baseline/v1/` in any way. That record stays frozen +exactly as merged in #58. diff --git a/review-suite/evals/v2/gate-manifest.json b/review-suite/evals/v2/gate-manifest.json new file mode 100644 index 0000000..1bd2e72 --- /dev/null +++ b/review-suite/evals/v2/gate-manifest.json @@ -0,0 +1,171 @@ +{ + "manifest_version": "1.0", + "status": "preregistered_before_scoring", + "status_detail": "Committed before any scored v2 output exists, per #59's own validation/delivery boundary. Every threshold below is a proposal grounded in the frozen v1 baseline's actual small-sample uncertainty (LIMITATIONS.md item 36) and must be confirmed or replaced by the repository owner before #57 treats it as a pass/fail gate. This manifest binds only #52 and #53's mechanisms; #54, #55, and #57 remain deferred and add nothing here until their own evidence exists.", + "predecessor_baseline": { + "v1_frozen_configuration": "review-suite/evals/baseline/v1/frozen-configuration.json", + "v1_review_behaviour_commit": "16560d807c66076fcbf3f00d3a87f543c6ae2458", + "v1_scoring_suite_commit": "2644c12", + "decision_record": "review-suite/evals/v2/DECISION-RECORD.md", + "failure_taxonomy": "review-suite/evals/v2/FAILURE-TAXONOMY.md" + }, + "corpus_and_grader_versions": { + "grader_version": "1.0", + "grader_version_note": "Unchanged from v1. The one-token surface-match-strength limitation (LIMITATIONS.md item 4) and the containment-only formulation matching (item 3) are known open issues escalated to #59 by name, but no #51-#57 child owns a grader.py change and no baseline evidence assigns one. Not resolved by this manifest; carried forward explicitly as unresolved rather than silently dropped. A future ticket must adopt it before grader.py changes.", + "grading_method": "three-way: matched, missed, or referred for adjudication; never calibrate a scored case on its own prose first (LIMITATIONS.md item 14, unchanged for v2)", + "corpus_version_scheme": "v2 corpus versions for s1-correctness-orchestrator use the suffix '-v2-', e.g. '0.2-s1-v2-traversal', so a re-scored stratum is never confused with its v1 '0.1-s1-populated' predecessor. s2 and s3 corpus versions are unchanged for this cycle: no #51-#57 mechanism in this manifest touches review-solution-simplicity or review-code-simplicity prompts." + }, + "runtime_model_stratum": { + "runtime": "claude", + "runtime_cli_version": "2.1.92", + "model": "claude-opus-4-6[1m]", + "stratum_rule": "Unchanged from v1: a change of runtime, runtime version, or model creates a new stratum and must never be compared with this manifest's thresholds. If the runtime/model available at #57's execution time differs from this pin, #57 must return blocked and this manifest must be revised and re-preregistered before scoring, not silently reinterpreted." + }, + "executor_and_suite_configuration": { + "executor_command": "python3 review-suite/scripts/evals/claude_executor.py", + "invocation_template": "just eval-review-suite '' --corpus review-suite/evals/strata/ --runs --timeout --artifact-dir review-suite/evals/artifacts//- --attempts-out review-suite/evals/artifacts//-.attempts.jsonl --report-out ", + "process_isolation": "one fresh executor process per attempt, unchanged from v1" + }, + "run_count_timeout_retry_cost": { + "runs_per_case": { + "value": 5, + "rationale_proposal": "Kept identical to v1 so s1's re-scored cases remain comparable to their v1 counterparts across the same stratum boundary. LIMITATIONS.md item 36 documents that 5 runs is a small-sample point estimate, not a measured capability; a larger runs_per_case would tighten every stability and recall estimate below, but changing it now would also break v1-to-v2 comparability for the two target cases. Recorded as a genuine judgment call: this manifest chooses comparability over precision, and the owner may prefer the opposite trade." + }, + "retry_policy": "none, unchanged from v1 — a failed attempt is an evaluation failure, never retried", + "timeout_seconds": { + "s1-correctness-orchestrator": { + "value": 300, + "conditional_raise_proposal": "If a pilot run of the two added review-correctness passes (traversal, verification-sufficiency) approaches 300s on the orchestrator stratum's larger packet, raise to 450s before the scored run, not after seeing scored output. Proposal only; not yet observed to be necessary." + }, + "s2-solution-simplicity-lens": { "value": 300, "note": "unchanged; no #51-#57 mechanism in this manifest touches this stratum's prompts" }, + "s3-code-simplicity-lens": { "value": 300, "note": "unchanged; no #51-#57 mechanism in this manifest touches this stratum's prompts" } + }, + "cost_ceiling_usd_proposal": { + "s1-correctness-orchestrator": { + "v1_ceiling": 9.0, + "v2_proposed_ceiling": 12.0, + "rationale": "Two added reasoning passes (consumer/impact traversal, verification-sufficiency) are expected to increase output tokens and cost. Sized from the v1 pilot's worst observed cold cost per attempt ($0.2021) times the same 35-attempt run, with headroom for larger per-attempt output; not derived from any actual v2 measurement, since none exists yet. Proposal requiring owner preregistration before #57 spends against it, exactly as the v1 ceiling required owner preregistration in #58." + }, + "s2-solution-simplicity-lens": { "v1_ceiling": 3.0, "v2_proposed_ceiling": 3.0, "note": "unchanged; no mechanism in this manifest touches this stratum" }, + "s3-code-simplicity-lens": { "v1_ceiling": 3.0, "v2_proposed_ceiling": 3.0, "note": "unchanged; no mechanism in this manifest touches this stratum" }, + "cost_ceiling_exhaustion_policy": "unchanged from v1: exceeding a stratum's preregistered ceiling stops further runs in that stratum and records an incomplete baseline for it; repetitions are never reduced after outputs are visible" + } + }, + "deterministic_invariants": [ + { + "id": "invariant.v1_1-validation-verdict-crosscheck", + "owner": "#51", + "schema_version": "review-result.schema.json 1.1", + "statement": "No `clean` aggregate may pair with any required focused/full validation entry whose status is `failed` or `unavailable`.", + "verified_by": "canonical deterministic fixtures, not scored corpus runs" + }, + { + "id": "invariant.v1_1-current-head-lens-evidence", + "owner": "#51", + "schema_version": "review-result.schema.json 1.1", + "statement": "`clean` requires exactly one fresh, current-head/current-base execution from each of the three required lenses; no old-head result may contribute.", + "verified_by": "canonical deterministic fixtures, not scored corpus runs" + }, + { + "id": "invariant.v1_2-consumer-impact-evidence-structure", + "owner": "#52", + "schema_version": "review-result.schema.json 1.2", + "statement": "When a changed shared symbol/contract has other call sites, `clean` requires a non-empty, structurally valid `consumer_impact_evidence` entry for it, or an explicitly evidenced `no_other_consumers` disposition.", + "verified_by": "canonical deterministic fixtures, not scored corpus runs" + }, + { + "id": "invariant.v1_3-verification-sufficiency-evidence-structure", + "owner": "#53", + "schema_version": "review-result.schema.json 1.3", + "statement": "For every claimed validation command/test touching a materially risky change, `clean` requires a `verification_sufficiency_evidence` entry recording whether it exercises the material risk.", + "verified_by": "canonical deterministic fixtures, not scored corpus runs" + }, + { + "id": "invariant.canonical-bundled-byte-identical", + "owner": "all schema-touching children (#51, #52, #53)", + "statement": "Canonical and bundled review-suite contract copies remain byte-identical after `just sync-contracts`.", + "verified_by": "existing bundled-contract drift test" + } + ], + "quality_stability_non_regression_thresholds": { + "note": "Proposals only. All figures below are judgment calls sized from a 5-attempts-per-case sample (LIMITATIONS.md item 36) and require explicit owner confirmation before #57 gates on them.", + "target_cases": [ + { + "case_id": "dependency-strictness-propagation", + "stratum": "s1-correctness-orchestrator", + "owner": "#52 (schema) + #53 (traversal pass)", + "v1_mean_recall": 0.0, + "v2_proposed_quality_gate": "mean_recall >= 0.6 (>= 3 of 5 attempts matched), or combined matched+referred rate >= 0.8 with referred attempts showing genuine content overlap with the expected root cause", + "v2_proposed_stability_floor": "verdict_stability and finding_stability must not fall below the v1 floor of 1.0" + }, + { + "case_id": "stale-claim-release-guard", + "stratum": "s1-correctness-orchestrator", + "owner": "#53 (verification-sufficiency pass)", + "v1_mean_recall": 0.0, + "v2_proposed_quality_gate": "mean_recall >= 0.6 (>= 3 of 5 attempts matched), or combined matched+referred rate >= 0.8 with referred attempts showing genuine content overlap with the expected root cause", + "v2_proposed_stability_floor": "verdict_stability and finding_stability must not fall below the v1 floor of 1.0" + } + ], + "non_regression_floors": [ + { + "cases": ["dependency-hint-parser-coverage", "post-bootstrap-module-load", "session-continuation-summary"], + "requirement": "false_positive_attempts stays at 0 for each; no new gating finding introduced by the two added passes" + }, + { + "case": "process-isolation-assertion", + "requirement": "false_alarm_attempts must not exceed its v1 value of 2 of 5; improvement is welcome but not required" + }, + { + "case": "optional-tool-probe", + "requirement": "recorded as an open question in FAILURE-TAXONOMY.md, not gated here; re-scoring must not regress its v1 mean_recall of 0.2 but no target above that is set, since its v1 shape is not attributed to a mechanism this manifest addresses" + }, + { + "stratum": "s2-solution-simplicity-lens and s3-code-simplicity-lens", + "requirement": "all 8 cases must replay with identical statuses to their v1 report, since no mechanism in this manifest touches either lens's prompt or schema consumption; any change here is itself a gate failure requiring investigation before #57 proceeds" + } + ], + "correlated_pair_caveat": "registry-client-layering (s2) and metrics-label-formatting-duplication (s3) remain correlated, not independent (LIMITATIONS.md item 32). Any future aggregate figure spanning both strata must disclose this and must not report their combined recall as two independent trials." + }, + "mechanism_ablations": [ + { + "mechanism": "#52 consumer_impact_evidence schema + validator requirement", + "ablation_design": "Deterministic, not stochastic: replay the dependency-strictness-propagation-shaped fixture against the v1.1 validator (no requirement) and the v1.2 validator (requirement present). The v1.1 validator must accept the fixture without evidence; the v1.2 validator must reject it. This proves the structural gate does what it claims before any stochastic run is spent on it.", + "removal_rule": "If, after #53's traversal pass is scored, dependency-strictness-propagation does not move per the quality gate above and no other case benefits from the evidence array, remove the schema requirement as unjustified complexity." + }, + { + "mechanism": "#53 consumer/impact-traversal pass", + "ablation_design": "Run s1-correctness-orchestrator with only the traversal pass enabled (verification-sufficiency pass disabled/no-op) for 5 runs against the full stratum.", + "removal_rule": "Remove if dependency-strictness-propagation does not move per the quality gate above in this isolated configuration." + }, + { + "mechanism": "#53 verification-sufficiency pass", + "ablation_design": "Run s1-correctness-orchestrator with only the verification-sufficiency pass enabled (traversal pass disabled/no-op) for 5 runs against the full stratum.", + "removal_rule": "Remove if stale-claim-release-guard does not move per the quality gate above in this isolated configuration." + }, + { + "mechanism": "#53 both passes combined", + "ablation_design": "Run s1-correctness-orchestrator with both passes enabled for 5 runs against the full stratum (this is the configuration actually shipped if both individual ablations pass).", + "removal_rule": "Ship only if both individual ablations above independently justify their own pass; a combined-only improvement with either isolated ablation failing is not sufficient, per #57's unique-contribution requirement." + } + ], + "efficiency_bounds": { + "note": "See run_count_timeout_retry_cost above for the authoritative cost/timeout proposals. This section restates the ceiling rule for clarity.", + "rule": "Exceeding any stratum's preregistered cost ceiling or timeout stops further scored runs in that stratum. A ceiling is never raised after scored output for that stratum has been examined; raising it requires a new manifest version preregistered before the next run." + }, + "invalid_runs_missing_data_drift_and_threshold_change_rules": { + "invalid_run": "An attempt classified spawn_failure, timeout, runtime_failure, output_too_large, malformed_output, or protocol_mismatch is never graded and never silently retried, per the unchanged v1 retry_policy.", + "missing_data": "A stratum that cannot complete its full runs_per_case x cases matrix within its cost ceiling is reported as an incomplete baseline for that stratum, not padded or extrapolated.", + "runtime_drift": "A runtime, runtime-version, or model change from runtime_model_stratum above creates a new, non-comparable stratum. #57 must return blocked rather than score against this manifest's thresholds if drift is detected before scoring.", + "threshold_change": "Every threshold in quality_stability_non_regression_thresholds is frozen the moment any scored v2 output for the affected stratum is examined. A threshold change after that point voids the affected comparison and requires a new manifest version (manifest_version bump) preregistered before any further scored run, exactly as this manifest itself was preregistered before v2 scoring begins. This is the mechanical enforcement of #59's own non-goal against tuning gates after seeing results." + }, + "scope_boundary": { + "mechanisms_bound_by_this_manifest": ["#51", "#52", "#53"], + "mechanisms_explicitly_out_of_scope": [ + "#54 (deferred; no evidence yet to bind a gate to)", + "#55 (deferred; downstream of #54)", + "#56 (evidence-backed but not itself scored against s1/s2/s3 by this manifest; see DECISION-RECORD.md)", + "#57 (executes against this manifest; does not itself add mechanisms here)" + ] + } +} From 94a42379e3c9c119e2ab176ca632fc47dd49814f Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Tue, 28 Jul 2026 14:17:09 -0700 Subject: [PATCH 2/4] fix: cite provenance for the #56 connector-source authorization claim ## Summary - `DECISION-RECORD.md`'s #56 entry now states plainly that the connector-source authorization was communicated as a session-level owner instruction during this epic's delegated handoff, not (yet) as a citable tracker comment, unlike every other owner adjudication cited elsewhere in this record. - Add an explicit reconciliation with `LIMITATIONS.md` item 1's recorded exclusion of a different private connector repository from #58's frozen v1 corpus: that exclusion is scoped to the v1 corpus specifically and is unchanged; this is a separate, later-granted permission scoped to #56's own v2-cycle workflow. ## Why - review-code-change's initial pass on this candidate returned `changes_required`: the authorization claim was asserted as settled fact with no citable trail, in unacknowledged tension with the v1 baseline's own recorded exclusion of the same class of material. --- review-suite/evals/v2/DECISION-RECORD.md | 34 ++++++++++++++++++------ 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/review-suite/evals/v2/DECISION-RECORD.md b/review-suite/evals/v2/DECISION-RECORD.md index ecf4f0d..a156e2e 100644 --- a/review-suite/evals/v2/DECISION-RECORD.md +++ b/review-suite/evals/v2/DECISION-RECORD.md @@ -320,14 +320,32 @@ basis and two small guardrails. - **Baseline cases and metrics demonstrating the problem.** A real connector-outcome dataset exists and the repository owner has authorized its - use for v2-cycle analysis. Per the owner's explicit instruction, **the source - repository is described here only generically — "a private, owner-authorized - connector-review source" — and is never named in this record, in #56's revised - body, or in any other public artifact.** This record does not attempt to - characterize the dataset's contents beyond that description; doing so is out - of scope for this ticket and, per the owner's own boundary, would require - stopping and returning `blocked` rather than searching for or inferring the - source. + use for v2-cycle analysis. **Provenance of this claim, stated plainly so it is + auditable rather than implied:** this authorization was communicated directly + to this ticket's implementing session by the repository owner, as part of this + epic's delegated handoff, and is not yet recorded as a GitHub comment or other + tracker artifact the way every other owner adjudication cited elsewhere in + this record is (compare `LIMITATIONS.md` items 23 and 33, each of which cites + a specific comment URL). Unlike those, this authorization has no citable + tracker trail today. It is recorded here as a session-level owner instruction, + not fabricated or inferred, and the repository owner should add a + tracker-visible confirmation (a comment on #56 or #59) if a durable audit + trail is wanted before this disposition is acted on further. +- **Reconciling this with the v1 baseline's recorded exclusion.** + `LIMITATIONS.md` item 1 records that a different private repository carrying + connector review history "was identified and deliberately excluded on + third-party authority and disclosure grounds" from **#58's frozen v1 corpus**, + and "was not read, and nothing here derives from it." That exclusion is scoped + to what #58 curates into `review-suite/evals/baseline/v1/` — a specific, + frozen, scored corpus with its own sourcing and retention rules — and stays + exactly as decided; this decision record does not reopen, reverse, or + contradict it. The authorization described above is a separate, later-granted + permission scoped only to #56's own v2-cycle intake/analysis workflow, not to + populating, rescoring, or otherwise touching the v1 corpus. Whether the two + describe the same underlying source repository or two different ones is + deliberately not stated here in either direction — doing so would risk + narrowing the generic description into something identifying, which the + owner's disclosure boundary forbids regardless of the answer. - **Explicit non-retroactivity note.** This source is a v2-cycle addition. It does **not** retroactively modify, extend, or rescore `review-suite/evals/baseline/v1/`, which stays exactly as merged and frozen in From 6d6a74b132822622a505136da1a4095cae10c781 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Tue, 28 Jul 2026 14:29:07 -0700 Subject: [PATCH 3/4] fix: remove a self-contradiction and a miscited item from the #56 reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - The #56 reconciliation bullet no longer calls the item-1-excluded repository "different" while separately claiming to leave the same-or-different question open in either direction; it now takes no position on that question at all, matching its own stated reason for staying silent. - The provenance bullet now cites only LIMITATIONS.md item 33 for "cites a specific comment URL" — item 23 makes no such claim and carries no URL. ## Why - The second review-code-change pass on this candidate returned `changes_required`: the previous fix introduced a self-contradiction (one sentence asserted the sources are different; the next sentence claimed that question was left open) and an overstated citation. --- review-suite/evals/v2/DECISION-RECORD.md | 41 ++++++++++++------------ 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/review-suite/evals/v2/DECISION-RECORD.md b/review-suite/evals/v2/DECISION-RECORD.md index a156e2e..19d9aba 100644 --- a/review-suite/evals/v2/DECISION-RECORD.md +++ b/review-suite/evals/v2/DECISION-RECORD.md @@ -325,27 +325,28 @@ basis and two small guardrails. to this ticket's implementing session by the repository owner, as part of this epic's delegated handoff, and is not yet recorded as a GitHub comment or other tracker artifact the way every other owner adjudication cited elsewhere in - this record is (compare `LIMITATIONS.md` items 23 and 33, each of which cites - a specific comment URL). Unlike those, this authorization has no citable - tracker trail today. It is recorded here as a session-level owner instruction, - not fabricated or inferred, and the repository owner should add a - tracker-visible confirmation (a comment on #56 or #59) if a durable audit - trail is wanted before this disposition is acted on further. + this record is (compare `LIMITATIONS.md` item 33, which cites two specific + `github.com/.../issuecomment-...` URLs for the owner's adjudications). Unlike + that, this authorization has no citable tracker trail today. It is recorded + here as a session-level owner instruction, not fabricated or inferred, and the + repository owner should add a tracker-visible confirmation (a comment on #56 + or #59) if a durable audit trail is wanted before this disposition is acted on + further. - **Reconciling this with the v1 baseline's recorded exclusion.** - `LIMITATIONS.md` item 1 records that a different private repository carrying - connector review history "was identified and deliberately excluded on - third-party authority and disclosure grounds" from **#58's frozen v1 corpus**, - and "was not read, and nothing here derives from it." That exclusion is scoped - to what #58 curates into `review-suite/evals/baseline/v1/` — a specific, - frozen, scored corpus with its own sourcing and retention rules — and stays - exactly as decided; this decision record does not reopen, reverse, or - contradict it. The authorization described above is a separate, later-granted - permission scoped only to #56's own v2-cycle intake/analysis workflow, not to - populating, rescoring, or otherwise touching the v1 corpus. Whether the two - describe the same underlying source repository or two different ones is - deliberately not stated here in either direction — doing so would risk - narrowing the generic description into something identifying, which the - owner's disclosure boundary forbids regardless of the answer. + `LIMITATIONS.md` item 1 records that a private repository carrying connector + review history "was identified and deliberately excluded on third-party + authority and disclosure grounds" from **#58's frozen v1 corpus**, and "was + not read, and nothing here derives from it." That exclusion is scoped to what + #58 curates into `review-suite/evals/baseline/v1/` — a specific, frozen, + scored corpus with its own sourcing and retention rules — and stays exactly as + decided; this decision record does not reopen, reverse, or contradict it. The + authorization described above is a separate, later-granted permission scoped + only to #56's own v2-cycle intake/analysis workflow, not to populating, + rescoring, or otherwise touching the v1 corpus. This record takes no position, + in either direction, on whether the two involve the same underlying source + repository — stating that, either way, would risk narrowing the generic + description into something identifying, which the owner's disclosure boundary + forbids regardless of the answer. - **Explicit non-retroactivity note.** This source is a v2-cycle addition. It does **not** retroactively modify, extend, or rescore `review-suite/evals/baseline/v1/`, which stays exactly as merged and frozen in From cc58fc91a5d67d27c15743dce51dd42f5358cdc3 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Tue, 28 Jul 2026 14:46:55 -0700 Subject: [PATCH 4/4] fix: stop overclaiming completed work not yet done by this candidate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - DECISION-RECORD.md's #57 entry no longer states the stale #58 cross-reference "is corrected" in #57's body; it now says the correction is pending, to be made when this ticket's later live issue-body edit step runs, not by this documentation-only repository candidate. - The "Net effect on the dependency graph" section no longer claims #51's `blockedBy #59` edge is already removed; it now describes the actual planned behavior (leave the edge in place, matching #58's own still-present edge to closed #50) and marks the predicted net effect as something the later graph audits verify, not an already-confirmed result. - README.md (v2/ and evals/) no longer describes `audits/` as delivered content; it is now explicitly marked as not yet produced, and the dangling "native-edge mutations below" and `audits/dependency-sequencing-audit.md` references are removed or reworded to point at real, current evidence only. ## Why - The third review-code-change pass on this candidate found two related overclaims: prose asserting work as already-done (a ticket-body correction, an audit read-back, edge removal) that in fact has not happened yet and is explicitly out of scope for this documentation-only candidate. Both are the same failure class the first review round already caught once in the #56 section — settled-fact framing for something not yet verifiable. --- review-suite/evals/README.md | 2 +- review-suite/evals/v2/DECISION-RECORD.md | 40 +++++++++++++++--------- review-suite/evals/v2/README.md | 9 +++--- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/review-suite/evals/README.md b/review-suite/evals/README.md index de89843..72153b7 100644 --- a/review-suite/evals/README.md +++ b/review-suite/evals/README.md @@ -44,7 +44,7 @@ review-suite/evals/ │ ├── FAILURE-TAXONOMY.md every v1 material outcome, classified │ ├── DECISION-RECORD.md per-mechanism evidence and disposition │ ├── gate-manifest.json preregistered v2 gates, before scoring -│ └── audits/ the three independent graph audits +│ └── audits/ the three graph audits (later step, empty for now) └── artifacts/ opt-in captured output, not in git review-suite/scripts/evals/ diff --git a/review-suite/evals/v2/DECISION-RECORD.md b/review-suite/evals/v2/DECISION-RECORD.md index 19d9aba..50bef99 100644 --- a/review-suite/evals/v2/DECISION-RECORD.md +++ b/review-suite/evals/v2/DECISION-RECORD.md @@ -404,21 +404,33 @@ basis and two small guardrails. **Disposition: defer, unchanged.** Downstream of #55 and #56; #57's own "Definition status" already names #59 as its gate for the exact scored-v2 configuration, which this record and `gate-manifest.json` provide. One prose -correction is in scope: #57's current body reads "Run the evidence-backed v2 -experiment approved by #58" in its Goal section. #58 curated the frozen v1 -corpus and baseline; it is #59 — this ticket — that approves the v2 mechanism -set and gate manifest #57 executes against. That single stale cross-reference is -corrected in #57's revised body; #57's disposition, required inputs, and +correction is in scope, still pending as of this record: #57's live body +currently reads "Run the evidence-backed v2 experiment approved by #58" in its +Goal section. #58 curated the frozen v1 corpus and baseline; it is #59 — this +ticket — that approves the v2 mechanism set and gate manifest #57 executes +against. That single stale cross-reference **must be corrected in #57's body +when this ticket's later live issue-body edit step is carried out** — not by +this repository candidate, which changes no live issue body itself, per #59's +own validation/delivery boundary; #57's disposition, required inputs, and acceptance criteria are otherwise unchanged. ## Net effect on the dependency graph -Applying the dispositions above to the native graph: `blockedBy #59` is removed -from #51 once #59 closes, leaving #51 with no open `blockedBy` edge. Every other -child (#52 through #57) keeps its existing `blockedBy` edge onto its current -predecessor (#52→#51, #53→#52, #54→#53, #55→#54, #56→#52, #57→#55,#56), all of -which stay open. **#51 is therefore the only open issue in this epic with zero -open blockers once #59 closes** — the expected net effect #59's own text names. -See `audits/dependency-sequencing-audit.md` for the verified read-back -confirming this against the live graph after the edge and body mutations are -applied. +Applying the dispositions above to the native graph: #51's existing +`blockedBy #59` edge is left in place rather than removed, matching this +repository's own established convention — #58's `blockedBy #50` edge was +likewise left in place after #50 closed (verified live: `gh api graphql` still +returns it today). A `blockedBy` edge onto a closed issue is inert rather than +deleted; #51 has no *open* blocker once #59 closes. Every other child (#52 +through #57) keeps its existing `blockedBy` edge onto its current predecessor +(#52→#51, #53→#52, #54→#53, #55→#54, #56→#52, #57→#55,#56), all of which stay +open. **#51 is therefore the only open issue in this epic with zero open +blockers once #59 closes** — the expected net effect #59's own text names. + +This is a prediction to be checked, not yet a verified result: the graph audits +required by #59's own sequencing (scope/completeness, dependency/sequencing, +shovel-readiness) run only after the live `#51`–`#57` body and edge changes +below are actually applied, as their own later step. Their findings — including +the read-back that confirms or corrects this predicted net effect — belong in +`audits/` once that step completes; no file exists there yet in this repository +candidate, and none is claimed to. diff --git a/review-suite/evals/v2/README.md b/review-suite/evals/v2/README.md index 9406d05..36d5ad0 100644 --- a/review-suite/evals/v2/README.md +++ b/review-suite/evals/v2/README.md @@ -19,10 +19,11 @@ implementation-ready decision for #51–#57. efficiency bounds, and the rules for invalid runs, missing data, runtime drift, and threshold changes. Committed before any scored v2 output exists; every numeric threshold is marked as a proposal requiring owner confirmation. -- [`audits/`](audits) — the three independent graph audits #59 requires - (scope/completeness, dependency/sequencing, shovel-readiness), each recorded - as its own pass with its own findings, run against the live issue graph after - the #51–#57 body and native-edge mutations below. +- `audits/` — **not yet produced by this repository candidate.** #59's own + sequencing runs the three independent graph audits (scope/completeness, + dependency/sequencing, shovel-readiness) only after the live `#51`–`#57` body + and native-edge mutations are applied as a later, separate step; each pass's + findings will be recorded here, as its own file, once that step completes. ## What this is not