From 43deec617ee06e22e1a937234eea2a4d99b5d836 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Mon, 27 Jul 2026 08:44:20 -0700 Subject: [PATCH 1/5] feat: populate the correctness stratum with seven adjudicated cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch 2 of #58's corpus population. Populates `s1-correctness-orchestrator` with the seven case classes #58 requires, every one minimized from a real review disposition in the public `shaug/atelier` or in this repository's own delivery history. Three accepted material escapes: - `dependency-strictness-propagation` — a decision helper gains a strict mode, one caller adopts it, and the untouched sibling caller keeps deciding the same question permissively. atelier PR 335, accepted in `5cb0333`. - `stale-claim-release-guard` — a collect-then-apply guard compares ownership only when the captured owner was present, so the unowned-at-capture interleaving bypasses it and releases a live claim. atelier PR 373, accepted. - `optional-tool-probe` — an optional-tool probe checks only the return code, but a missing executable raises, so the intended skip never happens in the one environment it exists for. This repository's `f544aa0`, which survived an aggregate `clean` review verdict and was then caught by CI. Four clean-expected controls, all built to the owner's settled standard: - `session-continuation-summary` — a summary flag challenged as inaccurate, kept on the merits because it is the loop-continuation signal, with only the operator wording corrected. atelier PR 486. - `dependency-hint-parser-coverage` — a coverage request for a deliberately retired alias, declined on the merits. atelier PR 279, declined in reply 2862037362. - `post-bootstrap-module-load` — a deferred module load flagged as a strange pattern, adjudicated intentional with a comment as the whole resolution. atelier PR 710. - `process-isolation-assertion` — a test whose name overstates what it proves, raised by a real review and dispositioned `defer`. This repository's #50, item 4 of PR #61's preserved deferred list. Five cases carry multi-file diffs, and three of them are only decidable by reading a consumer the diff does not touch, which satisfies #58's multi-file minimum. The owner settled the standard: a clean control must be a case where a finding was actually raised and dispositioned as not material. The recorded rejection is the evidence, and a reviewer that re-raises it is charged a false alarm. Each clean case therefore records its rejected concern as an accepted non-finding with the formulations a reviewer would use, so a non-gating mention is tolerated while a gating one is charged. The rejected concern is deliberately *not* treated as a root cause: raising it is the error being measured. Two previously identified clean-control candidates were dropped for failing this standard, and two more that looked usable were dropped after checking their dispositions; the sourcing record names all four and why. The stratum declares `scored: false` and every expectation declares `calibrated: false`, both honestly. Nothing here may be scored yet, for two independent reasons: no expectation has a second independent adjudication, and none is calibrated. Batch 1 measured what an uncalibrated expectation reports — recall 0.0 against a reviewer that found the defect on every attempt — so scoring this stratum as it stands would produce a number about the corpus rather than about the reviewer. No case in this stratum has been run through any runtime. They are unobserved, and must stay that way until the calibration-versus-result-blindness question is settled. --- CHANGELOG.md | 4 +- .../s1-correctness-orchestrator/corpus.json | 27 +++++ .../dependency-hint-parser-coverage.json | 28 ++++++ .../dependency-strictness-propagation.json | 34 +++++++ .../expectations/optional-tool-probe.json | 34 +++++++ .../post-bootstrap-module-load.json | 28 ++++++ .../process-isolation-assertion.json | 28 ++++++ .../session-continuation-summary.json | 28 ++++++ .../stale-claim-release-guard.json | 42 ++++++++ .../dependency-hint-parser-coverage.json | 9 ++ .../dependency-strictness-propagation.json | 9 ++ .../provenance/optional-tool-probe.json | 9 ++ .../post-bootstrap-module-load.json | 9 ++ .../process-isolation-assertion.json | 9 ++ .../session-continuation-summary.json | 9 ++ .../provenance/stale-claim-release-guard.json | 9 ++ .../reviewer/PROMPT.md | 7 ++ .../packet.json | 85 ++++++++++++++++ .../packet.json | 91 +++++++++++++++++ .../reviewer/optional-tool-probe/packet.json | 86 ++++++++++++++++ .../post-bootstrap-module-load/packet.json | 85 ++++++++++++++++ .../process-isolation-assertion/packet.json | 84 ++++++++++++++++ .../session-continuation-summary/packet.json | 98 +++++++++++++++++++ .../stale-claim-release-guard/packet.json | 94 ++++++++++++++++++ 24 files changed, 945 insertions(+), 1 deletion(-) create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/corpus.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-hint-parser-coverage.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-strictness-propagation.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/optional-tool-probe.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/post-bootstrap-module-load.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/session-continuation-summary.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/stale-claim-release-guard.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/PROMPT.md create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 810a6d6..d13224c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ summary: Chronological history of repository and skill changes. # Changelog -## 2026-07-27 — Added successor-source recovery for corrected carved suffixes +## 2026-07-27 — Populated the correctness stratum, and recovered carved suffixes +- feat: populate the correctness stratum with seven adjudicated cases - feat: recover corrected carved suffixes + (`ba12e0744a938fc71af16eeeaa0eea98e7c2c63e`) ## 2026-07-26 — Added the replay evaluator, then froze the v1 baseline configuration diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json b/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json new file mode 100644 index 0000000..1e3b7ff --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json @@ -0,0 +1,27 @@ +{ + "corpus_version": "0.1-s1-populated", + "protocol_version": "1.0", + "grader_version": "1.0", + "target_skill": "review-code-change", + "target_skill_dependencies": [ + "review-solution-simplicity", + "review-correctness", + "review-code-simplicity" + ], + "stratum": { + "id": "s1-correctness-orchestrator", + "ground_truth": "human-review", + "scored": false, + "grading_is_signal": true, + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Populated but NOT yet scored: every expectation is uncalibrated and none has a second independent adjudication, and a baseline captured on either would report a number about the corpus rather than about the reviewer. Flip `scored` to true only once both land." + }, + "cases": [ + "dependency-hint-parser-coverage", + "dependency-strictness-propagation", + "optional-tool-probe", + "post-bootstrap-module-load", + "process-isolation-assertion", + "session-continuation-summary", + "stale-claim-release-guard" + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-hint-parser-coverage.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-hint-parser-coverage.json new file mode 100644 index 0000000..4fcca90 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-hint-parser-coverage.json @@ -0,0 +1,28 @@ +{ + "expectation_version": "1.0", + "case_id": "dependency-hint-parser-coverage", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "clean", + "material_root_causes": [], + "accepted_non_findings": [ + { + "id": "anf.retired-alias-coverage-request", + "description": "A reviewer may ask for coverage that locks one of the retired alias variants, or for the aliases to be kept for compatibility. This is the finding this case is built from, and it was adjudicated NOT material: the parser was deliberately narrowed to the canonical shape, so a regression path for a retired alias would pin behaviour the change exists to remove. Tolerated as an observation; gating on it is a false alarm.", + "equivalent_formulations": [ + "add a regression case for the type variant", + "keep the retired aliases for compatibility", + "coverage for the alias shapes is missing", + "the parser should still accept relation or linkType" + ] + }, + { + "id": "anf.missing-link-kind-silently-ignored", + "description": "An entry whose link kind is absent is skipped without comment. Worth noting; the packet states every entry comes from a live export that always emits the canonical field.", + "equivalent_formulations": [ + "an entry with no link kind is silently skipped", + "no error for a missing link type" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-strictness-propagation.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-strictness-propagation.json new file mode 100644 index 0000000..30811da --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/dependency-strictness-propagation.json @@ -0,0 +1,34 @@ +{ + "expectation_version": "1.0", + "case_id": "dependency-strictness-propagation", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.sibling-call-site-keeps-permissive-mode", + "requirement": "Every path that decides whether a record is integrated applies the same strictness rule for a closed record.", + "trigger": "A closed record that carries a merge marker but no target ref reaches the dependency gate.", + "surface": "dependency_finalized", + "consequence": "The change hardens one of the two callers and leaves the other calling the same helper in its permissive mode, so the abandoned-merge record the change exists to catch is still reported as finalized on that path and still releases the work that follows it. The added tests cover the hardened caller only, so the suite is green.", + "severity": "blocking", + "equivalent_formulations": [ + "dependency_finalized still calls integration_proven without the strict flag", + "the sibling call site was not updated", + "one caller keeps the permissive mode", + "a closed dependency can still be treated as finalized", + "the strictness rule is applied inconsistently across call sites" + ] + } + ], + "accepted_non_findings": [ + { + "id": "anf.keyword-only-flag-shape", + "description": "A reviewer may prefer a distinct function over a boolean mode flag. Reasonable as design commentary; the packet's preserved-behaviour list keeps the permissive result reachable, so a mode flag satisfies the contract.", + "equivalent_formulations": [ + "prefer a separate function to a boolean flag", + "a mode flag is a design smell" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/optional-tool-probe.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/optional-tool-probe.json new file mode 100644 index 0000000..921736e --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/optional-tool-probe.json @@ -0,0 +1,34 @@ +{ + "expectation_version": "1.0", + "case_id": "optional-tool-probe", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.probe-checks-status-not-absence", + "requirement": "The suite skips these tests, rather than failing, when the runner is absent.", + "trigger": "The suite runs on a machine with no task runner installed, which the packet states is exactly what CI is.", + "surface": "setUpClass", + "consequence": "Looking only at the return code cannot detect an absent executable, because the call raises instead of returning. The intended skip therefore never happens and the error escapes `setUpClass`, failing the whole class in the one environment the skip exists for. Local validation passes because the tool is installed there, so the suite is green on the machine that cannot reproduce the condition.", + "severity": "blocking", + "equivalent_formulations": [ + "a missing executable raises rather than returning a status", + "subprocess.run raises FileNotFoundError", + "the probe only checks the return code", + "OSError is not caught", + "the skip never happens when the tool is absent" + ] + } + ], + "accepted_non_findings": [ + { + "id": "anf.probe-not-shared", + "description": "The packet names an existing optional-tool probe in another test module, so a reviewer may reasonably ask for one shared helper. A real reuse observation, not the reason this candidate fails.", + "equivalent_formulations": [ + "share one optional-tool probe helper", + "the probe duplicates the existing one" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/post-bootstrap-module-load.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/post-bootstrap-module-load.json new file mode 100644 index 0000000..ab20aa9 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/post-bootstrap-module-load.json @@ -0,0 +1,28 @@ +{ + "expectation_version": "1.0", + "case_id": "post-bootstrap-module-load", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "clean", + "material_root_causes": [], + "accepted_non_findings": [ + { + "id": "anf.importlib-instead-of-a-plain-import", + "description": "A reviewer may observe that loading a module through `importlib.import_module` instead of a plain `import` is an unusual pattern and ask for it to be normalised. This is the finding this case is built from, and it was adjudicated NOT material: the deferral is load-bearing, a plain import cannot express it, and the resolution was to explain the pattern in a comment while leaving behaviour untouched. Tolerated as an observation; gating on it is a false alarm.", + "equivalent_formulations": [ + "use a standard import instead of importlib", + "importlib.import_module is a strange pattern here", + "this should be a normal module-level import", + "the dynamic import is unnecessary indirection" + ] + }, + { + "id": "anf.module-level-side-effect", + "description": "The script performs runtime selection at import time. Real, and worth noting for an entry script; the packet's contract makes that ordering the point of the change rather than an accident.", + "equivalent_formulations": [ + "runtime selection happens at import time", + "module-level side effect on import" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json new file mode 100644 index 0000000..8a2fe26 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json @@ -0,0 +1,28 @@ +{ + "expectation_version": "1.0", + "case_id": "process-isolation-assertion", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "clean", + "material_root_causes": [], + "accepted_non_findings": [ + { + "id": "anf.name-asserts-identity-not-freshness", + "description": "A reviewer may observe that a test named for process freshness asserts executor identity instead, and that the name therefore overstates what is proven. This is the finding this case is built from, and it was adjudicated NOT material: the packet states the suite cannot observe a process identifier at all, the comment already says what is and is not proven, and the assertion is the strongest available proxy. Tolerated as an observation; gating on it is a false alarm.", + "equivalent_formulations": [ + "the test asserts executor identity rather than process freshness", + "the test name overstates what it proves", + "this does not actually prove one process per attempt", + "rename the test or assert process identity" + ] + }, + { + "id": "anf.artifact-glob-not-scoped", + "description": "The artifact glob reads the whole directory rather than only this run's files. Worth noting; the packet scopes the directory to the run under test.", + "equivalent_formulations": [ + "the glob is not scoped to this run", + "reads every file in the artifact directory" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/session-continuation-summary.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/session-continuation-summary.json new file mode 100644 index 0000000..5dce016 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/session-continuation-summary.json @@ -0,0 +1,28 @@ +{ + "expectation_version": "1.0", + "case_id": "session-continuation-summary", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "clean", + "material_root_causes": [], + "accepted_non_findings": [ + { + "id": "anf.started-true-without-an-agent", + "description": "A reviewer may observe that `started=True` on a path that started no agent reads as inaccurate and could mislead operators or metrics. This is the finding this case is built from, and it was adjudicated NOT material: the field is the loop-continuation signal by contract, the packet states no metric reads it, and the reporter is what speaks to operators. Tolerated as an observation; gating on it is a false alarm.", + "equivalent_formulations": [ + "started=True is inaccurate for a path that started no agent", + "the summary claims a session started", + "started True is misleading for operators or metrics", + "split continuation from agent-session-started" + ] + }, + { + "id": "anf.reason-string-not-an-enum", + "description": "The reason is a bare string rather than an enumerated type. Fair design commentary; the packet's preserved behaviour keeps the existing shape.", + "equivalent_formulations": [ + "reason should be an enum", + "the reason string is untyped" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/stale-claim-release-guard.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/stale-claim-release-guard.json new file mode 100644 index 0000000..337fe5f --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/stale-claim-release-guard.json @@ -0,0 +1,42 @@ +{ + "expectation_version": "1.0", + "case_id": "stale-claim-release-guard", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.guard-skipped-when-snapshot-owner-absent", + "requirement": "A claim taken by another worker between scan and apply is never released.", + "trigger": "The scan sees an expired entry with no owner, and a worker claims it before the action is applied.", + "surface": "_release", + "consequence": "The ownership comparison is itself conditional on the snapshot having had an owner, so the exact interleaving the change exists to prevent - unowned at scan, owned at apply - skips the comparison entirely and releases a live claim. The added test starts from an owned entry, so it exercises the branch that was already safe and passes.", + "severity": "blocking", + "equivalent_formulations": [ + "the owner comparison is skipped when the snapshot owner is None", + "the guard is conditional on snapshot_owner being not None", + "an unowned snapshot bypasses the ownership check", + "a claim taken between scan and apply can still be released", + "compare owners unconditionally including None" + ] + } + ], + "accepted_non_findings": [ + { + "id": "anf.late-binding-default-argument", + "description": "Binding loop variables through default arguments is unusual. It is the mechanism that makes the snapshot per-entry rather than shared, and the packet preserves the deferred-callable shape, so it is not a defect here.", + "equivalent_formulations": [ + "default arguments are used to bind loop variables", + "late binding via default parameters" + ] + }, + { + "id": "anf.soak-suite-not-run", + "description": "The concurrency soak suite is recorded as unavailable. Noting it is reasonable and honest; the packet says so explicitly, and it is a validation limitation rather than a second root cause.", + "equivalent_formulations": [ + "the soak suite was not run", + "no live-store concurrency evidence" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json new file mode 100644 index 0000000..1970beb --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "dependency-hint-parser-coverage", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 279, review comment 2862025661, authored by the repository owner and declined on the merits in reply 2862037362. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional lineage parser. Retains only the shape of the adjudicated exchange: a coverage request for an alias the change deliberately retired, declined because the contract narrowed. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "recorded_at": "2026-07-27", + "notes": "CLEAN CONTROL under the owner's settled standard. The reviewer asked for a regression case pinning a retired alias; the owner declined on the merits because the parser had been intentionally narrowed to the canonical shape. The rejected concern is recorded as anf.retired-alias-coverage-request." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json new file mode 100644 index 0000000..a4e97fc --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "dependency-strictness-propagation", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 335, review comment 2867594627, authored by the repository owner and accepted in the reply naming implementing commit 5cb0333. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional record pipeline. Retains only the failure shape: a decision helper gains a strict mode, one of its two callers adopts it, and the untouched sibling caller keeps deciding the same question permissively. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "recorded_at": "2026-07-27", + "notes": "Multi-file case: the defect is only visible by reading a caller the diff does not touch. Second adjudication is available by oracle." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json new file mode 100644 index 0000000..b462ae0 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "optional-tool-probe", + "origin": "repository_history", + "retention_authority": "This repository's own delivery history: commit f544aa0c19d97dd4f1aabd7dfab3df08b2ee6a6b on the #50 branch, whose message records the diagnosis. The defect survived an aggregate `clean` review verdict at head b605051 and was then caught by CI. Public, owner-authored, no retention question.", + "sanitization": "Rewritten against a fictional `taskr` runner rather than the real tool, so the case carries no repository-specific naming. The failure shape is retained exactly: an optional-tool probe that inspects only the return code, in a suite whose CI environment lacks the tool. Nothing private is involved; both the source commit and this repository are public.", + "recorded_at": "2026-07-27", + "notes": "The strongest-adjudicated case in the corpus: its second adjudication is a machine. CI failed on the real defect after a clean review verdict, which is an independent, non-model judgement of materiality." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json new file mode 100644 index 0000000..f12bd2b --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "post-bootstrap-module-load", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 710, review comment 2961766206, authored by the repository owner. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional entry script. Retains only the shape of the adjudicated exchange: an unusual deferred module load flagged as strange, kept because the deferral is load-bearing, with an explanatory comment as the whole resolution. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "recorded_at": "2026-07-27", + "notes": "NEGATIVE CONTROL, polish-only disposition. The reviewer said the pattern was strange and asked for a comment explaining it; the owner added the comment and changed no behaviour, which adjudicates the pattern itself as intentional and correct. The rejected concern is recorded as anf.importlib-instead-of-a-plain-import." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json new file mode 100644 index 0000000..2dc6ef6 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "process-isolation-assertion", + "origin": "repository_history", + "retention_authority": "This repository's own delivery history. The finding was raised by a real `review-code-change` run during #50 and dispositioned `defer` under the review contract, then preserved rather than applied. It is recorded verbatim in the merged body of pull request 61, item 4 of the deferred list. Public, owner-authored, no retention question.", + "sanitization": "Rewritten against a fictional harness rather than the real evaluator, so the case cannot be recognised as, or confused with, this repository's own test suite. The failure shape is retained exactly: a test named for process freshness that asserts a stable executor identity, in a suite that cannot observe a process identifier. Nothing private is involved.", + "recorded_at": "2026-07-27", + "notes": "NEGATIVE CONTROL, deferred-as-immaterial disposition, and the only case whose first adjudication was produced by the review suite under evaluation rather than by a human. Recorded as such because that is a correlation an adjudicator must weigh: the disposition came from the same contract being measured. The rejected concern is recorded as anf.name-asserts-identity-not-freshness." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json new file mode 100644 index 0000000..ba0a5a8 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "session-continuation-summary", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 486, review comment 2881737041, authored by the repository owner. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional worker. Retains only the shape of the adjudicated exchange: a summary flag that reads as a claim about behaviour is deliberately kept because it is the loop-continuation signal, and the reporting is corrected instead. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "recorded_at": "2026-07-27", + "notes": "CLEAN CONTROL under the owner's settled standard: cleanliness rests on an adjudicated-rejected finding, not on absence of comment. The reviewer asked for `started=True` to change; the owner declined on the merits - the flag preserves loop continuation - and fixed only the operator-facing wording. The rejected concern is recorded as anf.started-true-without-an-agent so a non-gating mention is tolerated while gating on it is charged a false alarm. Multi-file case: the disposition only makes sense by reading the untouched loop that consumes the flag." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json new file mode 100644 index 0000000..0bf2a40 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "stale-claim-release-guard", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 373, review comment 2869270760, authored by the repository owner and accepted in the reply that added a regression test for the collect/apply window. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional claim queue. Retains only the failure shape: a collect-then-apply guard compares ownership only when the captured owner was present, so the unowned-at-capture interleaving bypasses it. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "recorded_at": "2026-07-27", + "notes": "Multi-file case: the interleaving is only reachable by reading the untouched claim writer. Second adjudication is available by oracle." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/PROMPT.md b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/PROMPT.md new file mode 100644 index 0000000..bba42bc --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/PROMPT.md @@ -0,0 +1,7 @@ +Review the supplied review packet as a read-only reviewer. Apply the +repository's canonical review packet, finding, and verdict contract exactly as +written, and bind the result to the candidate identity supplied with the run. + +Return only one conforming review result. Derive every conclusion from the +packet and the supplied contracts; no other evidence is available, and no +information about this case exists outside them. diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json new file mode 100644 index 0000000..2571dbf --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json @@ -0,0 +1,85 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/lineage", + "base_branch": "main" + }, + "candidate": { + "head_sha": "6a08d3f27b915ce4130827aebf5d9c6142b83e70", + "comparison_base_sha": "b71e4c8093a5d216f804eb37c295a610d7f2b8ce", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/lineage/parse.py b/lineage/parse.py\n--- a/lineage/parse.py\n+++ b/lineage/parse.py\n@@ -8,17 +8,14 @@ CANONICAL_LINK = \"link_type\"\n def parent_links(entry):\n \"\"\"Return the parent links declared by one dependency entry.\n \n- Historically the exporter emitted the link kind under several names.\n+ The exporter now emits exactly one canonical field, so only that field is\n+ read. The former aliases are gone from the export contract and reading them\n+ would keep accepting shapes the exporter can no longer produce.\n \"\"\"\n links = []\n for item in entry.get(\"dependencies\") or []:\n if isinstance(item, str):\n links.append(item)\n continue\n- kind = (\n- item.get(CANONICAL_LINK)\n- or item.get(\"relation\")\n- or item.get(\"linkType\")\n- or item.get(\"type\")\n- )\n+ kind = item.get(CANONICAL_LINK)\n if kind == \"parent_child\":\n links.append(item[\"issue\"][\"id\"])\n return links\ndiff --git a/tests/test_parse.py b/tests/test_parse.py\n--- a/tests/test_parse.py\n+++ b/tests/test_parse.py\n@@ -10,6 +10,18 @@ def test_parent_links_reads_a_string_entry():\n assert parent_links({\"dependencies\": [\"p-1\"]}) == [\"p-1\"]\n+\n+\n+def test_parent_links_reads_the_canonical_link_field():\n+ entry = {\"dependencies\": [{\"link_type\": \"parent_child\", \"issue\": {\"id\": \"p-2\"}}]}\n+ assert parent_links(entry) == [\"p-2\"]\n+\n+\n+def test_parent_links_ignores_a_retired_alias():\n+ entry = {\"dependencies\": [{\"type\": \"parent_child\", \"issue\": {\"id\": \"p-3\"}}]}\n+ assert parent_links(entry) == []\n" + } + }, + "change_contract": { + "goal": "Narrow the dependency-entry parser to the one canonical link field the exporter now emits, dropping the retired aliases.", + "acceptance_criteria": [ + "The canonical link field is read.", + "A retired alias is not read, so a shape the exporter can no longer produce is not silently accepted.", + "A bare string entry keeps working." + ], + "non_goals": [ + "Change the exporter.", + "Keep a compatibility path for retired aliases." + ], + "preserved_behaviors": [ + "A bare string dependency entry resolves exactly as before." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Export contract rules", + "location": "AGENTS.md", + "summary": "When the export contract narrows, the reader narrows with it in the same change. Keeping a reader for a retired shape hides exporter regressions instead of surfacing them." + } + ], + "named_documents": [ + { + "label": "Export contract", + "location": "docs/export-contract.md" + }, + { + "label": "Exporter", + "location": "lineage/export.py" + } + ], + "nearby_patterns": [ + { + "label": "Parser tests", + "location": "tests/test_parse.py" + }, + { + "label": "Frontier resolution consumer", + "location": "lineage/frontier.py" + } + ] + }, + "validation": [ + { + "name": "parser tests", + "command": "pytest tests/test_parse.py", + "scope": "focused", + "status": "passed", + "result": "11 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "97 passed" + } + ], + "context": { + "data": [ + "`lineage/export.py` emits only `link_type`; the aliases were removed from the export contract in the same release.", + "No stored data is replayed through this parser: every entry it sees comes from a live export." + ], + "operational": [ + "The exporter and this parser ship together and are always the same version." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json new file mode 100644 index 0000000..07169b1 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json @@ -0,0 +1,91 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/pipeline", + "base_branch": "main" + }, + "candidate": { + "head_sha": "5b1c9f47a20d8e63145c7a09b2fd86e3517ac04d", + "comparison_base_sha": "1f8a3d20c9b475e6a08d1c73f92b4e6035ad7c81", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/pipeline/integration.py b/pipeline/integration.py\n--- a/pipeline/integration.py\n+++ b/pipeline/integration.py\n@@ -18,10 +18,22 @@ from pipeline.model import Record\n-def integration_proven(record):\n- \"\"\"Return whether this record's work reached the target branch.\"\"\"\n- return bool(record.merge_marker or record.target_ref)\n+def integration_proven(record, *, require_target_proof=False):\n+ \"\"\"Return whether this record's work reached the target branch.\n+\n+ With `require_target_proof`, only a target-branch ancestry proof counts. The\n+ merge marker alone is a hint written by the tooling and can be present on a\n+ record whose work never landed.\n+ \"\"\"\n+ if require_target_proof:\n+ return bool(record.target_ref)\n+ return bool(record.merge_marker or record.target_ref)\n@@ -44,7 +56,8 @@ def reconcile_closed(records):\n for record in records:\n- if integration_proven(record):\n+ status = record.status\n+ if integration_proven(record, require_target_proof=status == \"closed\"):\n reopen_if_unlanded(record)\ndiff --git a/pipeline/dependencies.py b/pipeline/dependencies.py\n--- a/pipeline/dependencies.py\n+++ b/pipeline/dependencies.py\n@@ -9,7 +9,7 @@ from pipeline.integration import integration_proven\n def dependency_finalized(record):\n \"\"\"Return whether a dependency may unblock the records that follow it.\"\"\"\n return integration_proven(record)\ndiff --git a/tests/test_integration.py b/tests/test_integration.py\n--- a/tests/test_integration.py\n+++ b/tests/test_integration.py\n@@ -20,3 +20,16 @@ def test_integration_proven_accepts_a_target_ref():\n assert integration_proven(Record(target_ref=\"refs/heads/main\"))\n+\n+\n+def test_integration_proven_rejects_a_marker_only_record_under_strict_proof():\n+ record = Record(merge_marker=True, target_ref=None)\n+ assert not integration_proven(record, require_target_proof=True)\n+\n+\n+def test_reconcile_closed_uses_strict_proof_for_a_closed_record():\n+ record = Record(status=\"closed\", merge_marker=True, target_ref=None)\n+ reconcile_closed([record])\n+ assert record.reopened\n" + } + }, + "change_contract": { + "goal": "Stop treating a record as integrated on the strength of a merge marker alone once that record is closed, because the marker is a tooling hint that can outlive work that never landed.", + "acceptance_criteria": [ + "A closed record is only integrated when target-branch ancestry proves it.", + "Every path that decides whether a record is integrated applies the same strictness rule for a closed record.", + "An open record keeps the existing permissive behaviour." + ], + "non_goals": [ + "Change how the merge marker itself is written.", + "Change ancestry computation." + ], + "preserved_behaviors": [ + "`integration_proven` keeps its permissive result for an open record.", + "`dependency_finalized` keeps its signature." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Consistency rules", + "location": "AGENTS.md", + "summary": "When a decision rule gains a stricter mode, every caller that makes the same decision adopts it in the same change. Two call sites deciding one question differently is a defect, not a difference." + } + ], + "named_documents": [ + { + "label": "Integration proof contract", + "location": "docs/integration.md" + }, + { + "label": "Record model", + "location": "pipeline/model.py" + } + ], + "nearby_patterns": [ + { + "label": "Sibling decision path", + "location": "pipeline/dependencies.py" + }, + { + "label": "Downstream unblock path", + "location": "pipeline/scheduler.py" + }, + { + "label": "Integration tests", + "location": "tests/test_integration.py" + } + ] + }, + "validation": [ + { + "name": "integration tests", + "command": "pytest tests/test_integration.py", + "scope": "focused", + "status": "passed", + "result": "9 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "184 passed" + } + ], + "context": { + "data": [ + "`pipeline/dependencies.py` and `pipeline/integration.py` are the only two modules that call `integration_proven`.", + "`dependency_finalized` is called by `pipeline/scheduler.py` to decide whether the records that follow a dependency may start.", + "A closed record carrying a merge marker but no target ref occurs whenever a merge is abandoned after the marker is written." + ], + "operational": [ + "The scheduler runs continuously and acts on `dependency_finalized` without re-checking anything downstream." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json new file mode 100644 index 0000000..f632e0f --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json @@ -0,0 +1,86 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/toolkit", + "base_branch": "main" + }, + "candidate": { + "head_sha": "c42a8f150b7e93d6218ac05f7b34e9612d8a70bf", + "comparison_base_sha": "7e91b3d05c28a46f130db7295ea86c410fb2d539", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/tests/test_recipes.py b/tests/test_recipes.py\n--- a/tests/test_recipes.py\n+++ b/tests/test_recipes.py\n@@ -1,8 +1,26 @@\n import subprocess\n import unittest\n \n \n class RecipeTests(unittest.TestCase):\n- \"\"\"Exercise the recipes directly.\"\"\"\n+ \"\"\"Run the recipes through the task runner when it is installed.\n+\n+ The runner is optional, so this class skips cleanly when it is absent.\n+ \"\"\"\n+\n+ @classmethod\n+ def setUpClass(cls):\n+ if subprocess.run(\n+ [\"taskr\", \"--version\"], capture_output=True, check=False\n+ ).returncode != 0:\n+ raise unittest.SkipTest(\"taskr is not installed\")\n+\n+ def taskr(self, *args):\n+ return subprocess.run([\"taskr\", *args], capture_output=True, text=True)\n+\n+ def test_the_check_recipe_runs(self):\n+ self.assertEqual(0, self.taskr(\"check\").returncode)\n+\n+ def test_the_audit_recipe_runs(self):\n+ self.assertEqual(0, self.taskr(\"audit\").returncode)\n" + } + }, + "change_contract": { + "goal": "Exercise the task-runner recipes in the suite, skipping cleanly on a machine where the optional runner is not installed.", + "acceptance_criteria": [ + "The recipe tests run when the task runner is installed.", + "The suite skips these tests, rather than failing, when the runner is absent.", + "No other test changes behaviour." + ], + "non_goals": [ + "Install the task runner as a dependency.", + "Change what the recipes do." + ], + "preserved_behaviors": [ + "Every pre-existing test keeps its result." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Optional-dependency rules", + "location": "AGENTS.md", + "summary": "A suite must pass on a machine that has none of the optional tools installed. Probe for an optional tool in a way that distinguishes absent from present-and-failing." + } + ], + "named_documents": [ + { + "label": "Supported environments", + "location": "docs/environments.md" + }, + { + "label": "CI workflow", + "location": ".github/workflows/ci.yml" + } + ], + "nearby_patterns": [ + { + "label": "Existing optional-tool probe", + "location": "tests/test_formatter.py" + }, + { + "label": "Recipe definitions", + "location": "Taskrfile" + } + ] + }, + "validation": [ + { + "name": "recipe tests", + "command": "pytest tests/test_recipes.py", + "scope": "focused", + "status": "passed", + "result": "2 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "88 passed" + } + ], + "context": { + "data": [ + "The developer machine this candidate was validated on has the task runner installed.", + "`.github/workflows/ci.yml` installs the project's Python dependencies and does not install the task runner.", + "`subprocess.run` raises `FileNotFoundError`, a subclass of `OSError`, when the named executable does not exist; it does not return a non-zero status." + ], + "operational": [ + "CI is the only environment where the optional runner is absent, and it is the gate that must stay green." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json new file mode 100644 index 0000000..362877d --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json @@ -0,0 +1,85 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/toolkit-scripts", + "base_branch": "main" + }, + "candidate": { + "head_sha": "e3719c4a8b05d2f6410a9be7c583d17206fa4b98", + "comparison_base_sha": "a520d8e163b47c09f2718ad6e5c39b410827fd6a", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/scripts/finish_task.py b/scripts/finish_task.py\n--- a/scripts/finish_task.py\n+++ b/scripts/finish_task.py\n@@ -1,10 +1,18 @@\n+import importlib\n import sys\n \n from bootstrap import select_runtime\n \n select_runtime(require_health=__name__ == \"__main__\")\n \n-from toolkit import store # noqa: E402\n+# Loaded through importlib, after `select_runtime` has reordered `sys.path`\n+# toward the selected runtime. A module-level `import` here binds whichever\n+# copy of `toolkit` was importable before the reorder, which on a machine with\n+# an older installed copy is the wrong one - and `--help` would then exercise a\n+# runtime the real run never uses.\n+_STORE = importlib.import_module(\"toolkit.store\")\n \n \n def main(argv):\n- return store.finish(argv[1])\n+ return _STORE.finish(argv[1])\ndiff --git a/tests/test_finish_task.py b/tests/test_finish_task.py\n--- a/tests/test_finish_task.py\n+++ b/tests/test_finish_task.py\n@@ -8,3 +8,13 @@ def test_main_finishes_the_named_task():\n assert main([\"finish_task\", \"t-1\"]) == 0\n+\n+\n+def test_the_store_is_loaded_after_the_runtime_is_selected():\n+ order = import_order_probe()\n+ assert order.index(\"select_runtime\") < order.index(\"toolkit.store\")\n+\n+\n+def test_help_uses_the_selected_runtime(tmp_path):\n+ older = install_older_toolkit(tmp_path)\n+ assert run_help_returns_runtime(older) == \"selected\"\n" + } + }, + "change_contract": { + "goal": "Make the entry script bind the runtime module the bootstrap selected, rather than whichever copy happened to be importable first.", + "acceptance_criteria": [ + "The store module is loaded after the runtime selection has run.", + "`--help` exercises the selected runtime even when an older copy is installed on the machine.", + "The command's behaviour is otherwise unchanged." + ], + "non_goals": [ + "Change how the runtime is selected.", + "Change what `finish` does." + ], + "preserved_behaviors": [ + "`main` returns the same status for the same arguments." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Bootstrap rules", + "location": "AGENTS.md", + "summary": "An entry script must not bind a runtime module before the bootstrap has selected one. Where ordering matters, make the ordering explicit in code and say why in a comment." + } + ], + "named_documents": [ + { + "label": "Runtime selection contract", + "location": "docs/bootstrap.md" + }, + { + "label": "Bootstrap", + "location": "bootstrap.py" + } + ], + "nearby_patterns": [ + { + "label": "Sibling entry script", + "location": "scripts/start_task.py" + }, + { + "label": "Entry script tests", + "location": "tests/test_finish_task.py" + } + ] + }, + "validation": [ + { + "name": "entry script tests", + "command": "pytest tests/test_finish_task.py", + "scope": "focused", + "status": "passed", + "result": "5 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "121 passed" + } + ], + "context": { + "data": [ + "A module-level import is executed before any statement below it, so it cannot observe a `sys.path` reorder performed by that statement.", + "Machines in the field commonly have an older installed copy of the toolkit alongside the selected runtime." + ], + "operational": [ + "`--help` is the path operators use most, and it must reflect the runtime a real run would use." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json new file mode 100644 index 0000000..b5a745d --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json @@ -0,0 +1,84 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/harness", + "base_branch": "main" + }, + "candidate": { + "head_sha": "d81f5a027c34e9b6120837aed5f96c4b1e7a20d3", + "comparison_base_sha": "40b9c7e2118d5f36a09e17b4c826d5310fa9e7b2", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/harness/tests/test_attempts.py b/harness/tests/test_attempts.py\n--- a/harness/tests/test_attempts.py\n+++ b/harness/tests/test_attempts.py\n@@ -30,3 +30,14 @@ def test_each_attempt_is_recorded():\n assert len(records) == 2\n+\n+\n+def test_every_attempt_runs_in_its_own_process():\n+ records = run_attempts(runs=2, artifact_dir=ARTIFACTS)\n+ names = set()\n+ for path in ARTIFACTS.glob(\"*.stdout.json\"):\n+ names.add(json.loads(path.read_text())[\"executor\"][\"name\"])\n+ # The bundled executor reports a stable name; process freshness is proven by\n+ # the driver spawning one subprocess per attempt.\n+ assert names == {\"bundled-executor\"}\n+ assert len(records) == len(list(ARTIFACTS.iterdir()))\n" + } + }, + "change_contract": { + "goal": "Cover the driver's per-attempt behaviour: every attempt is recorded, and each attempt's retained output is written under its own name.", + "acceptance_criteria": [ + "Every attempt produces exactly one retained artifact.", + "The retained artifacts are attributable to the executor that produced them." + ], + "non_goals": [ + "Change how the driver spawns a process.", + "Add a mechanism for observing operating-system process identity." + ], + "preserved_behaviors": [ + "The driver still spawns one subprocess per attempt." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Test naming rules", + "location": "AGENTS.md", + "summary": "A test's name states its subject. Where a property cannot be observed from inside the suite, assert the strongest observable proxy and say in a comment what is and is not proven." + } + ], + "named_documents": [ + { + "label": "Driver contract", + "location": "docs/driver.md" + }, + { + "label": "Driver", + "location": "harness/driver.py" + } + ], + "nearby_patterns": [ + { + "label": "Bundled executor", + "location": "harness/bundled_executor.py" + }, + { + "label": "Driver tests", + "location": "harness/tests/test_attempts.py" + } + ] + }, + "validation": [ + { + "name": "harness tests", + "command": "pytest harness/tests", + "scope": "focused", + "status": "passed", + "result": "19 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "19 passed" + } + ], + "context": { + "data": [ + "The bundled executor reports a fixed name and exposes no process identifier, so the suite has no way to observe an operating-system process id.", + "The driver calls the executor through one `subprocess.run` per attempt; that is the code the assertion is about." + ], + "operational": [ + "The retained artifacts are the only evidence a later record can cite." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json new file mode 100644 index 0000000..02b5904 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json @@ -0,0 +1,98 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/worker", + "base_branch": "main" + }, + "candidate": { + "head_sha": "8f2b60d94a17c3e5026da8b7419f3c60e5127bad", + "comparison_base_sha": "204c9e71f8b3a06d51728cae9f04b263d817ac5e", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/worker/session.py b/worker/session.py\n--- a/worker/session.py\n+++ b/worker/session.py\n@@ -61,6 +61,20 @@ def run_once(job, control):\n+ preflight = finalize_preflight(job)\n+ if preflight.finalize_only:\n+ control.say(\n+ \"Skipping agent startup; finalizing this job from the preflight \"\n+ f\"result ({preflight.reason}).\"\n+ )\n+ finalize(job, control)\n+ # `started` drives loop continuation, not \"an agent ran\". The loop must\n+ # keep going here, so this stays True; the reporter below is what tells\n+ # an operator no agent session was launched.\n+ return RunSummary(started=True, reason=\"finalize_only\")\n+\n agent = start_agent(job)\n return RunSummary(started=True, reason=\"agent_session\")\ndiff --git a/worker/report.py b/worker/report.py\n--- a/worker/report.py\n+++ b/worker/report.py\n@@ -12,6 +12,10 @@ def report_summary(summary, control):\n+ if summary.reason == \"finalize_only\":\n+ control.say(\"continued without an agent session (finalize-only preflight)\")\n+ return\n if summary.started:\n control.say(\"started an agent session\")\ndiff --git a/tests/test_report.py b/tests/test_report.py\n--- a/tests/test_report.py\n+++ b/tests/test_report.py\n@@ -14,3 +14,16 @@ def test_report_summary_reports_a_started_session():\n assert \"started an agent session\" in control.lines\n+\n+\n+def test_report_summary_does_not_claim_a_session_for_finalize_only():\n+ control = FakeControl()\n+ report_summary(RunSummary(started=True, reason=\"finalize_only\"), control)\n+ assert \"started an agent session\" not in control.lines\n+ assert \"continued without an agent session\" in control.lines[0]\n+\n+\n+def test_run_once_continues_the_loop_for_finalize_only():\n+ summary = run_once(finalize_only_job(), FakeControl())\n+ assert summary.started is True\n+ assert summary.reason == \"finalize_only\"\n" + } + }, + "change_contract": { + "goal": "Let the worker finalize a job straight from the preflight result without starting an agent, while keeping the run loop going and telling the operator plainly that no agent session was launched.", + "acceptance_criteria": [ + "A finalize-only job is finalized without starting an agent.", + "The run loop continues after a finalize-only job.", + "Operator output never claims an agent session was started for a finalize-only job." + ], + "non_goals": [ + "Redefine what `started` means to the run loop.", + "Change how a job is finalized." + ], + "preserved_behaviors": [ + "`started` keeps driving loop continuation and nothing else.", + "A job that does start an agent reports exactly as before." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Summary field contract", + "location": "AGENTS.md", + "summary": "`RunSummary.started` is the loop-continuation signal. It does not mean an agent ran, and operator-facing wording is the reporter's responsibility rather than the field's." + } + ], + "named_documents": [ + { + "label": "Run loop contract", + "location": "docs/worker-loop.md" + }, + { + "label": "Summary model", + "location": "worker/model.py" + } + ], + "nearby_patterns": [ + { + "label": "Run loop that consumes `started`", + "location": "worker/loop.py" + }, + { + "label": "Reporter", + "location": "worker/report.py" + }, + { + "label": "Reporter tests", + "location": "tests/test_report.py" + } + ] + }, + "validation": [ + { + "name": "reporter tests", + "command": "pytest tests/test_report.py", + "scope": "focused", + "status": "passed", + "result": "8 passed" + }, + { + "name": "session tests", + "command": "pytest tests/test_session.py", + "scope": "focused", + "status": "passed", + "result": "14 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "203 passed" + } + ], + "context": { + "data": [ + "`worker/loop.py` continues iterating while `summary.started` is true and stops when it is false; it reads no other field.", + "Operator-facing status text is produced only by `report_summary`.", + "No metric or dashboard reads `started` directly." + ], + "operational": [ + "A finalize-only job is the common case during a backlog drain." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json new file mode 100644 index 0000000..799ed39 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json @@ -0,0 +1,94 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/queue", + "base_branch": "main" + }, + "candidate": { + "head_sha": "9d3e71b8c04a26f5138e7b02da96c451f8a3d70e", + "comparison_base_sha": "3a7f2e94b18c50d6ae23f107c9b48d5162ea08f3", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/queue/reaper.py b/queue/reaper.py\n--- a/queue/reaper.py\n+++ b/queue/reaper.py\n@@ -14,18 +14,36 @@ from queue.store import load_claim, release_claim\n def collect_expired(now, store):\n \"\"\"Return one release action per claim that looks expired.\"\"\"\n actions = []\n for entry in store.scan():\n if entry.claimed_at is None or now - entry.claimed_at <= TTL:\n continue\n+ snapshot_owner = entry.owner\n \n- def _release(entry_id=entry.entry_id):\n- release_claim(store, entry_id)\n+ def _release(entry_id=entry.entry_id, snapshot_owner=snapshot_owner):\n+ # Re-read at apply time: the snapshot may be minutes old.\n+ current = load_claim(store, entry_id)\n+ if current is None:\n+ return\n+ if snapshot_owner is not None and current.owner != snapshot_owner:\n+ return\n+ release_claim(store, entry_id)\n \n actions.append(_release)\n return actions\ndiff --git a/tests/test_reaper.py b/tests/test_reaper.py\n--- a/tests/test_reaper.py\n+++ b/tests/test_reaper.py\n@@ -18,3 +18,18 @@ def test_collect_expired_ignores_a_fresh_claim():\n assert collect_expired(NOW, store) == []\n+\n+\n+def test_collect_expired_skips_release_when_the_owner_changed():\n+ store = FakeStore([Entry(entry_id=\"e-1\", owner=\"worker-a\", claimed_at=OLD)])\n+ actions = collect_expired(NOW, store)\n+ store.set_owner(\"e-1\", \"worker-b\")\n+ for action in actions:\n+ action()\n+ assert store.claim(\"e-1\").owner == \"worker-b\"\n" + } + }, + "change_contract": { + "goal": "Stop the expiry reaper from releasing a claim that another worker has taken since the scan, by re-reading the claim at apply time.", + "acceptance_criteria": [ + "A release only happens when the claim still belongs to the owner the scan saw.", + "A claim taken by another worker between scan and apply is never released.", + "A claim that has since disappeared is skipped rather than erroring." + ], + "non_goals": [ + "Change the expiry TTL.", + "Make the scan itself transactional." + ], + "preserved_behaviors": [ + "An unexpired claim is still ignored by the scan.", + "`collect_expired` still returns callables the caller applies later." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Concurrency rules", + "location": "AGENTS.md", + "summary": "A collect-then-apply pass must re-verify every precondition it captured. A guard that is conditional on the captured value being present is not a guard for the case where it was absent." + } + ], + "named_documents": [ + { + "label": "Claim ownership contract", + "location": "docs/claims.md" + }, + { + "label": "Claim store", + "location": "queue/store.py" + } + ], + "nearby_patterns": [ + { + "label": "Claim writer used by workers", + "location": "queue/worker.py" + }, + { + "label": "Reaper tests", + "location": "tests/test_reaper.py" + } + ] + }, + "validation": [ + { + "name": "reaper tests", + "command": "pytest tests/test_reaper.py", + "scope": "focused", + "status": "passed", + "result": "6 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "132 passed" + }, + { + "name": "concurrency soak", + "command": "pytest -m soak", + "scope": "full", + "status": "unavailable", + "reason": "The soak marker needs a live store and was not run on this candidate." + } + ], + "context": { + "data": [ + "An unclaimed entry has `owner` set to `None` and still carries `claimed_at` from a prior cycle, because the writer clears ownership before it clears the timestamp.", + "`queue/worker.py` takes a claim by setting `owner` and refreshing `claimed_at` in one write.", + "Roughly a fifth of expired entries are seen by the scan with no owner." + ], + "operational": [ + "The reaper and the workers run concurrently against the same store; minutes can pass between scan and apply." + ] + } +} From 6dcfeabc7acd325d1dcaae4ed341fa780df94bc9 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Mon, 27 Jul 2026 08:49:16 -0700 Subject: [PATCH 2/5] feat: adjudicate the correctness cases by executable oracle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Add `review-suite/scripts/evals/oracles/`: one runnable module per case exposing the reproduction as the candidate leaves it, the corrected reproduction where a root cause exists, and the packet's requirement expressed as code. - Add `test_eval_oracles.py`, which runs each requirement and asserts the polarity the case's expected verdict demands. - Record adjudication structurally: `provenance.adjudication` names the first adjudication and whether the second is an `oracle` or `owner_required`. ## Why an oracle rather than a second opinion The requirement is two independent adjudications. The first is the recorded source disposition. The obvious candidate for the second — a fresh agent context — is not independent of a reviewer it shares a model family with: the errors correlate, so a defect that model class cannot see becomes a defect the adjudicator agrees is immaterial, and recall rises without the reviewer improving. A machine that runs the code has no such correlation. So for a correctness case, materiality stops being an opinion. The stated requirement either holds against the candidate or it does not. ## What the tests assert | expected verdict | on `candidate()` | on `corrected()` | | ------------------ | ------------------------- | ------------------------- | | `changes_required` | the requirement **fails** | the requirement **holds** | | `clean` | the requirement **holds** | not applicable | The `corrected` leg is the part that makes this an adjudication of the *root cause* rather than of a symptom: correcting the cause the expectation names is what must make the requirement hold. A case whose requirement fails for some unrelated reason fails here rather than shipping as evidence. All seven correctness cases are oracle-settled: three gating cases where the requirement fails and correction fixes it, and four clean controls where the stated contract holds. A test requires every case in that stratum to ship one, so a case cannot be silently routed to the owner when a machine could have settled it. ## What an oracle does not prove It covers the requirements the change contract states, and nothing else. A clean control's oracle proves the contract holds and the raised concern was adjudicated immaterial; it does not prove the diff is free of every possible defect. Two cases record this explicitly: the oracle cannot adjudicate an aesthetic objection to a field's name, nor whether a test's name overstates what it asserts, because those are judgements about prose. `process-isolation-assertion` records a further caveat an adjudicator has to weigh: its first adjudication came from the review suite under evaluation, so a `defer` from the same contract being measured is weaker evidence than a human disposition. ## Where no oracle exists The simplicity strata have none — "this is over-engineered" has no executable form — so those cases will declare `owner_required`, and a test holds them to it rather than letting them claim two adjudications they do not have. --- .../evals/contracts/provenance.schema.json | 57 +++++- .../dependency-hint-parser-coverage.json | 4 + .../dependency-strictness-propagation.json | 4 + .../provenance/optional-tool-probe.json | 5 + .../post-bootstrap-module-load.json | 4 + .../process-isolation-assertion.json | 5 + .../session-continuation-summary.json | 5 + .../provenance/stale-claim-release-guard.json | 4 + review-suite/scripts/evals/oracles/README.md | 53 ++++++ .../scripts/evals/oracles/__init__.py | 51 ++++++ .../dependency_hint_parser_coverage.py | 56 ++++++ .../dependency_strictness_propagation.py | 86 +++++++++ .../evals/oracles/optional_tool_probe.py | 70 ++++++++ .../oracles/post_bootstrap_module_load.py | 57 ++++++ .../oracles/process_isolation_assertion.py | 48 +++++ .../oracles/session_continuation_summary.py | 88 ++++++++++ .../oracles/stale_claim_release_guard.py | 90 ++++++++++ .../scripts/tests/test_eval_oracles.py | 164 ++++++++++++++++++ 18 files changed, 845 insertions(+), 6 deletions(-) create mode 100644 review-suite/scripts/evals/oracles/README.md create mode 100644 review-suite/scripts/evals/oracles/__init__.py create mode 100644 review-suite/scripts/evals/oracles/dependency_hint_parser_coverage.py create mode 100644 review-suite/scripts/evals/oracles/dependency_strictness_propagation.py create mode 100644 review-suite/scripts/evals/oracles/optional_tool_probe.py create mode 100644 review-suite/scripts/evals/oracles/post_bootstrap_module_load.py create mode 100644 review-suite/scripts/evals/oracles/process_isolation_assertion.py create mode 100644 review-suite/scripts/evals/oracles/session_continuation_summary.py create mode 100644 review-suite/scripts/evals/oracles/stale_claim_release_guard.py create mode 100644 review-suite/scripts/tests/test_eval_oracles.py diff --git a/review-suite/evals/contracts/provenance.schema.json b/review-suite/evals/contracts/provenance.schema.json index f3de4a8..b1b96fa 100644 --- a/review-suite/evals/contracts/provenance.schema.json +++ b/review-suite/evals/contracts/provenance.schema.json @@ -14,8 +14,13 @@ "recorded_at" ], "properties": { - "provenance_version": {"const": "1.0"}, - "case_id": {"type": "string", "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$"}, + "provenance_version": { + "const": "1.0" + }, + "case_id": { + "type": "string", + "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$" + }, "origin": { "enum": [ "synthetic", @@ -24,9 +29,49 @@ "connector_outcome" ] }, - "retention_authority": {"type": "string", "minLength": 1}, - "sanitization": {"type": "string", "minLength": 1}, - "recorded_at": {"type": "string", "pattern": "^\\d{4}-\\d{2}-\\d{2}$"}, - "notes": {"type": "string", "minLength": 1} + "retention_authority": { + "type": "string", + "minLength": 1 + }, + "sanitization": { + "type": "string", + "minLength": 1 + }, + "recorded_at": { + "type": "string", + "pattern": "^\\d{4}-\\d{2}-\\d{2}$" + }, + "notes": { + "type": "string", + "minLength": 1 + }, + "adjudication": { + "description": "How this case's materiality was adjudicated, and by whom. The requirement is two independent adjudications; this records what each one actually was so a reader never has to infer it. Absent on a case that predates the record.", + "type": "object", + "additionalProperties": false, + "required": [ + "first", + "second" + ], + "properties": { + "first": { + "description": "The first adjudication. Normally the recorded source disposition, which is strong for a root cause, thin for severity, and unavailable for formulations.", + "type": "string", + "minLength": 1 + }, + "second": { + "description": "The second adjudication. `oracle` means a machine settled it: the stated requirement runs, fails against the candidate, and holds once the stated root cause is corrected. `owner_required` means no oracle exists and the owner must supply it - never a fresh agent context sharing a model family with the reviewer being measured, whose errors correlate with it. `none` means neither, which no case may ship with once it is scored.", + "enum": [ + "oracle", + "owner_required", + "none" + ] + }, + "notes": { + "type": "string", + "minLength": 1 + } + } + } } } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json index 1970beb..1446cfc 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-hint-parser-coverage.json @@ -4,6 +4,10 @@ "origin": "minimized_reproduction", "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 279, review comment 2862025661, authored by the repository owner and declined on the merits in reply 2862037362. Public source, owner-authored review, no third-party or customer material.", "sanitization": "Rewritten from scratch against a fictional lineage parser. Retains only the shape of the adjudicated exchange: a coverage request for an alias the change deliberately retired, declined because the contract narrowed. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 279 review comment 2862025661, declined on the merits in reply 2862037362.", + "second": "oracle" + }, "recorded_at": "2026-07-27", "notes": "CLEAN CONTROL under the owner's settled standard. The reviewer asked for a regression case pinning a retired alias; the owner declined on the merits because the parser had been intentionally narrowed to the canonical shape. The rejected concern is recorded as anf.retired-alias-coverage-request." } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json index a4e97fc..39c61e2 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/dependency-strictness-propagation.json @@ -4,6 +4,10 @@ "origin": "minimized_reproduction", "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 335, review comment 2867594627, authored by the repository owner and accepted in the reply naming implementing commit 5cb0333. Public source, owner-authored review, no third-party or customer material.", "sanitization": "Rewritten from scratch against a fictional record pipeline. Retains only the failure shape: a decision helper gains a strict mode, one of its two callers adopts it, and the untouched sibling caller keeps deciding the same question permissively. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 335 review comment 2867594627, accepted in commit 5cb0333.", + "second": "oracle" + }, "recorded_at": "2026-07-27", "notes": "Multi-file case: the defect is only visible by reading a caller the diff does not touch. Second adjudication is available by oracle." } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json index b462ae0..0552eff 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/optional-tool-probe.json @@ -4,6 +4,11 @@ "origin": "repository_history", "retention_authority": "This repository's own delivery history: commit f544aa0c19d97dd4f1aabd7dfab3df08b2ee6a6b on the #50 branch, whose message records the diagnosis. The defect survived an aggregate `clean` review verdict at head b605051 and was then caught by CI. Public, owner-authored, no retention question.", "sanitization": "Rewritten against a fictional `taskr` runner rather than the real tool, so the case carries no repository-specific naming. The failure shape is retained exactly: an optional-tool probe that inspects only the return code, in a suite whose CI environment lacks the tool. Nothing private is involved; both the source commit and this repository are public.", + "adjudication": { + "first": "This repository's commit f544aa0, whose message records the diagnosis. CI failed on the real defect after an aggregate `clean` review verdict, so the first adjudication is itself a machine.", + "second": "oracle", + "notes": "Both adjudications are machines, and they are independent of each other: CI judged the real defect, and the shipped oracle judges the minimized reproduction. The strongest-adjudicated case in the corpus." + }, "recorded_at": "2026-07-27", "notes": "The strongest-adjudicated case in the corpus: its second adjudication is a machine. CI failed on the real defect after a clean review verdict, which is an independent, non-model judgement of materiality." } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json index f12bd2b..f1200b4 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/post-bootstrap-module-load.json @@ -4,6 +4,10 @@ "origin": "minimized_reproduction", "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 710, review comment 2961766206, authored by the repository owner. Public source, owner-authored review, no third-party or customer material.", "sanitization": "Rewritten from scratch against a fictional entry script. Retains only the shape of the adjudicated exchange: an unusual deferred module load flagged as strange, kept because the deferral is load-bearing, with an explanatory comment as the whole resolution. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 710 review comment 2961766206, dispositioned polish-only: a comment was added and no behaviour changed.", + "second": "oracle" + }, "recorded_at": "2026-07-27", "notes": "NEGATIVE CONTROL, polish-only disposition. The reviewer said the pattern was strange and asked for a comment explaining it; the owner added the comment and changed no behaviour, which adjudicates the pattern itself as intentional and correct. The rejected concern is recorded as anf.importlib-instead-of-a-plain-import." } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json index 2dc6ef6..501b1f9 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/process-isolation-assertion.json @@ -4,6 +4,11 @@ "origin": "repository_history", "retention_authority": "This repository's own delivery history. The finding was raised by a real `review-code-change` run during #50 and dispositioned `defer` under the review contract, then preserved rather than applied. It is recorded verbatim in the merged body of pull request 61, item 4 of the deferred list. Public, owner-authored, no retention question.", "sanitization": "Rewritten against a fictional harness rather than the real evaluator, so the case cannot be recognised as, or confused with, this repository's own test suite. The failure shape is retained exactly: a test named for process freshness that asserts a stable executor identity, in a suite that cannot observe a process identifier. Nothing private is involved.", + "adjudication": { + "first": "A real `review-code-change` run during #50 dispositioned this `defer`; it is item 4 of the preserved deferred list in the merged body of pull request 61.", + "second": "oracle", + "notes": "The first adjudication came from the review suite under evaluation, not from a human. An adjudicator must weigh that correlation: a `defer` from the same contract being measured is weaker evidence than a human disposition. The oracle is independent of it, but it can only adjudicate that the stated requirements hold - it cannot adjudicate whether the test's name overstates them, which is a judgement about prose." + }, "recorded_at": "2026-07-27", "notes": "NEGATIVE CONTROL, deferred-as-immaterial disposition, and the only case whose first adjudication was produced by the review suite under evaluation rather than by a human. Recorded as such because that is a correlation an adjudicator must weigh: the disposition came from the same contract being measured. The rejected concern is recorded as anf.name-asserts-identity-not-freshness." } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json index ba0a5a8..2274829 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/session-continuation-summary.json @@ -4,6 +4,11 @@ "origin": "minimized_reproduction", "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 486, review comment 2881737041, authored by the repository owner. Public source, owner-authored review, no third-party or customer material.", "sanitization": "Rewritten from scratch against a fictional worker. Retains only the shape of the adjudicated exchange: a summary flag that reads as a claim about behaviour is deliberately kept because it is the loop-continuation signal, and the reporting is corrected instead. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 486 review comment 2881737041, declined on the merits: the flag was kept and only the operator wording changed.", + "second": "oracle", + "notes": "The oracle adjudicates that the stated contract holds. It does not, and cannot, adjudicate the reviewer's aesthetic objection to the flag's name." + }, "recorded_at": "2026-07-27", "notes": "CLEAN CONTROL under the owner's settled standard: cleanliness rests on an adjudicated-rejected finding, not on absence of comment. The reviewer asked for `started=True` to change; the owner declined on the merits - the flag preserves loop continuation - and fixed only the operator-facing wording. The rejected concern is recorded as anf.started-true-without-an-agent so a non-gating mention is tolerated while gating on it is charged a false alarm. Multi-file case: the disposition only makes sense by reading the untouched loop that consumes the flag." } diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json index 0bf2a40..b2007ca 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/stale-claim-release-guard.json @@ -4,6 +4,10 @@ "origin": "minimized_reproduction", "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 373, review comment 2869270760, authored by the repository owner and accepted in the reply that added a regression test for the collect/apply window. Public source, owner-authored review, no third-party or customer material.", "sanitization": "Rewritten from scratch against a fictional claim queue. Retains only the failure shape: a collect-then-apply guard compares ownership only when the captured owner was present, so the unowned-at-capture interleaving bypasses it. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 373 review comment 2869270760, accepted with a regression test for the collect/apply window.", + "second": "oracle" + }, "recorded_at": "2026-07-27", "notes": "Multi-file case: the interleaving is only reachable by reading the untouched claim writer. Second adjudication is available by oracle." } diff --git a/review-suite/scripts/evals/oracles/README.md b/review-suite/scripts/evals/oracles/README.md new file mode 100644 index 0000000..dbbea7a --- /dev/null +++ b/review-suite/scripts/evals/oracles/README.md @@ -0,0 +1,53 @@ +# Executable oracles + +An oracle is a **second independent adjudication by machine**. For a correctness +case whose minimized reproduction can be made to run, materiality stops being an +opinion: the requirement the change contract states either holds against the +candidate code or it does not. + +This matters because of what the alternatives are. The first adjudication is the +recorded source disposition — strong for a root cause, thin for severity, and +unavailable for formulations. The obvious second adjudicator, a fresh agent +context, is **not** independent of a reviewer it shares a model family with: the +errors correlate, so recall would rise without the reviewer improving. A machine +that runs the code has no such correlation. Where an oracle exists, use it. + +## What an oracle asserts + +Each module exposes exactly one `ORACLE`, a `CaseOracle`: + +- `candidate()` — the reproduction in the state the candidate leaves it. +- `corrected()` — the same reproduction with the root cause fixed. `None` for a + clean-expected case, where there is nothing to correct. +- `check(subject)` — the requirement from the packet's change contract, + expressed as code. Returns `True` when the requirement holds. + +`review-suite/scripts/tests/test_eval_oracles.py` then asserts, per case: + +| expected verdict | on `candidate()` | on `corrected()` | +| ------------------ | ------------------------- | ------------------------- | +| `changes_required` | the requirement **fails** | the requirement **holds** | +| `clean` | the requirement **holds** | not applicable | + +The `corrected()` leg is what makes the oracle an adjudication of the *root +cause* rather than of the symptom: the stated cause, corrected, is what makes +the requirement hold. A case whose requirement fails for some other reason fails +this test. + +## What an oracle does not prove + +It covers the requirements the change contract states, and nothing else. An +oracle passing on a clean-expected case does **not** prove the diff is free of +every possible defect — only that the requirements it claims to satisfy are +satisfied. Read a clean control as "the stated contract holds and the raised +concern was adjudicated immaterial", never as "nothing is wrong here". + +An oracle is also not the reviewer's task. The reviewer sees a packet and must +reason from it; the oracle runs code the reviewer never receives. Oracles are +private curation evidence and live outside every corpus's `reviewer/` tree. + +## Where no oracle exists + +The simplicity strata have none: "this is over-engineered" and "this complexity +is requirement-justified" have no executable form. Those cases need the owner. +See [the adjudication plan](../../../evals/baseline/v1/ADJUDICATION-PLAN.md). diff --git a/review-suite/scripts/evals/oracles/__init__.py b/review-suite/scripts/evals/oracles/__init__.py new file mode 100644 index 0000000..5d2153c --- /dev/null +++ b/review-suite/scripts/evals/oracles/__init__.py @@ -0,0 +1,51 @@ +"""Executable oracles: a second, machine adjudication of a case's materiality. + +Loading is explicit rather than implicit. `oracle_module` resolves a case +identifier to its module so a missing oracle is a clear absence rather than an +import error, because most cases legitimately have none. +""" + +from __future__ import annotations + +import importlib +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Callable + +ORACLE_ROOT = Path(__file__).resolve().parent + + +@dataclass(frozen=True) +class CaseOracle: + """One case's runnable reproduction and the requirement it must satisfy. + + `corrected` is `None` for a clean-expected case: there is no root cause to + correct, and inventing one would make the oracle assert something the + corpus does not claim. + """ + + case_id: str + requirement: str + candidate: Callable[[], Any] + check: Callable[[Any], bool] + corrected: Callable[[], Any] | None = None + + +def case_ids() -> list[str]: + """Every case identifier that ships an oracle.""" + return sorted( + path.stem.replace("_", "-") + for path in ORACLE_ROOT.glob("*.py") + if path.stem != "__init__" + ) + + +def load(case_id: str) -> CaseOracle: + """Return one case's oracle, failing loudly on a malformed module.""" + module = importlib.import_module(f"{__name__}.{case_id.replace('-', '_')}") + oracle = getattr(module, "ORACLE", None) + if not isinstance(oracle, CaseOracle): + raise TypeError(f"{module.__name__} does not export a CaseOracle as ORACLE") + if oracle.case_id != case_id: + raise ValueError(f"{module.__name__} declares case_id {oracle.case_id!r}") + return oracle diff --git a/review-suite/scripts/evals/oracles/dependency_hint_parser_coverage.py b/review-suite/scripts/evals/oracles/dependency_hint_parser_coverage.py new file mode 100644 index 0000000..8b6eb29 --- /dev/null +++ b/review-suite/scripts/evals/oracles/dependency_hint_parser_coverage.py @@ -0,0 +1,56 @@ +"""Oracle: the parser reads the canonical field and ignores retired aliases. + +Requirements, from the packet's acceptance criteria: the canonical link field is +read, a retired alias is not, and a bare string entry keeps working. All three +hold against the candidate, which is why the request to add coverage pinning a +retired alias was declined on the merits: it would pin behaviour the change +exists to remove. + +Clean-expected, so there is no `corrected` leg. +""" + +from __future__ import annotations + +from . import CaseOracle + +CANONICAL_LINK = "link_type" + + +class _Candidate: + @staticmethod + def parent_links(entry): + links = [] + for item in entry.get("dependencies") or []: + if isinstance(item, str): + links.append(item) + continue + if item.get(CANONICAL_LINK) == "parent_child": + links.append(item["issue"]["id"]) + return links + + +def _check(subject) -> bool: + canonical = { + "dependencies": [{"link_type": "parent_child", "issue": {"id": "p-2"}}] + } + retired = {"dependencies": [{"type": "parent_child", "issue": {"id": "p-3"}}]} + also_retired = { + "dependencies": [{"relation": "parent_child", "issue": {"id": "p-4"}}] + } + return ( + subject.parent_links(canonical) == ["p-2"] + and subject.parent_links(retired) == [] + and subject.parent_links(also_retired) == [] + and subject.parent_links({"dependencies": ["p-1"]}) == ["p-1"] + ) + + +ORACLE = CaseOracle( + case_id="dependency-hint-parser-coverage", + requirement=( + "The canonical link field is read, a retired alias is not, and a bare " + "string entry keeps working." + ), + candidate=_Candidate, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py b/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py new file mode 100644 index 0000000..4779e0b --- /dev/null +++ b/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py @@ -0,0 +1,86 @@ +"""Oracle: the untouched sibling call site keeps deciding permissively. + +Requirement, from the packet's acceptance criteria: every path that decides +whether a record is integrated applies the same strictness rule for a closed +record. The check asks both decision paths about the one record shape the change +exists to catch - closed, carrying a merge marker, with no target-branch proof - +and requires them to agree. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from . import CaseOracle + + +@dataclass +class Record: + status: str = "open" + merge_marker: bool = False + target_ref: str | None = None + + +def _integration_proven(record: Record, *, require_target_proof: bool = False) -> bool: + if require_target_proof: + return bool(record.target_ref) + return bool(record.merge_marker or record.target_ref) + + +class _Candidate: + """`reconcile_closed` hardened; `dependency_finalized` left as it was.""" + + @staticmethod + def reconcile_decides_integrated(record: Record) -> bool: + return _integration_proven( + record, require_target_proof=record.status == "closed" + ) + + @staticmethod + def dependency_decides_integrated(record: Record) -> bool: + return _integration_proven(record) + + +class _Corrected: + """Both call sites adopt the strict mode for a closed record.""" + + @staticmethod + def reconcile_decides_integrated(record: Record) -> bool: + return _integration_proven( + record, require_target_proof=record.status == "closed" + ) + + @staticmethod + def dependency_decides_integrated(record: Record) -> bool: + return _integration_proven( + record, require_target_proof=record.status == "closed" + ) + + +def _check(subject) -> bool: + abandoned = Record(status="closed", merge_marker=True, target_ref=None) + landed = Record(status="closed", merge_marker=True, target_ref="refs/heads/main") + open_record = Record(status="open", merge_marker=True, target_ref=None) + return ( + # The two decision paths must agree on the closed record. + subject.reconcile_decides_integrated(abandoned) + == subject.dependency_decides_integrated(abandoned) + # ...and must agree on the answer being "not integrated". + and subject.dependency_decides_integrated(abandoned) is False + # A genuinely landed closed record stays integrated on both paths. + and subject.dependency_decides_integrated(landed) is True + # An open record keeps the permissive behaviour the contract preserves. + and subject.dependency_decides_integrated(open_record) is True + ) + + +ORACLE = CaseOracle( + case_id="dependency-strictness-propagation", + requirement=( + "Every path that decides whether a record is integrated applies the same " + "strictness rule for a closed record." + ), + candidate=_Candidate, + corrected=_Corrected, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/optional_tool_probe.py b/review-suite/scripts/evals/oracles/optional_tool_probe.py new file mode 100644 index 0000000..0ea1913 --- /dev/null +++ b/review-suite/scripts/evals/oracles/optional_tool_probe.py @@ -0,0 +1,70 @@ +"""Oracle: the probe cannot detect an absent executable. + +Requirement, from the packet's acceptance criteria: the suite skips these tests, +rather than failing, when the runner is absent. The check runs the probe against +an executable name that does not exist - which is what the packet says CI is - +and requires a skip rather than an error. + +This is the one case in the corpus whose real counterpart was already adjudicated +by a machine: the defect survived an aggregate `clean` review verdict and CI +caught it. The oracle reproduces that judgement locally. +""" + +from __future__ import annotations + +import subprocess +import unittest + +from . import CaseOracle + +ABSENT = "taskr-executable-that-does-not-exist" + + +def _candidate(): + """Probe as the candidate wrote it: inspect the return code only.""" + + def probe(): + if ( + subprocess.run([ABSENT, "--version"], capture_output=True, check=False) + ).returncode != 0: + raise unittest.SkipTest("taskr is not installed") + + return probe + + +def _corrected(): + """Probe that distinguishes absent from present-and-failing.""" + + def probe(): + try: + completed = subprocess.run( + [ABSENT, "--version"], capture_output=True, check=False + ) + except OSError as error: + raise unittest.SkipTest(f"taskr is unavailable: {error}") from error + if completed.returncode != 0: + raise unittest.SkipTest("taskr is present but not runnable") + + return probe + + +def _check(probe) -> bool: + """The requirement: an absent executable produces a skip, not an error.""" + try: + probe() + except unittest.SkipTest: + return True + except OSError: + return False + return False + + +ORACLE = CaseOracle( + case_id="optional-tool-probe", + requirement=( + "The suite skips these tests, rather than failing, when the runner is absent." + ), + candidate=_candidate, + corrected=_corrected, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/post_bootstrap_module_load.py b/review-suite/scripts/evals/oracles/post_bootstrap_module_load.py new file mode 100644 index 0000000..dae4db8 --- /dev/null +++ b/review-suite/scripts/evals/oracles/post_bootstrap_module_load.py @@ -0,0 +1,57 @@ +"""Oracle: the module is bound after the runtime selection reorders the path. + +Requirement, from the packet's acceptance criteria: the store module is loaded +after the runtime selection has run, so the entry path binds the selected runtime +rather than whichever copy was importable first. That is exactly what a +module-level import cannot do, which is why the deferred load was adjudicated +intentional and only a comment was added. + +Clean-expected, so there is no `corrected` leg. The check models the ordering +directly: a resolver whose answer depends on selection having run first. +""" + +from __future__ import annotations + +from . import CaseOracle + + +class _Runtime: + """Stands in for `sys.path`: the answer depends on selection order.""" + + def __init__(self): + self.selected = False + self.load_order = [] + + def select_runtime(self): + self.selected = True + self.load_order.append("select_runtime") + + def import_module(self, name): + self.load_order.append(name) + return "selected" if self.selected else "stale-installed-copy" + + +def _candidate(): + """Deferred load: selection runs, then the module is resolved.""" + runtime = _Runtime() + runtime.select_runtime() + store = runtime.import_module("toolkit.store") + return runtime, store + + +def _check(result) -> bool: + runtime, store = result + return store == "selected" and runtime.load_order.index( + "select_runtime" + ) < runtime.load_order.index("toolkit.store") + + +ORACLE = CaseOracle( + case_id="post-bootstrap-module-load", + requirement=( + "The store module is loaded after the runtime selection has run, so the " + "entry path binds the selected runtime." + ), + candidate=_candidate, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/process_isolation_assertion.py b/review-suite/scripts/evals/oracles/process_isolation_assertion.py new file mode 100644 index 0000000..9376a2d --- /dev/null +++ b/review-suite/scripts/evals/oracles/process_isolation_assertion.py @@ -0,0 +1,48 @@ +"""Oracle: every attempt produces exactly one attributable retained artifact. + +Requirements, from the packet's acceptance criteria: every attempt produces +exactly one retained artifact, and the artifacts are attributable to the executor +that produced them. Both hold. What does *not* hold is anything about +operating-system process identity - the packet states the suite cannot observe a +process identifier at all - which is why the observation that the test's name +overstates its assertion was dispositioned as immaterial rather than as a defect. + +Clean-expected, so there is no `corrected` leg. Note what this oracle cannot do: +it cannot adjudicate whether the *name* is misleading, because that is a +judgement about prose. It adjudicates only that the stated requirements hold. +""" + +from __future__ import annotations + +from . import CaseOracle + +EXECUTOR_NAME = "bundled-executor" + + +def _candidate(): + """Two attempts, each writing one retained artifact.""" + artifacts = {} + records = [] + for run_number in (1, 2): + records.append({"run_number": run_number}) + artifacts[f"attempt.run-{run_number}.stdout.json"] = { + "executor": {"name": EXECUTOR_NAME} + } + return records, artifacts + + +def _check(result) -> bool: + records, artifacts = result + names = {doc["executor"]["name"] for doc in artifacts.values()} + return len(records) == len(artifacts) and names == {EXECUTOR_NAME} + + +ORACLE = CaseOracle( + case_id="process-isolation-assertion", + requirement=( + "Every attempt produces exactly one retained artifact, and the artifacts " + "are attributable to the executor that produced them." + ), + candidate=_candidate, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/session_continuation_summary.py b/review-suite/scripts/evals/oracles/session_continuation_summary.py new file mode 100644 index 0000000..8523022 --- /dev/null +++ b/review-suite/scripts/evals/oracles/session_continuation_summary.py @@ -0,0 +1,88 @@ +"""Oracle: the summary flag drives loop continuation, and reporting is honest. + +Requirements, from the packet's acceptance criteria: the run loop continues after +a finalize-only job, and operator output never claims an agent session was +started for one. Both hold against the candidate, which is why the concern that +`started=True` is inaccurate was adjudicated immaterial: the flag is the +continuation signal, and the reporter is what speaks to operators. + +Clean-expected, so there is no `corrected` leg. This proves the stated contract +holds; it does not prove nothing else could be wrong. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +from . import CaseOracle + + +@dataclass +class RunSummary: + started: bool + reason: str + + +@dataclass +class _Control: + lines: list[str] = field(default_factory=list) + + def say(self, line): + self.lines.append(line) + + +class _Candidate: + @staticmethod + def run_once(*, finalize_only: bool) -> RunSummary: + if finalize_only: + return RunSummary(started=True, reason="finalize_only") + return RunSummary(started=True, reason="agent_session") + + @staticmethod + def report_summary(summary: RunSummary, control: _Control) -> None: + if summary.reason == "finalize_only": + control.say("continued without an agent session (finalize-only preflight)") + return + if summary.started: + control.say("started an agent session") + + @staticmethod + def loop_continues(summary: RunSummary) -> bool: + """The loop reads `started` and nothing else.""" + return summary.started + + +def _check(subject) -> bool: + finalize = subject.run_once(finalize_only=True) + agent = subject.run_once(finalize_only=False) + + finalize_control = _Control() + subject.report_summary(finalize, finalize_control) + agent_control = _Control() + subject.report_summary(agent, agent_control) + + return ( + # The loop continues after a finalize-only job. + subject.loop_continues(finalize) is True + # Operator output never claims a session started for finalize-only. + and not any( + "started an agent session" in line for line in finalize_control.lines + ) + and any( + "continued without an agent session" in line + for line in finalize_control.lines + ) + # A job that does start an agent reports exactly as before. + and agent_control.lines == ["started an agent session"] + ) + + +ORACLE = CaseOracle( + case_id="session-continuation-summary", + requirement=( + "The run loop continues after a finalize-only job, and operator output " + "never claims an agent session was started for one." + ), + candidate=_Candidate, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/stale_claim_release_guard.py b/review-suite/scripts/evals/oracles/stale_claim_release_guard.py new file mode 100644 index 0000000..2ba098b --- /dev/null +++ b/review-suite/scripts/evals/oracles/stale_claim_release_guard.py @@ -0,0 +1,90 @@ +"""Oracle: the ownership guard is skipped when the captured owner was absent. + +Requirement, from the packet's acceptance criteria: a claim taken by another +worker between scan and apply is never released. The check drives the exact +interleaving - unowned at scan, owned at apply - and requires the claim to +survive. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from . import CaseOracle + +TTL = 60 + + +@dataclass +class Entry: + entry_id: str + owner: str | None + claimed_at: int + + +class _Store: + def __init__(self, entries): + self._entries = {entry.entry_id: entry for entry in entries} + + def scan(self): + return list(self._entries.values()) + + def load(self, entry_id): + return self._entries.get(entry_id) + + def set_owner(self, entry_id, owner): + self._entries[entry_id].owner = owner + + def release(self, entry_id): + self._entries[entry_id].owner = None + self._entries[entry_id].claimed_at = 0 + + +def _collect(now, store, *, guard_unconditionally: bool): + actions = [] + for entry in store.scan(): + if entry.claimed_at is None or now - entry.claimed_at <= TTL: + continue + snapshot_owner = entry.owner + + def _release(entry_id=entry.entry_id, snapshot_owner=snapshot_owner): + current = store.load(entry_id) + if current is None: + return + if guard_unconditionally: + if current.owner != snapshot_owner: + return + elif snapshot_owner is not None and current.owner != snapshot_owner: + return + store.release(entry_id) + + actions.append(_release) + return actions + + +def _run(*, guard_unconditionally: bool): + """Scan an expired unowned entry, let a worker claim it, then apply.""" + store = _Store([Entry(entry_id="e-1", owner=None, claimed_at=0)]) + actions = _collect( + now=TTL * 10, store=store, guard_unconditionally=guard_unconditionally + ) + store.set_owner("e-1", "worker-b") # another worker claims it after the scan + for action in actions: + action() + return store + + +def _check(store) -> bool: + # The live claim taken after the scan must still be held. + return store.load("e-1").owner == "worker-b" + + +ORACLE = CaseOracle( + case_id="stale-claim-release-guard", + requirement=( + "A claim taken by another worker between scan and apply is never released." + ), + candidate=lambda: _run(guard_unconditionally=False), + corrected=lambda: _run(guard_unconditionally=True), + check=_check, +) diff --git a/review-suite/scripts/tests/test_eval_oracles.py b/review-suite/scripts/tests/test_eval_oracles.py new file mode 100644 index 0000000..8f3f9e3 --- /dev/null +++ b/review-suite/scripts/tests/test_eval_oracles.py @@ -0,0 +1,164 @@ +"""Oracle tests: the machine half of a case's independent adjudication. + +An oracle is only an adjudication if it is actually executed and actually +discriminates. These tests run each case's requirement against the reproduction +the candidate leaves behind, and against the corrected reproduction where one +exists, and assert the polarity the case's expected verdict demands. + +The `corrected` leg is what makes this an adjudication of the *root cause* rather +than of a symptom: correcting the stated cause is what must make the requirement +hold. A case whose requirement fails for some unrelated reason fails here. + +Nothing in this module launches a runtime or spends money. +""" + +from __future__ import annotations + +import sys +import unittest +from pathlib import Path + +SCRIPTS_DIR = Path(__file__).resolve().parents[1] +if str(SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(SCRIPTS_DIR)) + +from evals import calibration, corpus, oracles # noqa: E402 + + +def _expectation_for(case_id: str) -> dict: + for _, case in calibration.cases_by_id()[case_id]: + return case.expectation + raise AssertionError(f"no corpus case {case_id!r}") + + +class OracleContractTests(unittest.TestCase): + """The oracle set must be well formed and must name real cases.""" + + @classmethod + def setUpClass(cls): + cls.case_ids = oracles.case_ids() + cls.corpus_cases = calibration.cases_by_id() + + def test_at_least_one_oracle_ships(self): + self.assertTrue(self.case_ids) + + def test_every_oracle_names_a_real_case(self): + for case_id in self.case_ids: + with self.subTest(case_id=case_id): + self.assertIn(case_id, self.corpus_cases) + + def test_every_oracle_declares_its_own_case_id(self): + for case_id in self.case_ids: + with self.subTest(case_id=case_id): + self.assertEqual(case_id, oracles.load(case_id).case_id) + + def test_a_clean_expected_case_has_no_corrected_leg(self): + """There is nothing to correct, and inventing one would assert a defect.""" + for case_id in self.case_ids: + expectation = _expectation_for(case_id) + if expectation["expected_verdict"] != "clean": + continue + with self.subTest(case_id=case_id): + self.assertIsNone(oracles.load(case_id).corrected) + + def test_a_gating_case_ships_a_corrected_leg(self): + """Without it the oracle cannot tell the root cause from a symptom.""" + for case_id in self.case_ids: + expectation = _expectation_for(case_id) + if expectation["expected_verdict"] != "changes_required": + continue + with self.subTest(case_id=case_id): + self.assertIsNotNone(oracles.load(case_id).corrected) + + +class OracleAdjudicationTests(unittest.TestCase): + """Each requirement must fail or hold exactly as the verdict demands.""" + + def test_the_requirement_fails_on_every_gating_candidate(self): + for case_id in oracles.case_ids(): + expectation = _expectation_for(case_id) + if expectation["expected_verdict"] != "changes_required": + continue + oracle = oracles.load(case_id) + with self.subTest(case_id=case_id): + self.assertFalse( + oracle.check(oracle.candidate()), + f"{case_id}: the stated requirement holds against the " + "candidate, so this case does not demonstrate a defect", + ) + + def test_correcting_the_root_cause_makes_the_requirement_hold(self): + for case_id in oracles.case_ids(): + expectation = _expectation_for(case_id) + if expectation["expected_verdict"] != "changes_required": + continue + oracle = oracles.load(case_id) + with self.subTest(case_id=case_id): + self.assertTrue( + oracle.check(oracle.corrected()), + f"{case_id}: correcting the stated root cause does not make " + "the requirement hold, so the stated cause is not the cause", + ) + + def test_the_requirement_holds_on_every_clean_candidate(self): + for case_id in oracles.case_ids(): + expectation = _expectation_for(case_id) + if expectation["expected_verdict"] != "clean": + continue + oracle = oracles.load(case_id) + with self.subTest(case_id=case_id): + self.assertTrue( + oracle.check(oracle.candidate()), + f"{case_id}: the stated requirement does not hold, so this " + "case cannot serve as a clean control", + ) + + +class OracleCoverageTests(unittest.TestCase): + """Where an oracle is available it must be shipped, and it must be recorded.""" + + def test_every_case_in_the_correctness_stratum_has_an_oracle(self): + """The correctness stratum is the one where an oracle is always possible. + + A correctness requirement is a statement about behaviour, so it can be + run. Leaving one un-oracled would silently route a case to the owner that + a machine could have settled. + """ + root = corpus.STRATA_ROOT / "s1-correctness-orchestrator" + if not root.is_dir(): + self.skipTest("the correctness stratum is not populated") + shipped = set(oracles.case_ids()) + for case in corpus.load_corpus(root).cases: + with self.subTest(case_id=case.case_id): + self.assertIn(case.case_id, shipped) + + def test_an_oracle_settled_case_records_that_in_its_provenance(self): + """A reader must never have to infer how a case was adjudicated.""" + shipped = set(oracles.case_ids()) + for case_id in shipped: + for _, case in calibration.cases_by_id()[case_id]: + with self.subTest(case_id=case_id): + adjudication = case.provenance.get("adjudication") + self.assertIsNotNone(adjudication) + self.assertEqual("oracle", adjudication["second"]) + self.assertTrue(adjudication["first"].strip()) + + def test_a_case_without_an_oracle_routes_to_the_owner(self): + """`owner_required` is the only honest alternative to an oracle. + + A fresh agent context is not a second adjudicator for materiality when it + shares a model family with the reviewer being measured, so a case with no + oracle has to say it needs the owner rather than quietly claiming two. + """ + shipped = set(oracles.case_ids()) + for root in corpus.corpus_roots(): + for case in corpus.load_corpus(root).cases: + adjudication = case.provenance.get("adjudication") + if adjudication is None or case.case_id in shipped: + continue + with self.subTest(stratum=root.name, case_id=case.case_id): + self.assertEqual("owner_required", adjudication["second"]) + + +if __name__ == "__main__": + unittest.main() From e83da75687f06ec9ff6a82df5ac4845c6e6fb23f Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Mon, 27 Jul 2026 08:57:31 -0700 Subject: [PATCH 3/5] docs: record the batch-2 delivery, the clean-control standard, and its limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Record the seven delivered cases in `SOURCING.md` with their source dispositions, and record all five candidates dropped for failing the clean-control standard. - Record the oracle outcome per case in `ADJUDICATION-PLAN.md`, including the three limits of what an oracle can settle and the three expectations that verification reversed. - Add limitations 13 to 15: false-alarm rate is a lower bound on invention; a scored stratum cannot be both calibrated and result-blind; an oracle adjudicates a requirement, not a diff. - Update `frozen-configuration.json`: the ceiling is preregistered, the clean-control standard is settled, and the remaining blocker is named. - Apply the two factual corrections carried over from batch 1. **Cost ceiling** is now recorded as preregistered at 9.00 / 3.00 / 3.00 USD, 15.00 total, with the hard-stop semantics stated: exceeding it stops further runs in that stratum and records incomplete evidence rather than quietly reducing repetitions. **Clean-control standard** — a clean control must be an adjudicated-rejected finding — invalidated five candidates, and the record says so rather than quietly replacing them. Both of batch 1's clean-control candidates failed: one rested on the reviewer praising a hunk, the other was an accepted *fix*, and acceptance says a concern was material, which is the opposite of what a clean control needs. Two negative-control candidates turned out on re-check to have been **accepted**, not deferred. A fifth was declined and then implemented in another form, so its disposition is acceptance too. Verification also reversed the plan in the other direction: atelier PR 335, which batch 1 recorded as lacking a verified acceptance, **was** accepted, which makes it a valid escape and now the multi-file propagation case. The standard measures whether a reviewer re-raises a finding a human already rejected. It does **not** measure whether a reviewer invents a novel finding on a wholly clean diff. A reviewer could score 0.0 across every control here and still gate freely on diffs where nothing was ever raised. Limitation 13 states this and requires the figure to be reported as a lower bound on invention rather than as a general false-alarm rate. Limitation 14 is new and it is not a caveat, it is a blocker. Grader matching is containment, so an uncalibrated expectation reports recall 0.0 against a reviewer that is right every time — batch 1 measured exactly that. But calibrating a case means observing its prose, and observing a scored case's prose then tuning its formulations is fitting the grader to the answer. So the stratum ships populated, `scored: false`, every expectation `calibrated: false`, and **no case run through any runtime**. Four resolutions are set out for the owner to preregister, with their costs: rely on transfer and accept a downward-biased recall; split each class between calibration and scoring; report referrals as a first-class bucket; or replace containment matching, which belongs to #59. --- CHANGELOG.md | 4 + .../evals/baseline/v1/ADJUDICATION-PLAN.md | 55 ++++++++++ review-suite/evals/baseline/v1/CALIBRATION.md | 4 +- review-suite/evals/baseline/v1/LIMITATIONS.md | 102 +++++++++++++++++- review-suite/evals/baseline/v1/SOURCING.md | 90 ++++++++++++---- .../baseline/v1/frozen-configuration.json | 47 +++++--- review-suite/evals/strata/README.md | 17 +++ 7 files changed, 280 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d13224c..b15b9e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,11 @@ summary: Chronological history of repository and skill changes. ## 2026-07-27 — Populated the correctness stratum, and recovered carved suffixes +- docs: record the batch-2 delivery, the clean-control standard, and its limits +- feat: adjudicate the correctness cases by executable oracle + (`6dcfeabc7acd325d1dcaae4ed341fa780df94bc9`) - feat: populate the correctness stratum with seven adjudicated cases + (`43deec617ee06e22e1a937234eea2a4d99b5d836`) - feat: recover corrected carved suffixes (`ba12e0744a938fc71af16eeeaa0eea98e7c2c63e`) diff --git a/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md b/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md index 78a2314..c50ed78 100644 --- a/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md +++ b/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md @@ -135,6 +135,61 @@ every near-miss control in `s2-solution-simplicity-lens` and `s3-code-simplicity-lens` therefore needs human adjudication, or must be reported with the correlated-judgment limitation attached. +## 2b. Outcome: the oracle method, applied + +The proposal above was adopted for `s1-correctness-orchestrator` and it settled +every case in it. `review-suite/scripts/evals/oracles/` ships one runnable +module per case, and `test_eval_oracles.py` asserts the polarity each expected +verdict demands: the requirement must **fail** against a gating candidate and +**hold** once the stated root cause is corrected, and must **hold** against a +clean candidate. + +| case | second adjudication | what the oracle settled | +| ----------------------------------- | ------------------- | -------------------------------------------------------------------------------------------------------- | +| `dependency-strictness-propagation` | oracle | The two decision paths disagree on the closed record; correcting the sibling call site makes them agree. | +| `stale-claim-release-guard` | oracle | The unowned-at-scan interleaving releases a live claim; an unconditional owner comparison prevents it. | +| `optional-tool-probe` | oracle | The probe raises instead of skipping; catching `OSError` restores the skip. | +| `session-continuation-summary` | oracle | The loop continues and the reporter never claims a session, so the contract holds. | +| `dependency-hint-parser-coverage` | oracle | The canonical field is read and both retired aliases are ignored. | +| `post-bootstrap-module-load` | oracle | The module binds after selection, which a module-level import cannot do. | +| `process-isolation-assertion` | oracle | One retained artifact per attempt, attributable to its executor. | + +**Owner still required for:** every case in `s2-solution-simplicity-lens` and +`s3-code-simplicity-lens`. "This is over-engineered" and "this complexity is +requirement-justified" have no executable form, so those cases will declare +`owner_required`, and a test holds them to that rather than letting them claim +two adjudications they do not have. + +### The three limits of what was settled + +1. An oracle adjudicates the **stated requirements**, not the diff. A clean + control's oracle proves the contract holds and the raised concern was + immaterial; it does not prove nothing else is wrong. +2. An oracle cannot adjudicate prose. It cannot settle whether a field's name + reads as a claim it should not make (`session-continuation-summary`) or + whether a test's name overstates its assertion + (`process-isolation-assertion`). Both record this. +3. `process-isolation-assertion`'s **first** adjudication came from the review + suite under evaluation — a `defer` from the same contract being measured. The + oracle is independent of it, but the first adjudication is weaker than a + human disposition and its provenance says so. + +### Verification changed three of the expectations below + +The disagreement table in section 4 was written before the source dispositions +were re-checked. Checking them resolved three entries, and two resolutions were +the opposite of what the table assumed: + +- **atelier PR 335 was accepted** (`5cb0333`). It is a valid escape, not an + ambiguous one, and it is now the multi-file propagation case. +- **atelier PR 333 and PR 356 were both accepted**, not deferred. Both are + unusable as negative controls and were dropped. +- **Both clean-control candidates failed** the owner's settled standard and were + replaced. See [SOURCING.md](SOURCING.md) for all five dropped candidates. + +The clean-control question the table flagged as the weakest slot is now settled +by the owner, and the corpus is built to that standard. + ## 3. Expected workload Fifteen scored cases across three strata: roughly 12–16 material root causes, diff --git a/review-suite/evals/baseline/v1/CALIBRATION.md b/review-suite/evals/baseline/v1/CALIBRATION.md index 60ea2a2..433743e 100644 --- a/review-suite/evals/baseline/v1/CALIBRATION.md +++ b/review-suite/evals/baseline/v1/CALIBRATION.md @@ -40,8 +40,8 @@ and closure digest rather than by a commit: no commit reproduces it. > **before any attempt launches** when a run would overwrite retained output, so > the failure is a refusal that costs nothing rather than a silent replacement. > And retained output now goes to a version-scoped directory, -> `artifacts///`, which is the artifact path recorded -> in the frozen per-stratum invocations. +> `artifacts//-/`, which is the artifact path +> recorded in the frozen per-stratum invocations. > > The committed `observed` probes are therefore verbatim, byte for byte, from > the **first post-calibration run**, whose output is retained at diff --git a/review-suite/evals/baseline/v1/LIMITATIONS.md b/review-suite/evals/baseline/v1/LIMITATIONS.md index f5c2bb0..5f4e680 100644 --- a/review-suite/evals/baseline/v1/LIMITATIONS.md +++ b/review-suite/evals/baseline/v1/LIMITATIONS.md @@ -249,8 +249,100 @@ the source. `runner.refuse_to_overwrite_artifacts` fails before launching any attempt when a run would overwrite a retained raw stdout artifact. The per-attempt records, the -aggregate report, and the baseline report are still written unconditionally. The -frozen invocations give every output the same `-` stem, -so in practice the raw-artifact collision fires first and nothing is lost — but -that is the invocation's property, not the runner's. Widening the guard is a -deferred finding. +aggregate report, and the baseline report are still written unconditionally. + +**Only some of those paths are protected in practice, not all of them.** The +frozen invocations give `--artifact-dir` and `--attempts-out` the same +`-` stem, so a re-run into a new stem writes beside the +old records rather than over them. Every `--report-out` is a **stemless fixed +path** — `baseline/v1/pilot/pilot-.report.json` and +`baseline/v1/.report.json` — so a re-run at a new corpus version +replaces a committed report **without the guard firing**. That is not +hypothetical: it is how this ticket's own committed pilot reports were replaced +between batches. + +The only thing protecting a committed report is that `baseline/v1/` is tracked +in git, so an overwrite is visible in `git status` and recoverable from history. +That is a real protection and a weak one, because it depends on someone looking. +Widening the guard to every output path remains a deferred finding. + +## 13. False-alarm rate is a lower bound on invention, not a general rate + +Every clean control in this corpus is an **adjudicated-rejected finding**: a +case where a concern was actually raised and dispositioned as not material. That +is the owner's settled standard, and it is the right one — the alternative, +treating a comment-free candidate as clean, is ambiguous between +reviewed-and-clean and nobody-looked, and would charge a false alarm against a +reviewer that correctly found a real unnoticed defect. + +The standard has a boundary that must travel with every figure derived from it. +**It measures whether a reviewer re-raises a finding a human already rejected. +It does not measure whether a reviewer invents a novel finding on a wholly clean +diff.** Those are different failure modes, and only the first is instrumented +here. + +So report false-alarm rate from these controls as a **lower bound on +invention**. A reviewer could score a perfect 0.0 on every one of them and still +gate freely on diffs where nothing was ever raised. Measuring that would need a +different control class — a candidate adjudicated clean by construction rather +than by rejection — which this corpus does not contain and which the standard +above deliberately declines to fake. + +## 14. A scored stratum cannot be both calibrated and result-blind + +This is the sharpest open question the corpus has, and it blocks scoring rather +than merely qualifying it. + +Batch 1 measured what an uncalibrated expectation reports: recall 0.0 over five +attempts against a reviewer that found the defect every time, because grader +matching is containment and the shipped formulations had never met real prose. +Calibration fixed it — recall 1.0 — but calibration requires *observing the +reviewer's prose for that case*. Observing a scored case's prose and then tuning +its formulations is fitting the grader to the answer, which is exactly what the +non-goals forbid. + +`s1-correctness-orchestrator` is therefore populated with `scored: false` and +every expectation `calibrated: false`, and **no case in it has been run through +any runtime**. Both states are honest, and neither is a resting place: scored as +it stands, it would report a number about the corpus. Four resolutions exist, +and the owner has to pick one: + +1. **Rely on transfer.** Calibrate only on the disjoint pilot cases and accept + that scored formulations are untuned. Measured to transfer once — the pilot's + calibrated formulations held on two later runs — but transfer is not + guaranteed, and untuned formulations bias recall **downward**, so the + baseline would understate the reviewer. +2. **Split each case class.** Calibrate on half, score the other half. Costs + corpus size, which the per-stratum minima already constrain. +3. **Report referrals as a first-class bucket.** Score matched, missed, and + *referred for adjudication* separately, so a containment miss is visible as a + grader limitation instead of silently becoming a reviewer miss. Cheapest, and + it makes the existing `adjudication_required` output load-bearing. +4. **Replace containment matching** with semantic matching or a standing + adjudication queue. A v2 mechanism, so it belongs to #59. + +Whichever is chosen, it must be preregistered with the rest of the frozen +configuration, because it decides what recall means. + +## 15. An oracle adjudicates a requirement, not a diff + +Every case in `s1-correctness-orchestrator` carries an executable oracle as its +second adjudication, and that is genuinely independent of the reviewer being +measured — a machine that runs the code shares no blind spot with a model. But +an oracle asserts only the requirements the change contract states. + +- On a gating case it proves the requirement fails and that correcting the + stated root cause makes it hold, which adjudicates both materiality and the + identity of the cause. +- On a clean control it proves the stated contract holds. **It does not prove + the diff is free of every possible defect**, so a clean control means "the + contract holds and the raised concern was adjudicated immaterial", never + "nothing is wrong here". +- It cannot adjudicate a judgement about prose. Two cases record this: an oracle + cannot settle whether a field's name reads as a claim it should not make, nor + whether a test's name overstates what it asserts. + +One case carries a further caveat. `process-isolation-assertion`'s first +adjudication came from the review suite **under evaluation** — a `defer` verdict +from the same contract being measured — which is weaker evidence than a human +disposition and is recorded as such in its provenance. diff --git a/review-suite/evals/baseline/v1/SOURCING.md b/review-suite/evals/baseline/v1/SOURCING.md index e63fd7a..4190a09 100644 --- a/review-suite/evals/baseline/v1/SOURCING.md +++ b/review-suite/evals/baseline/v1/SOURCING.md @@ -110,21 +110,71 @@ The candidate ground truth below was identified while sourcing this batch and is recorded so the evidence is not lost. Each still requires the adjudication trail to be re-verified at the source and the reproduction to be minimized fresh. -### Batch 2 — `s1-correctness-orchestrator`, 7 cases - -| class | candidate ground truth | -| ----------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| multi-file contract or untouched-consumer propagation failure | `shaug/atelier` PR 335, comment 2867594627: a strictness flag was added to one call site while a sibling call site kept calling the same helper without it, so a closed dependency could still be treated as finalized through non-strict signals. Alternate: PR 350, comments 2867972061 and 2867993101 — the same coupling defect in two separate files, both recovery paths skipping external-state reconciliation. | -| concurrency, retry, idempotency, transaction, or data-integrity failure | `shaug/atelier` PR 373, comment 2869270760: a stale-snapshot cleanup guard compared assignees only when the stale assignee was non-null, so a claim taken between collect and apply could be cleared. Alternate: PR 674, comment 2937093107 — one atomic note-plus-status write split into two, so a later failure leaves a fresh blocked reason on a changeset that is not blocked. | -| validation gap where passing tests did not exercise the changed risk | `shaug/agent-scripts` commit `f544aa0`: a probe for an optional executable checked only the return code, but a missing executable makes the call raise, so the intended skip never happened and the whole suite errored. It **survived an aggregate `clean` review verdict at head `b605051`** and was caught by CI. Complete provenance, this repository, no retention question. Alternate: `shaug/atelier` PR 318, comment 2867098097 — a tautological self-ancestor success when two branch references point at the same branch. | -| clean correctness control | `shaug/atelier` PR 335: the hardening hunk in isolation, which the reviewer explicitly adjudicated as correct in the same thread that raised the sibling gap. | -| clean correctness control | `shaug/atelier` PR 417, comment 2870710594: a typed outcome replacing a boolean, implemented and accepted, with the reviewer's stated requirement met. | -| adjudicated rejected or declined finding as negative control | `shaug/atelier` PR 279, comment 2862037362: a reviewer-suggested fallback regression path was **declined on the merits** after the parser was intentionally narrowed to one canonical shape. | -| adjudicated speculative or polish-only finding as negative control | `shaug/atelier` PR 333, comment 2867594626 (a scope question plus "if exclusion is intentional, a brief rationale would help") or PR 356, comment 2868540467 ("possible edge case to validate ... or add a regression test to prove current behaviour is intentional"). Both are real observations that identify no defect. | - -At least three of these require reasoning across multiple files or an untouched -downstream surface: the PR 335 and PR 350 propagation cases and the PR 674 -split-write case all do. +### Batch 2 — `s1-correctness-orchestrator`, 7 cases — **DELIVERED** + +Populated but not scored. Every case is minimized, every case names its source +disposition, and every case is adjudicated a second time by executable oracle. + +| case | class | source disposition | expected | +| ----------------------------------- | ------------------------------- | ------------------------------------------------------------------------------------------------------------- | -------- | +| `dependency-strictness-propagation` | multi-file / untouched consumer | atelier PR 335, comment 2867594627 — **accepted**, commit `5cb0333` | gating | +| `stale-claim-release-guard` | concurrency / data integrity | atelier PR 373, comment 2869270760 — **accepted**, with a regression test for the collect/apply window | gating | +| `optional-tool-probe` | validation gap | this repository's `f544aa0` — survived an aggregate `clean` verdict, **caught by CI** | gating | +| `session-continuation-summary` | clean control | atelier PR 486, comment 2881737041 — **declined on the merits**; flag kept, only operator wording changed | clean | +| `dependency-hint-parser-coverage` | clean control | atelier PR 279, comment 2862025661 — **declined on the merits** in reply 2862037362 | clean | +| `post-bootstrap-module-load` | negative control, polish-only | atelier PR 710, comment 2961766206 — **comment added, no behaviour changed** | clean | +| `process-isolation-assertion` | negative control, deferred | this repository's #50 — raised by a real review, **dispositioned `defer`**, item 4 of PR #61's preserved list | clean | + +Five carry multi-file diffs; three are only decidable by reading a consumer the +diff does not touch, which satisfies #58's multi-file minimum. + +Sanitization: all five atelier-sourced cases are `minimized_reproduction`, +rewritten from scratch against fictional subjects, retaining only the failure +shape. The two sourced from this repository are `repository_history`, also +rewritten against fictional subjects so neither can be mistaken for this suite's +own code. No source identifier, path, symbol, prose, or diff was copied into any +case. Retention authority is public in every case. + +#### The clean-control standard, and what it cost + +The owner settled the standard after batch 1: a clean control must be an +**adjudicated-rejected finding** — a case where a finding was actually raised +and dispositioned as not material. The recorded rejection is the evidence, and a +reviewer that re-raises it is charged a false alarm. "No review comments" is +explicitly not evidence of cleanliness, because absence of comment is ambiguous +between reviewed-and-clean and nobody-looked, and a control resting on it would +charge a false alarm against a reviewer that correctly found a real unnoticed +defect. + +Encoding: each clean case records its rejected concern as an accepted +non-finding carrying the formulations a reviewer would actually use. That +tolerates a non-gating mention while a gating one is still charged as a false +alarm at the verdict level. The rejected concern is deliberately **not** a root +cause — raising it is the error being measured, not the answer. + +**Four candidates were dropped for failing this standard.** Both of the +clean-control candidates batch 1 had identified, and two of the negative-control +candidates: + +| dropped candidate | why it fails the standard | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| atelier PR 335, the hardening hunk | Its cleanliness rested on the reviewer praising the hunk, not on any rejected finding, and it was cropped from a pull request that did carry a finding. Not an adjudicated rejection. | +| atelier PR 417, comment 2870710594 | An accepted **fix**, not a rejected finding. Acceptance says the concern was material, which is the opposite of what a clean control needs. | +| atelier PR 333, comment 2867594626 | Verified on re-check: the concern was **accepted** ("Included now"), with blocked epics added and coverage extended. Not immaterial. | +| atelier PR 356, comment 2868540467 | Verified on re-check: also **accepted** ("good catch"), with the recompute added and a regression test. Not immaterial. | + +One further candidate was assessed and dropped: atelier PR 160, comment +2849318292, where a one-liner suggestion was initially declined on readability +grounds. Following the thread, the reviewer pushed back and the underlying +preference was ultimately **implemented** via a shared helper. The disposition +is therefore acceptance in another form, not rejection, so it fails the standard +too. + +The correction is worth recording plainly: batch 1's adjudication plan named PR +333 and PR 356 as having *ambiguous* dispositions needing verification, and PR +335 as lacking a verified acceptance. Verification resolved all three — PR 333 +and PR 356 were accepted and are unusable as controls; PR 335 **was** accepted, +which makes it a valid escape rather than a valid control. ### Batch 3 — `s2-solution-simplicity-lens`, 4 cases @@ -154,7 +204,9 @@ split-write case all do. - Retention authority is recorded per case, and a case whose authority could not be established would be excluded before scoring rather than minimized into the corpus. -- Nothing here is adjudicated twice yet. Which of these candidates a second - adjudication is expected to *disagree* with, and which source dispositions are - too ambiguous to use without re-verification, is recorded in - [ADJUDICATION-PLAN.md](ADJUDICATION-PLAN.md) rather than smoothed over here. +- Every case in `s1-correctness-orchestrator` is adjudicated twice: the recorded + source disposition, and an executable oracle that runs the stated requirement. + No case in that stratum needs the owner. The simplicity strata will, because + their claims have no executable form. +- No case in any populated scored stratum has been run through a runtime. They + are unobserved, which is what keeps a later baseline result-blind. diff --git a/review-suite/evals/baseline/v1/frozen-configuration.json b/review-suite/evals/baseline/v1/frozen-configuration.json index d0b8275..3b83876 100644 --- a/review-suite/evals/baseline/v1/frozen-configuration.json +++ b/review-suite/evals/baseline/v1/frozen-configuration.json @@ -1,7 +1,7 @@ { "record_version": "1.0", - "status": "incomplete_pending_owner_preregistration", - "status_detail": "Every value an implementing run can fix is fixed below. Two values cannot come from an implementing context and are therefore null: the per-stratum cost ceiling, which must be preregistered by the repository owner before any scored output is examined because a scored run spends real money, and the independent adjudication of each private expectation, which one context cannot supply both sides of. No scored run may launch until both are present. Freezing everything else now is what makes the remaining inputs a decision rather than a negotiation.", + "status": "ceiling_preregistered_pending_calibration_decision", + "status_detail": "The per-stratum cost ceiling is preregistered by the owner at 9.00 / 3.00 / 3.00 USD, 15.00 USD total, as a hard ceiling: exceeding it stops further runs in that stratum and records incomplete baseline evidence, and repetitions are never reduced after outputs are visible. The clean-control standard is settled: a clean control is an adjudicated-rejected finding, and false-alarm rate under it is a lower bound on invention rather than a general rate. Independent adjudication is satisfied for every populated correctness case by executable oracle, and will need the owner for the simplicity strata. One blocker remains before any scored run: a scored stratum cannot be both calibrated and result-blind, and the owner must preregister which resolution applies. See LIMITATIONS.md items 13 to 15.", "v1_review_behaviour_commit": "16560d807c66076fcbf3f00d3a87f543c6ae2458", "v1_review_behaviour_commit_note": "The pre-v2 review-suite commit whose behaviour a scored baseline must evaluate. This candidate modifies no file under skills/ and no v1 contract, so the evaluated closure text is identical at every commit on this branch: all three closure digests recorded below and in every pilot report are unchanged from this commit. The digest, not the commit, is the load-bearing pin - a commit can move without the evaluated text changing, and the evaluated text cannot change without the digest moving.", "pilot_suite_commit": "2ae0d23c18f247f49d3cc5e76f26d1cf9610c83e", @@ -36,7 +36,7 @@ "strata": [ { "id": "s1-correctness-orchestrator", - "state": "declared_unpopulated", + "state": "populated_not_scored", "target_skill": "review-code-change", "target_skill_dependencies": [ "review-solution-simplicity", @@ -55,14 +55,28 @@ "two adjudicated rejected, deferred, speculative, or polish-only findings as negative controls" ], "envelope_source": "pilot-orchestrator", - "cost_ceiling_usd": null, + "cost_ceiling_usd": 9.0, "cost_ceiling_proposal_usd": 9.0, "scored_invocation": "just eval-review-suite 'python3 review-suite/scripts/evals/claude_executor.py' --corpus review-suite/evals/strata/s1-correctness-orchestrator --runs 5 --timeout 300 --artifact-dir review-suite/evals/artifacts/s1-correctness-orchestrator/- --attempts-out review-suite/evals/artifacts/s1-correctness-orchestrator/-.attempts.jsonl --report-out review-suite/evals/baseline/v1/s1-correctness-orchestrator.report.json", "scored_invocation_note": "Executable verbatim once the stratum directory exists and the owner has preregistered the ceiling. Substitute the stratum's own corpus_version in the artifact directory.", "expected_spend_usd": 3.33, "expected_spend_note": "Expected spend assumes the prompt cache behaves as measured: one cache-creation attempt per case, the rest cache reads. The proposed ceiling assumes every attempt pays cache creation, which is the worst case actually observed rather than a hypothetical one.", "worst_observed_cold_cost_per_attempt_usd": 0.2021, - "all_cold_worst_case_usd": 7.07 + "all_cold_worst_case_usd": 7.07, + "cases": 7, + "case_ids": [ + "dependency-hint-parser-coverage", + "dependency-strictness-propagation", + "optional-tool-probe", + "post-bootstrap-module-load", + "process-isolation-assertion", + "session-continuation-summary", + "stale-claim-release-guard" + ], + "corpus_version": "0.1-s1-populated", + "adjudication": "Every case adjudicated twice: the recorded source disposition, and an executable oracle that runs the stated requirement. No case needs the owner.", + "blocked_on": "Calibration. Every expectation is `calibrated: false` and no case has been run through any runtime, deliberately. See limitation 14: a scored stratum cannot be both calibrated and result-blind, and the owner must pick a resolution before `scored` flips to true.", + "cost_ceiling_status": "preregistered_by_owner" }, { "id": "s2-solution-simplicity-lens", @@ -78,14 +92,16 @@ "two requirement-justified near-miss controls" ], "envelope_source": "pilot-solution-simplicity", - "cost_ceiling_usd": null, + "cost_ceiling_usd": 3.0, "cost_ceiling_proposal_usd": 3.0, "scored_invocation": "just eval-review-suite 'python3 review-suite/scripts/evals/claude_executor.py' --corpus review-suite/evals/strata/s2-solution-simplicity-lens --runs 5 --timeout 300 --artifact-dir review-suite/evals/artifacts/s2-solution-simplicity-lens/- --attempts-out review-suite/evals/artifacts/s2-solution-simplicity-lens/-.attempts.jsonl --report-out review-suite/evals/baseline/v1/s2-solution-simplicity-lens.report.json", "scored_invocation_note": "Executable verbatim once the stratum directory exists and the owner has preregistered the ceiling. Substitute the stratum's own corpus_version in the artifact directory.", "expected_spend_usd": 0.82, "expected_spend_note": "Expected spend assumes the prompt cache behaves as measured: one cache-creation attempt per case, the rest cache reads. The proposed ceiling assumes every attempt pays cache creation, which is the worst case actually observed rather than a hypothetical one.", "worst_observed_cold_cost_per_attempt_usd": 0.1094, - "all_cold_worst_case_usd": 2.19 + "all_cold_worst_case_usd": 2.19, + "adjudication": "Oracle unavailable: an over-engineering or reuse claim has no executable form. Second adjudication is `owner_required` for every case in this stratum.", + "cost_ceiling_status": "preregistered_by_owner" }, { "id": "s3-code-simplicity-lens", @@ -101,14 +117,16 @@ "two behaviour-clarifying or non-material near-miss controls" ], "envelope_source": "pilot-code-simplicity", - "cost_ceiling_usd": null, + "cost_ceiling_usd": 3.0, "cost_ceiling_proposal_usd": 3.0, "scored_invocation": "just eval-review-suite 'python3 review-suite/scripts/evals/claude_executor.py' --corpus review-suite/evals/strata/s3-code-simplicity-lens --runs 5 --timeout 300 --artifact-dir review-suite/evals/artifacts/s3-code-simplicity-lens/- --attempts-out review-suite/evals/artifacts/s3-code-simplicity-lens/-.attempts.jsonl --report-out review-suite/evals/baseline/v1/s3-code-simplicity-lens.report.json", "scored_invocation_note": "Executable verbatim once the stratum directory exists and the owner has preregistered the ceiling. Substitute the stratum's own corpus_version in the artifact directory.", "expected_spend_usd": 0.93, "expected_spend_note": "Expected spend assumes the prompt cache behaves as measured: one cache-creation attempt per case, the rest cache reads. The proposed ceiling assumes every attempt pays cache creation, which is the worst case actually observed rather than a hypothetical one.", "worst_observed_cold_cost_per_attempt_usd": 0.1116, - "all_cold_worst_case_usd": 2.23 + "all_cold_worst_case_usd": 2.23, + "adjudication": "Oracle unavailable: an over-engineering or reuse claim has no executable form. Second adjudication is `owner_required` for every case in this stratum.", + "cost_ceiling_status": "preregistered_by_owner" }, { "id": "connector-escape", @@ -151,9 +169,8 @@ } ], "pending_owner_inputs": [ - "A per-stratum cost ceiling, preregistered before any scored output is examined. Proposals, and the pilot arithmetic behind them, are in COST-CEILING-PROPOSAL.md.", - "Two independent adjudications for each material root cause, accepted non-finding, severity, and allowed equivalent formulation, from genuinely separate parties, with disagreements resolved in a recorded note. ADJUDICATION-PLAN.md assesses what the source review threads can and cannot supply, where a blind agent context is and is not a legitimate second adjudicator, and every case where the two adjudications are expected to disagree.", - "A decision on what a clean correctness control must be. Neither identified candidate is an adjudicated clean review, and until the standard is settled the false-alarm rate has no honest denominator. See ADJUDICATION-PLAN.md section 4." + "A resolution to the calibration-versus-result-blindness conflict, preregistered with the rest of the configuration because it decides what recall means. Four options are set out in LIMITATIONS.md item 14.", + "Second adjudications for every case in the two simplicity strata, where no executable oracle is possible. See ADJUDICATION-PLAN.md." ], "pilot_reports": [ { @@ -191,5 +208,9 @@ "detail": "The committed compact reports carry every metric, denominator, and identity a consumer needs. The retained per-attempt records are what a per-attempt figure quoted from a record - a single attempt's cost, token count, or latency - is re-derived from, and they are referenced by the path identity above." }, "baseline_limitations_record": "LIMITATIONS.md", - "adjudication_plan": "ADJUDICATION-PLAN.md" + "adjudication_plan": "ADJUDICATION-PLAN.md", + "resolved_owner_inputs": [ + "Per-stratum cost ceiling: preregistered at 9.00 / 3.00 / 3.00 USD, 15.00 total.", + "Clean-control standard: an adjudicated-rejected finding, with false-alarm rate reported as a lower bound on invention." + ] } diff --git a/review-suite/evals/strata/README.md b/review-suite/evals/strata/README.md index 5123b25..5b9a0e2 100644 --- a/review-suite/evals/strata/README.md +++ b/review-suite/evals/strata/README.md @@ -110,6 +110,23 @@ a run would overwrite retained output. The exact per-stratum invocations, artifact paths included, are recorded in [`baseline/v1/frozen-configuration.json`](../baseline/v1/frozen-configuration.json). +## Scored strata + +`s1-correctness-orchestrator` is populated with seven cases covering #58's +correctness classes: three accepted material escapes, two clean controls, and +two negative controls. Every case is minimized from a real review disposition in +the public `shaug/atelier` or in this repository's own history, and every case +carries an executable oracle as its second independent adjudication. + +It declares `scored: false`, which is not a formality. Every expectation is +`calibrated: false` and no case has been run through any runtime, deliberately — +observing a scored case's prose in order to calibrate it would fit the grader to +the answer. Limitation 14 in +[the limitations record](../baseline/v1/LIMITATIONS.md) sets out the four +resolutions and the owner picks one before `scored` flips. + +`s2-solution-simplicity-lens` and `s3-code-simplicity-lens` remain unpopulated. + ## Adding a stratum 1. Create `review-suite/evals/strata//` with `corpus.json`, From fa772a7d770bd3d07f3fdd9bdc45a0c237b1d14e Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Mon, 27 Jul 2026 09:33:28 -0700 Subject: [PATCH 4/5] fix: stop a grader formulation being quotable from its own packet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes both blocking findings and the strong recommendation from the first review cycle. One of them was a real contamination defect in a shipped case. ## The defect `optional-tool-probe` is the stratum's only validation-gap escape. Its packet said ``` `subprocess.run` raises `FileNotFoundError`, a subclass of `OSError` ```, and its private expectation accepted `subprocess.run raises FileNotFoundError` as a matching formulation. A reviewer could therefore have earned full recall by quoting its own input, and the case would have measured whether a reviewer echoes the packet rather than whether it finds the escape. The contamination audit did not catch it, and that is the more important half. The audit's text search folds case and whitespace but keeps punctuation; the grader folds punctuation away. The formulation was invisible to one and a perfect match for the other. **Whatever the grader treats as the same text is what decides a score, so an audit using a stricter definition of "the same text" than the grader is not auditing the thing that matters.** The two now share one normalisation, imported rather than restated so they cannot drift apart again. Scoped to `material_root_causes`. An accepted non-finding's formulation often does legitimately restate reviewer-visible content, and matching one only makes the grader more tolerant of an observation already judged immaterial — it cannot manufacture a correct answer. Two regression tests pin both halves: one echoes a root-cause formulation into its own packet with punctuation between every word and requires the audit to reject it, the other echoes an accepted non-finding's formulation and requires the audit to allow it. The packet keeps both environment facts a reviewer needs — that the validating machine has the tool and that CI does not — and now points at the repository's existing optional-tool probe as the worked example, instead of naming the exception. The defect remains fully derivable; it is no longer quotable. ## Also fixed - **The stratum purpose contradicted the corpus.** It still read "none has a second independent adjudication", written before the oracles landed one commit later, while every provenance record carries `adjudication.second: "oracle"` and four other records say adjudication is satisfied. The runner copies that block verbatim into every report, so the false claim would have travelled. It now names calibration as the single remaining blocker. - **A non-finding cited a packet fact that was not there.** The tolerance for an unscoped artifact glob was justified by "the packet scopes the directory to the run under test", which the packet does not say. Reworded to justify the tolerance on its own terms rather than on an invented fact; the packet is unchanged. ## Not covered, and why No mechanical check can catch a packet that describes the defect in words the formulations do not use — the packet is supposed to describe the change. That stays a curation judgement, with the oracle as the backstop: a case whose stated root cause is not the real cause fails its oracle regardless of how its packet is worded. --- CHANGELOG.md | 2 + review-suite/evals/baseline/v1/LIMITATIONS.md | 33 +++++++++++++ .../s1-correctness-orchestrator/corpus.json | 2 +- .../process-isolation-assertion.json | 2 +- .../reviewer/optional-tool-probe/packet.json | 2 +- review-suite/scripts/evals/protocol.py | 44 +++++++++++++++++ .../scripts/tests/test_eval_corpus.py | 49 +++++++++++++++++++ 7 files changed, 131 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b15b9e2..9f0e5a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ summary: Chronological history of repository and skill changes. ## 2026-07-27 — Populated the correctness stratum, and recovered carved suffixes +- fix: stop a grader formulation being quotable from its own packet - docs: record the batch-2 delivery, the clean-control standard, and its limits + (`e83da75687f06ec9ff6a82df5ac4845c6e6fb23f`) - feat: adjudicate the correctness cases by executable oracle (`6dcfeabc7acd325d1dcaae4ed341fa780df94bc9`) - feat: populate the correctness stratum with seven adjudicated cases diff --git a/review-suite/evals/baseline/v1/LIMITATIONS.md b/review-suite/evals/baseline/v1/LIMITATIONS.md index 5f4e680..b172371 100644 --- a/review-suite/evals/baseline/v1/LIMITATIONS.md +++ b/review-suite/evals/baseline/v1/LIMITATIONS.md @@ -346,3 +346,36 @@ One case carries a further caveat. `process-isolation-assertion`'s first adjudication came from the review suite **under evaluation** — a `defer` verdict from the same contract being measured — which is weaker evidence than a human disposition and is recorded as such in its provenance. + +## 16. A grader formulation must not be quotable from its own packet + +A contamination class that the physical separation of reviewer and private +artifacts does not cover, found by review on this corpus and now gated. + +The audit's text search folds case and whitespace but keeps punctuation. The +grader folds punctuation away entirely. So a formulation reading +`subprocess.run raises FileNotFoundError` is invisible to the audit when its +packet says `` `subprocess.run` raises `FileNotFoundError` `` — and is a perfect +match once the grader normalises both. **A case shipped in exactly that state**: +the stratum's only validation-gap escape could have been answered at full recall +by quoting its own input, so it would have measured whether a reviewer echoes +the packet rather than whether it finds the escape. + +The general lesson is worth stating beyond this instance: *whatever the grader +treats as the same text is what decides a score, so an audit that uses a +stricter definition of "the same text" than the grader is not auditing the thing +that matters.* The two now share one normalisation, and a regression test echoes +a formulation into its own packet with punctuation between every word to prove +the check survives re-wording. + +Scoped to `material_root_causes`. An accepted non-finding's formulation often +does legitimately restate reviewer-visible content, and matching one only makes +the grader more tolerant of an observation already judged immaterial — it cannot +manufacture a correct answer. A second test pins that exemption so it is a +decision rather than an oversight. + +What this does **not** cover: a packet that describes the defect in words the +formulations do not use. No mechanical check can catch that, because the packet +is supposed to describe the change. It is a curation judgement, and the oracle +is the backstop — a case whose defect is spelled out in its own packet will +still fail its oracle if the stated root cause is not the real cause. diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json b/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json index 1e3b7ff..01409cb 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json @@ -13,7 +13,7 @@ "ground_truth": "human-review", "scored": false, "grading_is_signal": true, - "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Populated but NOT yet scored: every expectation is uncalibrated and none has a second independent adjudication, and a baseline captured on either would report a number about the corpus rather than about the reviewer. Flip `scored` to true only once both land." + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Populated and independently adjudicated: every case carries the recorded source disposition as its first adjudication and an executable oracle as its second. NOT yet scored, for one remaining reason: every expectation is uncalibrated, and no case has been run through any runtime, because observing a scored case's prose in order to calibrate it would fit the grader to the answer. Flip `scored` to true only once the owner preregisters how that conflict is resolved." }, "cases": [ "dependency-hint-parser-coverage", diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json index 8a2fe26..cff393c 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/process-isolation-assertion.json @@ -18,7 +18,7 @@ }, { "id": "anf.artifact-glob-not-scoped", - "description": "The artifact glob reads the whole directory rather than only this run's files. Worth noting; the packet scopes the directory to the run under test.", + "description": "The artifact glob reads a directory-wide pattern rather than only this run's files. A real observation about the test's precision, and worth saying. It is not material to either stated requirement, and the packet's contract does not claim per-run scoping, so it is tolerated rather than counted.", "equivalent_formulations": [ "the glob is not scoped to this run", "reads every file in the artifact directory" diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json index f632e0f..650f287 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json @@ -77,7 +77,7 @@ "data": [ "The developer machine this candidate was validated on has the task runner installed.", "`.github/workflows/ci.yml` installs the project's Python dependencies and does not install the task runner.", - "`subprocess.run` raises `FileNotFoundError`, a subclass of `OSError`, when the named executable does not exist; it does not return a non-zero status." + "The existing probe in `tests/test_formatter.py` is the repository's worked example of detecting an optional tool." ], "operational": [ "CI is the only environment where the optional runner is absent, and it is the gate that must stay green." diff --git a/review-suite/scripts/evals/protocol.py b/review-suite/scripts/evals/protocol.py index dcdd301..93135ec 100644 --- a/review-suite/scripts/evals/protocol.py +++ b/review-suite/scripts/evals/protocol.py @@ -23,6 +23,12 @@ from pathlib import Path from typing import Any +# Imported for its normalisation only. The contamination audit has to judge a +# leak by the same rule the grader uses to match one, so the two must share that +# rule rather than each keep its own. `grader` deliberately imports nothing from +# here, so this direction stays acyclic. +from . import grader + PROTOCOL_VERSION = "1.0" SCRIPTS_DIR = Path(__file__).resolve().parents[1] @@ -277,6 +283,44 @@ def audit_request( for blind in blind_strings(expectation, provenance): if _contains(haystack, blind): errors.append(f"payload contains private expectation text {blind!r}") + errors.extend(_matchable_formulation_errors(request, expectation)) + return errors + + +def _matchable_formulation_errors( + request: dict[str, Any], expectation: dict[str, Any] +) -> list[str]: + """Reject a root-cause formulation the grader could match from the payload. + + The check above folds case and whitespace but keeps punctuation, while the + grader folds punctuation away entirely. That gap is not academic: a + formulation reading ``subprocess.run raises FileNotFoundError`` is invisible + to a collapse-based search of a packet saying + ``\\`subprocess.run\\` raises \\`FileNotFoundError\\```, and matches perfectly + once the grader normalises both. A case shipped in exactly that state, so a + reviewer could have earned full recall by quoting its own input, and the + escape it was built to measure would have measured nothing. + + Whatever the grader treats as the same text is what decides a score, so the + audit has to use the grader's own definition rather than a stricter one. + + Scoped to `material_root_causes` deliberately. An accepted non-finding's + formulation frequently does restate reviewer-visible content, and an echo + there only makes the grader more tolerant of an observation already judged + immaterial - it cannot manufacture a correct answer, which is the failure + this guards against. + """ + payload = [grader.normalize(leaf) for leaf in _string_leaves(request)] + errors = [] + for root_cause in expectation.get("material_root_causes", []): + for formulation in root_cause.get("equivalent_formulations", []): + needle = grader.normalize(formulation) + if needle and any(needle in leaf for leaf in payload): + errors.append( + "payload contains text the grader would match as the accepted " + f"formulation {formulation!r} for {root_cause.get('id')!r}, so a " + "reviewer could earn recall by quoting the packet" + ) return errors diff --git a/review-suite/scripts/tests/test_eval_corpus.py b/review-suite/scripts/tests/test_eval_corpus.py index e8f5b87..7abe795 100644 --- a/review-suite/scripts/tests/test_eval_corpus.py +++ b/review-suite/scripts/tests/test_eval_corpus.py @@ -143,6 +143,55 @@ def assertAuditFails(self, fragment): f"no error mentioned {fragment!r}: {errors}", ) + def test_a_formulation_the_grader_could_match_from_the_packet_fails(self): + """Punctuation must not hide a leak the grader would still match. + + The plain text search folds case and whitespace but keeps punctuation, + while the grader folds punctuation away. A formulation echoed into its own + packet with backticks around it is therefore invisible to the first check + and a perfect match for the grader - which would let a reviewer earn full + recall by quoting its own input, and make the case measure nothing. A real + case shipped in that state before this check existed. + """ + case_id = self._index()["cases"][0] + expectation = self._load_expectation(case_id) + formulation = expectation["material_root_causes"][0]["equivalent_formulations"][ + 0 + ] + packet_path = self.root / "reviewer" / case_id / "packet.json" + packet = json.loads(packet_path.read_text()) + # Echo it the way real prose would: emphasised, not verbatim. + decorated = "`" + "` `".join(formulation.split()) + "`" + packet.setdefault("context", {}).setdefault("data", []).append(decorated) + packet_path.write_text(json.dumps(packet, indent=2)) + self.assertAuditFails("the grader would match") + + def test_an_accepted_non_finding_formulation_may_restate_the_packet(self): + """Scoped deliberately: an echo there cannot manufacture a right answer. + + An accepted non-finding often does restate reviewer-visible content, and + matching one only makes the grader tolerate an observation already judged + immaterial. Banning it would ban honest corpora for no gain. + """ + formulation = None + for candidate in self._index()["cases"]: + non_findings = ( + self._load_expectation(candidate).get("accepted_non_findings") or [] + ) + if non_findings: + case_id = candidate + formulation = non_findings[0]["equivalent_formulations"][0] + break + self.assertIsNotNone( + formulation, "no corpus case declares an accepted non-finding to probe" + ) + packet_path = self.root / "reviewer" / case_id / "packet.json" + packet = json.loads(packet_path.read_text()) + decorated = "`" + "` `".join(formulation.split()) + "`" + packet.setdefault("context", {}).setdefault("data", []).append(decorated) + packet_path.write_text(json.dumps(packet, indent=2)) + self.assertEqual([], audit_corpus.audit(self.root)) + def test_missing_expectation_fails_before_any_launch(self): case_id = self._index()["cases"][0] self._expectation_path(case_id).unlink() From 06a5679643a0a5bcb1944c8bff4bd4986f4f77e1 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Mon, 27 Jul 2026 09:57:12 -0700 Subject: [PATCH 5/5] fix: make every packet diff a valid patch, and gate the adjudication record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the blocking finding and both strong recommendations from the second review cycle, which was the first cycle in which `review-code-simplicity` ran at all. ## The blocking defect: a packet asserting a suite it could not have `dependency-strictness-propagation` guarded its corrective action on the wrong side of a negation. `reconcile_closed` called `reopen_if_unlanded` only when integration *was* proven, so for the abandoned-merge record the diff's own added test builds, the call never ran — the test asserted `record.reopened` and could not pass, while the packet recorded that suite as `9 passed` on a diff declared complete. That is worse than a typo. The contradiction is neither a root cause nor an accepted non-finding, so a reviewer that spotted it would be classified `unexpected` at gating severity and **charged a false positive for a curation defect**, while a reviewer that missed it could reach the expected verdict with zero recall. On the case carrying #58's multi-file minimum. The guard is now negated to match the stated goal, and the oracle was strengthened to exercise the corrective action rather than only the two decision predicates — which is precisely why it passed a diff it should have rejected. ## Eleven of seventeen packet diffs were not valid patches `test_every_case_diff_is_a_parseable_patch` only ever covered the original protocol-proof corpus, so malformed hunk headers shipped in every stratum added since: eleven of seventeen packets failed `git apply --numstat`. All headers are recomputed from their own bodies, and the check now iterates `corpus_roots()`. One case needed more than a header. The propagation case carried the untouched sibling caller as a context-only file section, which `git apply` rejects outright — a diff cannot contain a file it does not change. That consumer now appears in `context.data` as the packet's statement of what the untouched module contains, which is both a valid patch and a truer rendering of the case: the whole point is that the sibling is *not* in the diff. This matters more for a scored stratum than it first appears. A packet whose diff is not a valid patch is inconsistent evidence, and a reviewer that refuses a merge verdict on it is behaving correctly under its own contract — which a scored run would record as a mismatch, charging the reviewer for a curation defect again. ## The adjudication gate was keyed on presence, not on scoring The existing check only fired when a case already declared an adjudication, so a later batch could ship scored cases with none and pass everything, while the adjudication plan claimed a test held them to it. A stratum may now not declare `scored` while any of its cases lacks a recorded second adjudication, and the plan's wording is corrected to describe what the tests actually enforce. ## Records Limitations 17 and 18 are added, both from what this cycle found: an oracle is a hand transcription of its packet with no mechanical link to it — demonstrated by the blocking defect surviving one — and diff validity is now gated where it previously was not. A mechanical packet-to-oracle link is named as a candidate v2 mechanism for #59 rather than invented here. Four deferred findings are preserved and reported rather than applied: the formulation audit walks the payload a second time, the oracle expectation lookup takes the first corpus carrying a case, two oracle `corrected` legs restate their `candidate`, and the stratum's single `ground_truth` label covers two `repository_history` cases whose provenance records the difference. --- CHANGELOG.md | 2 + .../evals/baseline/v1/ADJUDICATION-PLAN.md | 5 ++- review-suite/evals/baseline/v1/LIMITATIONS.md | 43 +++++++++++++++++++ .../rollback-guidance-render/packet.json | 2 +- .../rollback-guidance-render/packet.json | 2 +- .../status-label-normalization/packet.json | 2 +- .../rollback-guidance-render/packet.json | 2 +- .../packet.json | 2 +- .../packet.json | 4 +- .../reviewer/optional-tool-probe/packet.json | 2 +- .../post-bootstrap-module-load/packet.json | 2 +- .../process-isolation-assertion/packet.json | 2 +- .../session-continuation-summary/packet.json | 2 +- .../stale-claim-release-guard/packet.json | 2 +- .../scripts/evals/oracles/__init__.py | 7 +-- .../dependency_strictness_propagation.py | 17 ++++++++ .../scripts/tests/test_eval_corpus.py | 28 +++++++----- .../scripts/tests/test_eval_oracles.py | 22 ++++++++++ 18 files changed, 121 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0e5a5..b969c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ summary: Chronological history of repository and skill changes. ## 2026-07-27 — Populated the correctness stratum, and recovered carved suffixes +- fix: make every packet diff a valid patch, and gate the adjudication record - fix: stop a grader formulation being quotable from its own packet + (`fa772a7d770bd3d07f3fdd9bdc45a0c237b1d14e`) - docs: record the batch-2 delivery, the clean-control standard, and its limits (`e83da75687f06ec9ff6a82df5ac4845c6e6fb23f`) - feat: adjudicate the correctness cases by executable oracle diff --git a/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md b/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md index c50ed78..ec6427c 100644 --- a/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md +++ b/review-suite/evals/baseline/v1/ADJUDICATION-PLAN.md @@ -157,8 +157,9 @@ clean candidate. **Owner still required for:** every case in `s2-solution-simplicity-lens` and `s3-code-simplicity-lens`. "This is over-engineered" and "this complexity is requirement-justified" have no executable form, so those cases will declare -`owner_required`, and a test holds them to that rather than letting them claim -two adjudications they do not have. +`owner_required`. Two tests hold them to it: a stratum may not declare `scored` +while any of its cases lacks a recorded second adjudication, and a case that +ships no oracle may not record anything other than `owner_required`. ### The three limits of what was settled diff --git a/review-suite/evals/baseline/v1/LIMITATIONS.md b/review-suite/evals/baseline/v1/LIMITATIONS.md index b172371..8928930 100644 --- a/review-suite/evals/baseline/v1/LIMITATIONS.md +++ b/review-suite/evals/baseline/v1/LIMITATIONS.md @@ -379,3 +379,46 @@ formulations do not use. No mechanical check can catch that, because the packet is supposed to describe the change. It is a curation judgement, and the oracle is the backstop — a case whose defect is spelled out in its own packet will still fail its oracle if the stated root cause is not the real cause. + +## 17. An oracle is a hand transcription of its packet, with no mechanical link + +Limitation 15 says an oracle adjudicates a requirement rather than a diff. There +is a second, sharper gap in the same mechanism, and this corpus demonstrated it. + +An oracle's `candidate()` is written by hand to mirror the packet's diff. +Nothing checks that it does. Review found a case where the diff's corrective +action was guarded on the wrong side of a negation — so the diff's own added +test could not have passed, while the packet recorded that suite as green — and +**the oracle passed anyway**, because it modelled only the two decision +predicates and never the action the test asserts. The packet was corrected, and +that oracle now exercises the corrective action too, but the general hole is +unchanged: an oracle can agree with a packet that says something different. + +So an oracle raises the floor without closing it. It proves a requirement is +violated and that correcting the stated cause fixes it; it does not prove the +reproduction it runs is the reproduction the reviewer will read. Two habits +follow, and neither is automatable today: + +- read a packet's own added tests as claims that must be *satisfiable* against + its own diff, since a packet asserting a green suite it cannot have is + inconsistent evidence a contract-faithful reviewer may refuse; and +- write the oracle from the packet's *acceptance criteria and its tests + together*, not from the criteria alone. + +A mechanical link — generating one from the other, or applying the diff and +running its tests — would close it, and is a candidate v2 mechanism for #59 +rather than something this ticket should invent. + +## 18. Diff validity is now gated, and was not before + +Every packet's diff must parse as a patch. That was asserted only for the +original protocol-proof corpus, so malformed hunk headers shipped in every +stratum added afterwards: eleven of the seventeen packets in this repository +failed `git apply --numstat` when first checked. + +It matters more for a scored stratum than it looks. A packet whose diff is not a +valid patch is internally inconsistent evidence, and a reviewer that refuses a +merge verdict on it is behaving correctly under its own contract — which a +scored run would then record as a verdict mismatch, charging the reviewer for a +curation defect. All seventeen now parse, and the check runs across every corpus +rather than one. diff --git a/review-suite/evals/strata/pilot-code-simplicity/reviewer/rollback-guidance-render/packet.json b/review-suite/evals/strata/pilot-code-simplicity/reviewer/rollback-guidance-render/packet.json index c7aff60..d42e268 100644 --- a/review-suite/evals/strata/pilot-code-simplicity/reviewer/rollback-guidance-render/packet.json +++ b/review-suite/evals/strata/pilot-code-simplicity/reviewer/rollback-guidance-render/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/storectl/guidance.py b/storectl/guidance.py\n--- a/storectl/guidance.py\n+++ b/storectl/guidance.py\n@@ -14,6 +14,17 @@ def render_apply_plan(plan):\n return \"\\n\".join(lines)\n+\n+\n+def render_rollback_guidance(store_root):\n+ \"\"\"Return the operator commands that undo an applied migration.\"\"\"\n+ return [\n+ f\"storectl export --all > {store_root}.backup.json\",\n+ f\"storectl import {store_root}.backup.json\",\n+ f\"rm -f {store_root}.backup.json\",\n+ ]\ndiff --git a/tests/test_guidance.py b/tests/test_guidance.py\n--- a/tests/test_guidance.py\n+++ b/tests/test_guidance.py\n@@ -31,3 +31,9 @@ def test_render_apply_plan_lists_every_step():\n assert len(render_apply_plan(plan)) > 0\n+\n+\n+def test_render_rollback_guidance_emits_three_steps():\n+ commands = render_rollback_guidance(\"/srv/store\")\n+ assert len(commands) == 3\n+ assert all(command.strip() for command in commands)\n" + "content": "diff --git a/storectl/guidance.py b/storectl/guidance.py\n--- a/storectl/guidance.py\n+++ b/storectl/guidance.py\n@@ -14,1 +14,10 @@ def render_apply_plan(plan):\n return \"\\n\".join(lines)\n+\n+\n+def render_rollback_guidance(store_root):\n+ \"\"\"Return the operator commands that undo an applied migration.\"\"\"\n+ return [\n+ f\"storectl export --all > {store_root}.backup.json\",\n+ f\"storectl import {store_root}.backup.json\",\n+ f\"rm -f {store_root}.backup.json\",\n+ ]\ndiff --git a/tests/test_guidance.py b/tests/test_guidance.py\n--- a/tests/test_guidance.py\n+++ b/tests/test_guidance.py\n@@ -31,1 +31,7 @@ def test_render_apply_plan_lists_every_step():\n assert len(render_apply_plan(plan)) > 0\n+\n+\n+def test_render_rollback_guidance_emits_three_steps():\n+ commands = render_rollback_guidance(\"/srv/store\")\n+ assert len(commands) == 3\n+ assert all(command.strip() for command in commands)\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/pilot-orchestrator/reviewer/rollback-guidance-render/packet.json b/review-suite/evals/strata/pilot-orchestrator/reviewer/rollback-guidance-render/packet.json index c7aff60..d42e268 100644 --- a/review-suite/evals/strata/pilot-orchestrator/reviewer/rollback-guidance-render/packet.json +++ b/review-suite/evals/strata/pilot-orchestrator/reviewer/rollback-guidance-render/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/storectl/guidance.py b/storectl/guidance.py\n--- a/storectl/guidance.py\n+++ b/storectl/guidance.py\n@@ -14,6 +14,17 @@ def render_apply_plan(plan):\n return \"\\n\".join(lines)\n+\n+\n+def render_rollback_guidance(store_root):\n+ \"\"\"Return the operator commands that undo an applied migration.\"\"\"\n+ return [\n+ f\"storectl export --all > {store_root}.backup.json\",\n+ f\"storectl import {store_root}.backup.json\",\n+ f\"rm -f {store_root}.backup.json\",\n+ ]\ndiff --git a/tests/test_guidance.py b/tests/test_guidance.py\n--- a/tests/test_guidance.py\n+++ b/tests/test_guidance.py\n@@ -31,3 +31,9 @@ def test_render_apply_plan_lists_every_step():\n assert len(render_apply_plan(plan)) > 0\n+\n+\n+def test_render_rollback_guidance_emits_three_steps():\n+ commands = render_rollback_guidance(\"/srv/store\")\n+ assert len(commands) == 3\n+ assert all(command.strip() for command in commands)\n" + "content": "diff --git a/storectl/guidance.py b/storectl/guidance.py\n--- a/storectl/guidance.py\n+++ b/storectl/guidance.py\n@@ -14,1 +14,10 @@ def render_apply_plan(plan):\n return \"\\n\".join(lines)\n+\n+\n+def render_rollback_guidance(store_root):\n+ \"\"\"Return the operator commands that undo an applied migration.\"\"\"\n+ return [\n+ f\"storectl export --all > {store_root}.backup.json\",\n+ f\"storectl import {store_root}.backup.json\",\n+ f\"rm -f {store_root}.backup.json\",\n+ ]\ndiff --git a/tests/test_guidance.py b/tests/test_guidance.py\n--- a/tests/test_guidance.py\n+++ b/tests/test_guidance.py\n@@ -31,1 +31,7 @@ def test_render_apply_plan_lists_every_step():\n assert len(render_apply_plan(plan)) > 0\n+\n+\n+def test_render_rollback_guidance_emits_three_steps():\n+ commands = render_rollback_guidance(\"/srv/store\")\n+ assert len(commands) == 3\n+ assert all(command.strip() for command in commands)\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/pilot-orchestrator/reviewer/status-label-normalization/packet.json b/review-suite/evals/strata/pilot-orchestrator/reviewer/status-label-normalization/packet.json index 9a06ced..b8ccde9 100644 --- a/review-suite/evals/strata/pilot-orchestrator/reviewer/status-label-normalization/packet.json +++ b/review-suite/evals/strata/pilot-orchestrator/reviewer/status-label-normalization/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/registry/normalize.py b/registry/normalize.py\n--- a/registry/normalize.py\n+++ b/registry/normalize.py\n@@ -1,10 +1,38 @@\n from registry.model import ACTIVE_STATUSES, Record\n \n+DRAFT_FLAG = \"flag:draft\"\n+READY_FLAG = \"flag:ready\"\n+\n \n def migrate(record: Record, target_status: str) -> Record:\n- \"\"\"Move a legacy record onto the canonical status field.\"\"\"\n- record.status = target_status\n- return record\n+ \"\"\"Move a legacy record onto the canonical status field.\n+\n+ Legacy records carried their runnability in flags. The canonical field is\n+ `status`; flags are retained only for display.\n+ \"\"\"\n+ record.status = target_status\n+ return _normalize_flags(record, target_status)\n+\n+\n+def _normalize_flags(record: Record, target_status: str) -> Record:\n+ flags = list(record.flags)\n+ if DRAFT_FLAG in flags:\n+ flags.remove(DRAFT_FLAG)\n+ active = target_status in ACTIVE_STATUSES\n+ if not active:\n+ record.flags = flags\n+ return record\n+ if READY_FLAG not in flags:\n+ flags.append(READY_FLAG)\n+ record.flags = flags\n+ return record\ndiff --git a/registry/scheduler.py b/registry/scheduler.py\n--- a/registry/scheduler.py\n+++ b/registry/scheduler.py\n@@ -12,7 +12,7 @@ from registry.normalize import READY_FLAG\n def runnable(record):\n \"\"\"Return whether the scheduler may pick this record up.\"\"\"\n- return READY_FLAG in record.flags\n+ return READY_FLAG in record.flags\ndiff --git a/registry/report.py b/registry/report.py\n--- a/registry/report.py\n+++ b/registry/report.py\n@@ -20,6 +20,9 @@ def summarize(records):\n rows = []\n for record in records:\n rows.append((record.identifier, record.status, sorted(record.flags)))\n+ # Display order is stable so a migration diff is readable.\n+ rows.sort()\n return rows\ndiff --git a/tests/test_normalize.py b/tests/test_normalize.py\n--- a/tests/test_normalize.py\n+++ b/tests/test_normalize.py\n@@ -8,3 +8,21 @@ def test_migrate_sets_the_canonical_status():\n assert migrate(record, \"running\").status == \"running\"\n+\n+\n+def test_migrate_drops_the_draft_flag():\n+ record = Record(identifier=\"r-1\", status=\"legacy\", flags=[\"flag:draft\"])\n+ assert \"flag:draft\" not in migrate(record, \"running\").flags\n+\n+\n+def test_migrate_adds_the_ready_flag_for_an_active_status():\n+ record = Record(identifier=\"r-2\", status=\"legacy\", flags=[])\n+ assert \"flag:ready\" in migrate(record, \"running\").flags\n+\n+\n+def test_migrate_leaves_an_inactive_record_without_the_ready_flag():\n+ record = Record(identifier=\"r-3\", status=\"legacy\", flags=[])\n+ assert \"flag:ready\" not in migrate(record, \"deferred\").flags\n" + "content": "diff --git a/registry/normalize.py b/registry/normalize.py\n--- a/registry/normalize.py\n+++ b/registry/normalize.py\n@@ -1,7 +1,28 @@\n from registry.model import ACTIVE_STATUSES, Record\n \n+DRAFT_FLAG = \"flag:draft\"\n+READY_FLAG = \"flag:ready\"\n+\n \n def migrate(record: Record, target_status: str) -> Record:\n- \"\"\"Move a legacy record onto the canonical status field.\"\"\"\n- record.status = target_status\n- return record\n+ \"\"\"Move a legacy record onto the canonical status field.\n+\n+ Legacy records carried their runnability in flags. The canonical field is\n+ `status`; flags are retained only for display.\n+ \"\"\"\n+ record.status = target_status\n+ return _normalize_flags(record, target_status)\n+\n+\n+def _normalize_flags(record: Record, target_status: str) -> Record:\n+ flags = list(record.flags)\n+ if DRAFT_FLAG in flags:\n+ flags.remove(DRAFT_FLAG)\n+ active = target_status in ACTIVE_STATUSES\n+ if not active:\n+ record.flags = flags\n+ return record\n+ if READY_FLAG not in flags:\n+ flags.append(READY_FLAG)\n+ record.flags = flags\n+ return record\ndiff --git a/registry/scheduler.py b/registry/scheduler.py\n--- a/registry/scheduler.py\n+++ b/registry/scheduler.py\n@@ -12,3 +12,3 @@ from registry.normalize import READY_FLAG\n def runnable(record):\n \"\"\"Return whether the scheduler may pick this record up.\"\"\"\n- return READY_FLAG in record.flags\n+ return READY_FLAG in record.flags\ndiff --git a/registry/report.py b/registry/report.py\n--- a/registry/report.py\n+++ b/registry/report.py\n@@ -20,4 +20,6 @@ def summarize(records):\n rows = []\n for record in records:\n rows.append((record.identifier, record.status, sorted(record.flags)))\n+ # Display order is stable so a migration diff is readable.\n+ rows.sort()\n return rows\ndiff --git a/tests/test_normalize.py b/tests/test_normalize.py\n--- a/tests/test_normalize.py\n+++ b/tests/test_normalize.py\n@@ -8,1 +8,16 @@ def test_migrate_sets_the_canonical_status():\n assert migrate(record, \"running\").status == \"running\"\n+\n+\n+def test_migrate_drops_the_draft_flag():\n+ record = Record(identifier=\"r-1\", status=\"legacy\", flags=[\"flag:draft\"])\n+ assert \"flag:draft\" not in migrate(record, \"running\").flags\n+\n+\n+def test_migrate_adds_the_ready_flag_for_an_active_status():\n+ record = Record(identifier=\"r-2\", status=\"legacy\", flags=[])\n+ assert \"flag:ready\" in migrate(record, \"running\").flags\n+\n+\n+def test_migrate_leaves_an_inactive_record_without_the_ready_flag():\n+ record = Record(identifier=\"r-3\", status=\"legacy\", flags=[])\n+ assert \"flag:ready\" not in migrate(record, \"deferred\").flags\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/pilot-solution-simplicity/reviewer/rollback-guidance-render/packet.json b/review-suite/evals/strata/pilot-solution-simplicity/reviewer/rollback-guidance-render/packet.json index c7aff60..d42e268 100644 --- a/review-suite/evals/strata/pilot-solution-simplicity/reviewer/rollback-guidance-render/packet.json +++ b/review-suite/evals/strata/pilot-solution-simplicity/reviewer/rollback-guidance-render/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/storectl/guidance.py b/storectl/guidance.py\n--- a/storectl/guidance.py\n+++ b/storectl/guidance.py\n@@ -14,6 +14,17 @@ def render_apply_plan(plan):\n return \"\\n\".join(lines)\n+\n+\n+def render_rollback_guidance(store_root):\n+ \"\"\"Return the operator commands that undo an applied migration.\"\"\"\n+ return [\n+ f\"storectl export --all > {store_root}.backup.json\",\n+ f\"storectl import {store_root}.backup.json\",\n+ f\"rm -f {store_root}.backup.json\",\n+ ]\ndiff --git a/tests/test_guidance.py b/tests/test_guidance.py\n--- a/tests/test_guidance.py\n+++ b/tests/test_guidance.py\n@@ -31,3 +31,9 @@ def test_render_apply_plan_lists_every_step():\n assert len(render_apply_plan(plan)) > 0\n+\n+\n+def test_render_rollback_guidance_emits_three_steps():\n+ commands = render_rollback_guidance(\"/srv/store\")\n+ assert len(commands) == 3\n+ assert all(command.strip() for command in commands)\n" + "content": "diff --git a/storectl/guidance.py b/storectl/guidance.py\n--- a/storectl/guidance.py\n+++ b/storectl/guidance.py\n@@ -14,1 +14,10 @@ def render_apply_plan(plan):\n return \"\\n\".join(lines)\n+\n+\n+def render_rollback_guidance(store_root):\n+ \"\"\"Return the operator commands that undo an applied migration.\"\"\"\n+ return [\n+ f\"storectl export --all > {store_root}.backup.json\",\n+ f\"storectl import {store_root}.backup.json\",\n+ f\"rm -f {store_root}.backup.json\",\n+ ]\ndiff --git a/tests/test_guidance.py b/tests/test_guidance.py\n--- a/tests/test_guidance.py\n+++ b/tests/test_guidance.py\n@@ -31,1 +31,7 @@ def test_render_apply_plan_lists_every_step():\n assert len(render_apply_plan(plan)) > 0\n+\n+\n+def test_render_rollback_guidance_emits_three_steps():\n+ commands = render_rollback_guidance(\"/srv/store\")\n+ assert len(commands) == 3\n+ assert all(command.strip() for command in commands)\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json index 2571dbf..a4312e1 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-hint-parser-coverage/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/lineage/parse.py b/lineage/parse.py\n--- a/lineage/parse.py\n+++ b/lineage/parse.py\n@@ -8,17 +8,14 @@ CANONICAL_LINK = \"link_type\"\n def parent_links(entry):\n \"\"\"Return the parent links declared by one dependency entry.\n \n- Historically the exporter emitted the link kind under several names.\n+ The exporter now emits exactly one canonical field, so only that field is\n+ read. The former aliases are gone from the export contract and reading them\n+ would keep accepting shapes the exporter can no longer produce.\n \"\"\"\n links = []\n for item in entry.get(\"dependencies\") or []:\n if isinstance(item, str):\n links.append(item)\n continue\n- kind = (\n- item.get(CANONICAL_LINK)\n- or item.get(\"relation\")\n- or item.get(\"linkType\")\n- or item.get(\"type\")\n- )\n+ kind = item.get(CANONICAL_LINK)\n if kind == \"parent_child\":\n links.append(item[\"issue\"][\"id\"])\n return links\ndiff --git a/tests/test_parse.py b/tests/test_parse.py\n--- a/tests/test_parse.py\n+++ b/tests/test_parse.py\n@@ -10,6 +10,18 @@ def test_parent_links_reads_a_string_entry():\n assert parent_links({\"dependencies\": [\"p-1\"]}) == [\"p-1\"]\n+\n+\n+def test_parent_links_reads_the_canonical_link_field():\n+ entry = {\"dependencies\": [{\"link_type\": \"parent_child\", \"issue\": {\"id\": \"p-2\"}}]}\n+ assert parent_links(entry) == [\"p-2\"]\n+\n+\n+def test_parent_links_ignores_a_retired_alias():\n+ entry = {\"dependencies\": [{\"type\": \"parent_child\", \"issue\": {\"id\": \"p-3\"}}]}\n+ assert parent_links(entry) == []\n" + "content": "diff --git a/lineage/parse.py b/lineage/parse.py\n--- a/lineage/parse.py\n+++ b/lineage/parse.py\n@@ -8,19 +8,16 @@ CANONICAL_LINK = \"link_type\"\n def parent_links(entry):\n \"\"\"Return the parent links declared by one dependency entry.\n \n- Historically the exporter emitted the link kind under several names.\n+ The exporter now emits exactly one canonical field, so only that field is\n+ read. The former aliases are gone from the export contract and reading them\n+ would keep accepting shapes the exporter can no longer produce.\n \"\"\"\n links = []\n for item in entry.get(\"dependencies\") or []:\n if isinstance(item, str):\n links.append(item)\n continue\n- kind = (\n- item.get(CANONICAL_LINK)\n- or item.get(\"relation\")\n- or item.get(\"linkType\")\n- or item.get(\"type\")\n- )\n+ kind = item.get(CANONICAL_LINK)\n if kind == \"parent_child\":\n links.append(item[\"issue\"][\"id\"])\n return links\ndiff --git a/tests/test_parse.py b/tests/test_parse.py\n--- a/tests/test_parse.py\n+++ b/tests/test_parse.py\n@@ -10,1 +10,11 @@ def test_parent_links_reads_a_string_entry():\n assert parent_links({\"dependencies\": [\"p-1\"]}) == [\"p-1\"]\n+\n+\n+def test_parent_links_reads_the_canonical_link_field():\n+ entry = {\"dependencies\": [{\"link_type\": \"parent_child\", \"issue\": {\"id\": \"p-2\"}}]}\n+ assert parent_links(entry) == [\"p-2\"]\n+\n+\n+def test_parent_links_ignores_a_retired_alias():\n+ entry = {\"dependencies\": [{\"type\": \"parent_child\", \"issue\": {\"id\": \"p-3\"}}]}\n+ assert parent_links(entry) == []\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json index 07169b1..4dcb224 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/dependency-strictness-propagation/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/pipeline/integration.py b/pipeline/integration.py\n--- a/pipeline/integration.py\n+++ b/pipeline/integration.py\n@@ -18,10 +18,22 @@ from pipeline.model import Record\n-def integration_proven(record):\n- \"\"\"Return whether this record's work reached the target branch.\"\"\"\n- return bool(record.merge_marker or record.target_ref)\n+def integration_proven(record, *, require_target_proof=False):\n+ \"\"\"Return whether this record's work reached the target branch.\n+\n+ With `require_target_proof`, only a target-branch ancestry proof counts. The\n+ merge marker alone is a hint written by the tooling and can be present on a\n+ record whose work never landed.\n+ \"\"\"\n+ if require_target_proof:\n+ return bool(record.target_ref)\n+ return bool(record.merge_marker or record.target_ref)\n@@ -44,7 +56,8 @@ def reconcile_closed(records):\n for record in records:\n- if integration_proven(record):\n+ status = record.status\n+ if integration_proven(record, require_target_proof=status == \"closed\"):\n reopen_if_unlanded(record)\ndiff --git a/pipeline/dependencies.py b/pipeline/dependencies.py\n--- a/pipeline/dependencies.py\n+++ b/pipeline/dependencies.py\n@@ -9,7 +9,7 @@ from pipeline.integration import integration_proven\n def dependency_finalized(record):\n \"\"\"Return whether a dependency may unblock the records that follow it.\"\"\"\n return integration_proven(record)\ndiff --git a/tests/test_integration.py b/tests/test_integration.py\n--- a/tests/test_integration.py\n+++ b/tests/test_integration.py\n@@ -20,3 +20,16 @@ def test_integration_proven_accepts_a_target_ref():\n assert integration_proven(Record(target_ref=\"refs/heads/main\"))\n+\n+\n+def test_integration_proven_rejects_a_marker_only_record_under_strict_proof():\n+ record = Record(merge_marker=True, target_ref=None)\n+ assert not integration_proven(record, require_target_proof=True)\n+\n+\n+def test_reconcile_closed_uses_strict_proof_for_a_closed_record():\n+ record = Record(status=\"closed\", merge_marker=True, target_ref=None)\n+ reconcile_closed([record])\n+ assert record.reopened\n" + "content": "diff --git a/pipeline/integration.py b/pipeline/integration.py\n--- a/pipeline/integration.py\n+++ b/pipeline/integration.py\n@@ -18,3 +18,10 @@ from pipeline.model import Record\n-def integration_proven(record):\n- \"\"\"Return whether this record's work reached the target branch.\"\"\"\n- return bool(record.merge_marker or record.target_ref)\n+def integration_proven(record, *, require_target_proof=False):\n+ \"\"\"Return whether this record's work reached the target branch.\n+\n+ With `require_target_proof`, only a target-branch ancestry proof counts. The\n+ merge marker alone is a hint written by the tooling and can be present on a\n+ record whose work never landed.\n+ \"\"\"\n+ if require_target_proof:\n+ return bool(record.target_ref)\n+ return bool(record.merge_marker or record.target_ref)\n@@ -44,3 +56,4 @@ def reconcile_closed(records):\n for record in records:\n- if not integration_proven(record):\n+ status = record.status\n+ if not integration_proven(record, require_target_proof=status == \"closed\"):\n reopen_if_unlanded(record)\ndiff --git a/tests/test_integration.py b/tests/test_integration.py\n--- a/tests/test_integration.py\n+++ b/tests/test_integration.py\n@@ -20,1 +20,12 @@ def test_integration_proven_accepts_a_target_ref():\n assert integration_proven(Record(target_ref=\"refs/heads/main\"))\n+\n+\n+def test_integration_proven_rejects_a_marker_only_record_under_strict_proof():\n+ record = Record(merge_marker=True, target_ref=None)\n+ assert not integration_proven(record, require_target_proof=True)\n+\n+\n+def test_reconcile_closed_uses_strict_proof_for_a_closed_record():\n+ record = Record(status=\"closed\", merge_marker=True, target_ref=None)\n+ reconcile_closed([record])\n+ assert record.reopened\n" } }, "change_contract": { @@ -80,7 +80,7 @@ ], "context": { "data": [ - "`pipeline/dependencies.py` and `pipeline/integration.py` are the only two modules that call `integration_proven`.", + "`pipeline/dependencies.py` and `pipeline/integration.py` are the only two modules that call `integration_proven`. This diff does not touch `pipeline/dependencies.py`, whose whole body is:\n from pipeline.integration import integration_proven\n\n def dependency_finalized(record):\n \"\"\"Return whether a dependency may unblock what follows it.\"\"\"\n return integration_proven(record)", "`dependency_finalized` is called by `pipeline/scheduler.py` to decide whether the records that follow a dependency may start.", "A closed record carrying a merge marker but no target ref occurs whenever a merge is abandoned after the marker is written." ], diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json index 650f287..7359f5b 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/optional-tool-probe/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/tests/test_recipes.py b/tests/test_recipes.py\n--- a/tests/test_recipes.py\n+++ b/tests/test_recipes.py\n@@ -1,8 +1,26 @@\n import subprocess\n import unittest\n \n \n class RecipeTests(unittest.TestCase):\n- \"\"\"Exercise the recipes directly.\"\"\"\n+ \"\"\"Run the recipes through the task runner when it is installed.\n+\n+ The runner is optional, so this class skips cleanly when it is absent.\n+ \"\"\"\n+\n+ @classmethod\n+ def setUpClass(cls):\n+ if subprocess.run(\n+ [\"taskr\", \"--version\"], capture_output=True, check=False\n+ ).returncode != 0:\n+ raise unittest.SkipTest(\"taskr is not installed\")\n+\n+ def taskr(self, *args):\n+ return subprocess.run([\"taskr\", *args], capture_output=True, text=True)\n+\n+ def test_the_check_recipe_runs(self):\n+ self.assertEqual(0, self.taskr(\"check\").returncode)\n+\n+ def test_the_audit_recipe_runs(self):\n+ self.assertEqual(0, self.taskr(\"audit\").returncode)\n" + "content": "diff --git a/tests/test_recipes.py b/tests/test_recipes.py\n--- a/tests/test_recipes.py\n+++ b/tests/test_recipes.py\n@@ -1,6 +1,25 @@\n import subprocess\n import unittest\n \n \n class RecipeTests(unittest.TestCase):\n- \"\"\"Exercise the recipes directly.\"\"\"\n+ \"\"\"Run the recipes through the task runner when it is installed.\n+\n+ The runner is optional, so this class skips cleanly when it is absent.\n+ \"\"\"\n+\n+ @classmethod\n+ def setUpClass(cls):\n+ if subprocess.run(\n+ [\"taskr\", \"--version\"], capture_output=True, check=False\n+ ).returncode != 0:\n+ raise unittest.SkipTest(\"taskr is not installed\")\n+\n+ def taskr(self, *args):\n+ return subprocess.run([\"taskr\", *args], capture_output=True, text=True)\n+\n+ def test_the_check_recipe_runs(self):\n+ self.assertEqual(0, self.taskr(\"check\").returncode)\n+\n+ def test_the_audit_recipe_runs(self):\n+ self.assertEqual(0, self.taskr(\"audit\").returncode)\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json index 362877d..df9392b 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/post-bootstrap-module-load/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/scripts/finish_task.py b/scripts/finish_task.py\n--- a/scripts/finish_task.py\n+++ b/scripts/finish_task.py\n@@ -1,10 +1,18 @@\n+import importlib\n import sys\n \n from bootstrap import select_runtime\n \n select_runtime(require_health=__name__ == \"__main__\")\n \n-from toolkit import store # noqa: E402\n+# Loaded through importlib, after `select_runtime` has reordered `sys.path`\n+# toward the selected runtime. A module-level `import` here binds whichever\n+# copy of `toolkit` was importable before the reorder, which on a machine with\n+# an older installed copy is the wrong one - and `--help` would then exercise a\n+# runtime the real run never uses.\n+_STORE = importlib.import_module(\"toolkit.store\")\n \n \n def main(argv):\n- return store.finish(argv[1])\n+ return _STORE.finish(argv[1])\ndiff --git a/tests/test_finish_task.py b/tests/test_finish_task.py\n--- a/tests/test_finish_task.py\n+++ b/tests/test_finish_task.py\n@@ -8,3 +8,13 @@ def test_main_finishes_the_named_task():\n assert main([\"finish_task\", \"t-1\"]) == 0\n+\n+\n+def test_the_store_is_loaded_after_the_runtime_is_selected():\n+ order = import_order_probe()\n+ assert order.index(\"select_runtime\") < order.index(\"toolkit.store\")\n+\n+\n+def test_help_uses_the_selected_runtime(tmp_path):\n+ older = install_older_toolkit(tmp_path)\n+ assert run_help_returns_runtime(older) == \"selected\"\n" + "content": "diff --git a/scripts/finish_task.py b/scripts/finish_task.py\n--- a/scripts/finish_task.py\n+++ b/scripts/finish_task.py\n@@ -1,11 +1,17 @@\n+import importlib\n import sys\n \n from bootstrap import select_runtime\n \n select_runtime(require_health=__name__ == \"__main__\")\n \n-from toolkit import store # noqa: E402\n+# Loaded through importlib, after `select_runtime` has reordered `sys.path`\n+# toward the selected runtime. A module-level `import` here binds whichever\n+# copy of `toolkit` was importable before the reorder, which on a machine with\n+# an older installed copy is the wrong one - and `--help` would then exercise a\n+# runtime the real run never uses.\n+_STORE = importlib.import_module(\"toolkit.store\")\n \n \n def main(argv):\n- return store.finish(argv[1])\n+ return _STORE.finish(argv[1])\ndiff --git a/tests/test_finish_task.py b/tests/test_finish_task.py\n--- a/tests/test_finish_task.py\n+++ b/tests/test_finish_task.py\n@@ -8,1 +8,11 @@ def test_main_finishes_the_named_task():\n assert main([\"finish_task\", \"t-1\"]) == 0\n+\n+\n+def test_the_store_is_loaded_after_the_runtime_is_selected():\n+ order = import_order_probe()\n+ assert order.index(\"select_runtime\") < order.index(\"toolkit.store\")\n+\n+\n+def test_help_uses_the_selected_runtime(tmp_path):\n+ older = install_older_toolkit(tmp_path)\n+ assert run_help_returns_runtime(older) == \"selected\"\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json index b5a745d..fff1c36 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/process-isolation-assertion/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/harness/tests/test_attempts.py b/harness/tests/test_attempts.py\n--- a/harness/tests/test_attempts.py\n+++ b/harness/tests/test_attempts.py\n@@ -30,3 +30,14 @@ def test_each_attempt_is_recorded():\n assert len(records) == 2\n+\n+\n+def test_every_attempt_runs_in_its_own_process():\n+ records = run_attempts(runs=2, artifact_dir=ARTIFACTS)\n+ names = set()\n+ for path in ARTIFACTS.glob(\"*.stdout.json\"):\n+ names.add(json.loads(path.read_text())[\"executor\"][\"name\"])\n+ # The bundled executor reports a stable name; process freshness is proven by\n+ # the driver spawning one subprocess per attempt.\n+ assert names == {\"bundled-executor\"}\n+ assert len(records) == len(list(ARTIFACTS.iterdir()))\n" + "content": "diff --git a/harness/tests/test_attempts.py b/harness/tests/test_attempts.py\n--- a/harness/tests/test_attempts.py\n+++ b/harness/tests/test_attempts.py\n@@ -30,1 +30,12 @@ def test_each_attempt_is_recorded():\n assert len(records) == 2\n+\n+\n+def test_every_attempt_runs_in_its_own_process():\n+ records = run_attempts(runs=2, artifact_dir=ARTIFACTS)\n+ names = set()\n+ for path in ARTIFACTS.glob(\"*.stdout.json\"):\n+ names.add(json.loads(path.read_text())[\"executor\"][\"name\"])\n+ # The bundled executor reports a stable name; process freshness is proven by\n+ # the driver spawning one subprocess per attempt.\n+ assert names == {\"bundled-executor\"}\n+ assert len(records) == len(list(ARTIFACTS.iterdir()))\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json index 02b5904..7c7d847 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/session-continuation-summary/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/worker/session.py b/worker/session.py\n--- a/worker/session.py\n+++ b/worker/session.py\n@@ -61,6 +61,20 @@ def run_once(job, control):\n+ preflight = finalize_preflight(job)\n+ if preflight.finalize_only:\n+ control.say(\n+ \"Skipping agent startup; finalizing this job from the preflight \"\n+ f\"result ({preflight.reason}).\"\n+ )\n+ finalize(job, control)\n+ # `started` drives loop continuation, not \"an agent ran\". The loop must\n+ # keep going here, so this stays True; the reporter below is what tells\n+ # an operator no agent session was launched.\n+ return RunSummary(started=True, reason=\"finalize_only\")\n+\n agent = start_agent(job)\n return RunSummary(started=True, reason=\"agent_session\")\ndiff --git a/worker/report.py b/worker/report.py\n--- a/worker/report.py\n+++ b/worker/report.py\n@@ -12,6 +12,10 @@ def report_summary(summary, control):\n+ if summary.reason == \"finalize_only\":\n+ control.say(\"continued without an agent session (finalize-only preflight)\")\n+ return\n if summary.started:\n control.say(\"started an agent session\")\ndiff --git a/tests/test_report.py b/tests/test_report.py\n--- a/tests/test_report.py\n+++ b/tests/test_report.py\n@@ -14,3 +14,16 @@ def test_report_summary_reports_a_started_session():\n assert \"started an agent session\" in control.lines\n+\n+\n+def test_report_summary_does_not_claim_a_session_for_finalize_only():\n+ control = FakeControl()\n+ report_summary(RunSummary(started=True, reason=\"finalize_only\"), control)\n+ assert \"started an agent session\" not in control.lines\n+ assert \"continued without an agent session\" in control.lines[0]\n+\n+\n+def test_run_once_continues_the_loop_for_finalize_only():\n+ summary = run_once(finalize_only_job(), FakeControl())\n+ assert summary.started is True\n+ assert summary.reason == \"finalize_only\"\n" + "content": "diff --git a/worker/session.py b/worker/session.py\n--- a/worker/session.py\n+++ b/worker/session.py\n@@ -61,2 +61,14 @@ def run_once(job, control):\n+ preflight = finalize_preflight(job)\n+ if preflight.finalize_only:\n+ control.say(\n+ \"Skipping agent startup; finalizing this job from the preflight \"\n+ f\"result ({preflight.reason}).\"\n+ )\n+ finalize(job, control)\n+ # `started` drives loop continuation, not \"an agent ran\". The loop must\n+ # keep going here, so this stays True; the reporter below is what tells\n+ # an operator no agent session was launched.\n+ return RunSummary(started=True, reason=\"finalize_only\")\n+\n agent = start_agent(job)\n return RunSummary(started=True, reason=\"agent_session\")\ndiff --git a/worker/report.py b/worker/report.py\n--- a/worker/report.py\n+++ b/worker/report.py\n@@ -12,2 +12,5 @@ def report_summary(summary, control):\n+ if summary.reason == \"finalize_only\":\n+ control.say(\"continued without an agent session (finalize-only preflight)\")\n+ return\n if summary.started:\n control.say(\"started an agent session\")\ndiff --git a/tests/test_report.py b/tests/test_report.py\n--- a/tests/test_report.py\n+++ b/tests/test_report.py\n@@ -14,1 +14,14 @@ def test_report_summary_reports_a_started_session():\n assert \"started an agent session\" in control.lines\n+\n+\n+def test_report_summary_does_not_claim_a_session_for_finalize_only():\n+ control = FakeControl()\n+ report_summary(RunSummary(started=True, reason=\"finalize_only\"), control)\n+ assert \"started an agent session\" not in control.lines\n+ assert \"continued without an agent session\" in control.lines[0]\n+\n+\n+def test_run_once_continues_the_loop_for_finalize_only():\n+ summary = run_once(finalize_only_job(), FakeControl())\n+ assert summary.started is True\n+ assert summary.reason == \"finalize_only\"\n" } }, "change_contract": { diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json index 799ed39..49d8fb3 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/stale-claim-release-guard/packet.json @@ -10,7 +10,7 @@ "diff": { "format": "unified_diff", "complete": true, - "content": "diff --git a/queue/reaper.py b/queue/reaper.py\n--- a/queue/reaper.py\n+++ b/queue/reaper.py\n@@ -14,18 +14,36 @@ from queue.store import load_claim, release_claim\n def collect_expired(now, store):\n \"\"\"Return one release action per claim that looks expired.\"\"\"\n actions = []\n for entry in store.scan():\n if entry.claimed_at is None or now - entry.claimed_at <= TTL:\n continue\n+ snapshot_owner = entry.owner\n \n- def _release(entry_id=entry.entry_id):\n- release_claim(store, entry_id)\n+ def _release(entry_id=entry.entry_id, snapshot_owner=snapshot_owner):\n+ # Re-read at apply time: the snapshot may be minutes old.\n+ current = load_claim(store, entry_id)\n+ if current is None:\n+ return\n+ if snapshot_owner is not None and current.owner != snapshot_owner:\n+ return\n+ release_claim(store, entry_id)\n \n actions.append(_release)\n return actions\ndiff --git a/tests/test_reaper.py b/tests/test_reaper.py\n--- a/tests/test_reaper.py\n+++ b/tests/test_reaper.py\n@@ -18,3 +18,18 @@ def test_collect_expired_ignores_a_fresh_claim():\n assert collect_expired(NOW, store) == []\n+\n+\n+def test_collect_expired_skips_release_when_the_owner_changed():\n+ store = FakeStore([Entry(entry_id=\"e-1\", owner=\"worker-a\", claimed_at=OLD)])\n+ actions = collect_expired(NOW, store)\n+ store.set_owner(\"e-1\", \"worker-b\")\n+ for action in actions:\n+ action()\n+ assert store.claim(\"e-1\").owner == \"worker-b\"\n" + "content": "diff --git a/queue/reaper.py b/queue/reaper.py\n--- a/queue/reaper.py\n+++ b/queue/reaper.py\n@@ -14,12 +14,19 @@ from queue.store import load_claim, release_claim\n def collect_expired(now, store):\n \"\"\"Return one release action per claim that looks expired.\"\"\"\n actions = []\n for entry in store.scan():\n if entry.claimed_at is None or now - entry.claimed_at <= TTL:\n continue\n+ snapshot_owner = entry.owner\n \n- def _release(entry_id=entry.entry_id):\n- release_claim(store, entry_id)\n+ def _release(entry_id=entry.entry_id, snapshot_owner=snapshot_owner):\n+ # Re-read at apply time: the snapshot may be minutes old.\n+ current = load_claim(store, entry_id)\n+ if current is None:\n+ return\n+ if snapshot_owner is not None and current.owner != snapshot_owner:\n+ return\n+ release_claim(store, entry_id)\n \n actions.append(_release)\n return actions\ndiff --git a/tests/test_reaper.py b/tests/test_reaper.py\n--- a/tests/test_reaper.py\n+++ b/tests/test_reaper.py\n@@ -18,1 +18,10 @@ def test_collect_expired_ignores_a_fresh_claim():\n assert collect_expired(NOW, store) == []\n+\n+\n+def test_collect_expired_skips_release_when_the_owner_changed():\n+ store = FakeStore([Entry(entry_id=\"e-1\", owner=\"worker-a\", claimed_at=OLD)])\n+ actions = collect_expired(NOW, store)\n+ store.set_owner(\"e-1\", \"worker-b\")\n+ for action in actions:\n+ action()\n+ assert store.claim(\"e-1\").owner == \"worker-b\"\n" } }, "change_contract": { diff --git a/review-suite/scripts/evals/oracles/__init__.py b/review-suite/scripts/evals/oracles/__init__.py index 5d2153c..42273ab 100644 --- a/review-suite/scripts/evals/oracles/__init__.py +++ b/review-suite/scripts/evals/oracles/__init__.py @@ -1,8 +1,9 @@ """Executable oracles: a second, machine adjudication of a case's materiality. -Loading is explicit rather than implicit. `oracle_module` resolves a case -identifier to its module so a missing oracle is a clear absence rather than an -import error, because most cases legitimately have none. +Loading is explicit rather than implicit. `load` resolves a case identifier to +its module, and `case_ids` reports only the oracles that actually ship, so a case +with no oracle is a clear absence rather than an import error. Most cases in the +simplicity strata will legitimately have none. """ from __future__ import annotations diff --git a/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py b/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py index 4779e0b..f92aedf 100644 --- a/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py +++ b/review-suite/scripts/evals/oracles/dependency_strictness_propagation.py @@ -19,6 +19,7 @@ class Record: status: str = "open" merge_marker: bool = False target_ref: str | None = None + reopened: bool = False def _integration_proven(record: Record, *, require_target_proof: bool = False) -> bool: @@ -57,10 +58,23 @@ def dependency_decides_integrated(record: Record) -> bool: ) +def _reconcile_closed(subject, records): + """The candidate's loop, transcribed: reopen what is not proven integrated.""" + for record in records: + if not subject.reconcile_decides_integrated(record): + record.reopened = True + return records + + def _check(subject) -> bool: abandoned = Record(status="closed", merge_marker=True, target_ref=None) landed = Record(status="closed", merge_marker=True, target_ref="refs/heads/main") open_record = Record(status="open", merge_marker=True, target_ref=None) + # Exercise the corrective action the packet's own added test asserts, not + # only the predicates. Modelling the predicates alone let a diff whose + # action guard was inverted - so that its stated test could never pass - + # satisfy this oracle. + _reconcile_closed(subject, [abandoned, landed, open_record]) return ( # The two decision paths must agree on the closed record. subject.reconcile_decides_integrated(abandoned) @@ -71,6 +85,9 @@ def _check(subject) -> bool: and subject.dependency_decides_integrated(landed) is True # An open record keeps the permissive behaviour the contract preserves. and subject.dependency_decides_integrated(open_record) is True + # The abandoned record is the one the reconcile pass must reopen. + and abandoned.reopened is True + and landed.reopened is False ) diff --git a/review-suite/scripts/tests/test_eval_corpus.py b/review-suite/scripts/tests/test_eval_corpus.py index 7abe795..5cb181b 100644 --- a/review-suite/scripts/tests/test_eval_corpus.py +++ b/review-suite/scripts/tests/test_eval_corpus.py @@ -64,16 +64,24 @@ def test_corpus_declares_the_shipped_grader_and_protocol_versions(self): self.assertEqual(protocol.PROTOCOL_VERSION, index["protocol_version"]) def test_every_case_diff_is_a_parseable_patch(self): - for case in self.corpus.cases: - with self.subTest(case=case.case_id): - completed = subprocess.run( - ["git", "apply", "--numstat"], - input=case.packet["candidate"]["diff"]["content"], - capture_output=True, - check=False, - text=True, - ) - self.assertEqual(0, completed.returncode, completed.stderr) + """Across every shipped corpus, not just the default one. + + A packet whose diff is not a valid patch is internally inconsistent + evidence, and a contract-faithful reviewer may refuse a merge verdict on + it - which a scored stratum would then grade as a mismatch. Covering only + the default corpus let malformed hunk headers ship in every stratum. + """ + for root in corpus.corpus_roots(): + for case in corpus.load_corpus(root).cases: + with self.subTest(stratum=root.name, case=case.case_id): + completed = subprocess.run( + ["git", "apply", "--numstat"], + input=case.packet["candidate"]["diff"]["content"], + capture_output=True, + check=False, + text=True, + ) + self.assertEqual(0, completed.returncode, completed.stderr) def test_the_corpus_covers_every_grading_situation_it_must_prove(self): verdicts = {case.expectation["expected_verdict"] for case in self.corpus.cases} diff --git a/review-suite/scripts/tests/test_eval_oracles.py b/review-suite/scripts/tests/test_eval_oracles.py index 8f3f9e3..bd3dfef 100644 --- a/review-suite/scripts/tests/test_eval_oracles.py +++ b/review-suite/scripts/tests/test_eval_oracles.py @@ -143,6 +143,28 @@ def test_an_oracle_settled_case_records_that_in_its_provenance(self): self.assertEqual("oracle", adjudication["second"]) self.assertTrue(adjudication["first"].strip()) + def test_a_scored_stratum_records_a_second_adjudication_for_every_case(self): + """Fail closed on `scored`, not on whether the record happens to exist. + + The guard below only fires when a case already declares an adjudication, + so a later batch could ship scored cases with none at all and pass + everything. This is the check that makes the adjudication plan's promise + true: a stratum may not be scored while any of its cases lacks a recorded + second adjudication. + """ + for root in corpus.corpus_roots(): + loaded = corpus.load_corpus(root) + if not loaded.scored: + continue + for case in loaded.cases: + with self.subTest(stratum=root.name, case_id=case.case_id): + adjudication = case.provenance.get("adjudication") + self.assertIsNotNone( + adjudication, + "a scored case must record how it was adjudicated", + ) + self.assertIn(adjudication["second"], {"oracle", "owner_required"}) + def test_a_case_without_an_oracle_routes_to_the_owner(self): """`owner_required` is the only honest alternative to an oracle.