diff --git a/.gitignore b/.gitignore index fc793c6..9fe6e28 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ __pycache__/ .DS_Store .venv/ .tools/agentskills/ +review-suite/evals/artifacts/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba1175..a899a8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,31 @@ summary: Chronological history of repository and skill changes. # Changelog +## 2026-07-26 — Added the result-blind review replay evaluator + +- fix: skip the recipe-execution tests when `just` is absent +- feat: record the evaluated skill closure with every run + (`b605051a7385dd310b0eff9dbf14c10dda87c633`) +- docs: record the measured smoke evaluation and its variance + (`87ec303d949301c908c3a29cb220bed22d44c775`) +- fix: evaluate the target skill's whole declared closure + (`62a9ed8fab166c7d380724e426449f0585714b07`) +- docs: pin the recorded smoke evaluation to its run + (`f00ce2db80ed3a7bed6afb4962cf0bb5a68390fe`) +- fix: complete the evaluated skill text and the audit ordering + (`67efd94339034674de6ca250f2b03e4a0213fc8b`) +- fix: close the replay evaluator's review-gate gaps + (`6ef8e25ce2e0183ef270111549660461493da5f4`) +- fix: stop misattributing review failures in the replay evaluator + (`e46184d6e856199fe0792d43e7f6e0c5a86e131f`) +- feat: add the result-blind review replay evaluator + (`8f0e9d646ec4e959d7adc7448f5fc7a82f4334d8`) + ## 2026-07-25 — Added coordinator-neutral delegated ticket execution - fix: pin CI to the established Ruff rule set so dependency drift cannot redefine the repository-wide lint gate + (`901dc3596207a88b6c8edcf548b5be3151ca7ab2`) - feat: add a versioned delegated-execution contract for `implement-ticket` (`b53efa674e929c181bdaac63ff0306cb756386db`) diff --git a/README.md b/README.md index a09349f..e009a63 100644 --- a/README.md +++ b/README.md @@ -54,8 +54,9 @@ causes the workflow to fail closed. ## Repository Layout - `skills/` — skill folders, each containing a `SKILL.md` and bundled resources -- `review-suite/` — canonical code-review contracts, validators, and raw - evaluation fixtures shared by repository-owned review skills +- `review-suite/` — canonical code-review contracts, validators, raw evaluation + fixtures, and the result-blind replay evaluator shared by repository-owned + review skills - `justfile` — common tasks for testing, validation, and formatting Current reusable agent skills: @@ -175,6 +176,53 @@ observations with `--output-dir`. A Claude Code headless adapter is bundled: just eval-implement-ticket-claude ``` +### Result-blind review replay evaluation + +`review-suite/evals/` is the canonical evaluator that measures what the review +skills actually do when a real agent runtime executes them repeatedly, rather +than asserting quality through expected JSON. It changes no review behaviour. +See [its README](review-suite/evals/README.md) for the protocol, the corpus +contract, and the grading interface. + +Three commands cover it: + +```bash +just test-review-suite # deterministic tests, no runtime +just audit-review-corpus # corpus integrity, no runtime +just eval-review-suite '' # the only one that may cost money +``` + +`just test` includes the deterministic evaluator tests and never launches a paid +runtime. `just eval-review-suite` is deliberately absent from `test`, `lint`, +and `check`. The bundled real-runtime adapter needs the `claude` CLI on `PATH`: + +```bash +just eval-review-suite "python3 review-suite/scripts/evals/claude_executor.py" +``` + +Each attempt starts a fresh process and receives one result-blind JSON request: +the target skill text, the raw review packet, the review contracts, and public +run identity. Expected findings, private grader labels, provenance, and even the +case name stay out of the payload, and `just audit-review-corpus` proves it by +inspecting the complete payload every case would produce. Spawn, timeout, +runtime, oversized-output, malformed, and protocol failures are reported +separately from a valid review and are never scored as clean. + +The bundled `fixture_executor.py` is a deterministic simulation of a compliant +reviewer, not a model. Its runs are marked `simulation`, so no baseline report +can be produced from them. Run the runner directly for repeated attempts, +per-attempt records, and the aggregate report: + +```bash +python3 review-suite/scripts/evals/runner.py \ + --executor "python3 review-suite/scripts/evals/claude_executor.py" \ + --runs 5 --report-out out/report.json +``` + +Corpus curation, grader calibration, and the frozen v1 baseline are separate +follow-up work; this evaluator supplies only the synthetic cases needed to prove +the protocol. + ## Prerequisites - Python 3.11+ diff --git a/justfile b/justfile index adf3ce2..2cef535 100644 --- a/justfile +++ b/justfile @@ -41,6 +41,18 @@ test: test-plugins test-review-suite: python3 -m unittest discover -s review-suite/scripts/tests -p 'test_*.py' +# Validate the replay corpus: schemas, cross-field expectation semantics, +# reviewer/private separation, provenance shape, outcome-revealing names, and +# the complete executor payload. Never launches a model. +audit-review-corpus: + python3 review-suite/scripts/evals/audit_corpus.py + +# Result-blind replay evaluation through an explicit real-runtime executor. +# Deliberately excluded from `test`, `lint`, and `check`: this is the only +# review-suite command that may spend money. +eval-review-suite executor: + python3 review-suite/scripts/evals/runner.py --executor "{{executor}}" + test-plugins: python3 -m unittest discover -s scripts/tests -p 'test_*.py' diff --git a/review-suite/evals/README.md b/review-suite/evals/README.md new file mode 100644 index 0000000..844bfa8 --- /dev/null +++ b/review-suite/evals/README.md @@ -0,0 +1,314 @@ +# Result-blind review replay evaluation + +This directory is the canonical evaluator for the repository-owned review suite. +It measures what the review skills actually do when a real agent runtime +executes them repeatedly, instead of asserting quality through expected JSON +that the same change also authored. + +Review skills may reference this evaluator. They must not fork it. + +The evaluator changes no review behaviour. It does not read, modify, or extend +`../CONTRACT.md`, the packet and result schemas, the lens prompts, or the +existing `../fixtures/` corpus. + +## Layout + +```text +review-suite/evals/ +├── contracts/ versioned evaluator schemas +│ ├── executor-request.schema.json the complete result-blind payload +│ ├── executor-response.schema.json the single reply an executor returns +│ ├── corpus.schema.json versions, target closure, case list +│ ├── expectation.schema.json private material root causes +│ └── provenance.schema.json origin and retention authority +├── corpus/ +│ ├── corpus.json versions, target closure, case ids +│ ├── reviewer/PROMPT.md shared reviewer instructions +│ ├── reviewer//packet.json reviewer-visible artifacts +│ ├── private/expectations/.json +│ └── private/provenance/.json +└── artifacts/ opt-in captured output, not in git + +review-suite/scripts/evals/ +├── protocol.py versioned request/response contract, failure taxonomy +├── corpus.py corpus loading, separation, and naming rules +├── grader.py root-cause grading interface and reference grader +├── report.py per-attempt records and the aggregate report +├── runner.py fresh-process replay driver +├── audit_corpus.py `just audit-review-corpus` +├── fixture_executor.py deterministic simulation, never a baseline +└── claude_executor.py documented real-runtime adapter +``` + +## Commands + +```bash +just test-review-suite # deterministic tests, no runtime +just audit-review-corpus # corpus integrity, no runtime +just eval-review-suite '' # the only command that may cost money +``` + +`just test` includes the deterministic evaluator tests and never launches a paid +runtime. `just eval-review-suite` is deliberately absent from `test`, `lint`, +and `check`. + +The bundled real-runtime adapter needs the `claude` CLI on `PATH`: + +```bash +just eval-review-suite "python3 review-suite/scripts/evals/claude_executor.py" +``` + +The runner accepts `--runs N` for repeated attempts, `--timeout`, +`--max-output-bytes`, `--artifact-dir`, `--attempts-out`, `--report-out`, and +`--baseline-report`. Run it directly when you need those options: + +```bash +python3 review-suite/scripts/evals/runner.py \ + --executor "python3 review-suite/scripts/evals/claude_executor.py" \ + --runs 5 --report-out out/report.json +``` + +## Executor protocol + +One attempt is one fresh process. The runner writes a single JSON request to +stdin and reads a single JSON response from stdout. `protocol_version` is `1.0`. + +The request carries the target skill name, a digest pinning the exact skill +text, the reviewer-visible instructions, the raw review packet, the bundled +review contracts, and public run metadata: an opaque case reference, the run +number, the suite commit, the corpus version, the exact candidate and +comparison-base identity, and a start timestamp. + +### The evaluated skill is a closure, not a file + +`corpus.json` declares `target_skill` plus `target_skill_dependencies`, and the +payload ships that whole closure: each skill's `SKILL.md` and every Markdown +file under its `references/`, excluding the bundled `review-suite/` mirror that +`contract_documents` already supplies canonically. Every section is labelled, +and the target's own `SKILL.md` is labelled `## Target skill:` so a reviewer can +tell which skill it is being asked to execute. + +This is load-bearing rather than tidy. `review-code-change` instructs its +reviewer to verify that `review-solution-simplicity`, `review-correctness`, and +`review-code-simplicity` are available and readable, and to return an aggregate +`blocked` result naming any that are missing. An executor is told to reason only +from what it is given. Ship the target alone and a fully compliant reviewer must +refuse every case, so recall and false-clean rates move the *wrong* way as the +reviewer becomes *more* compliant — the measurement inverts. Earlier revisions +had exactly that defect. Supplying the closure removes the inversion; it does +not by itself make any single case answer correctly, and the recorded runs below +show that case still varying between runs. + +`target_skill_digest` hashes the rendered closure, so it changes when any part +of any included skill changes. Loading fails closed when a declared skill is +absent, duplicated, or names the target itself. + +Which target a *scored* corpus should measure, which strata it contains, and the +cost envelope that follows from the closure's size are corpus-composition +decisions and are deliberately not settled here. + +The request never carries expected findings, private labels, prior conclusions, +suspected issues, implementation transcripts, or the case name. + +The response reports `outcome` as `review_result`, `blocked`, or +`runtime_failure`, plus `simulation`, executor identity, and optional usage. The +runner classifies each attempt into exactly one status: + +| status | meaning | +| ------------------- | ------------------------------------------------- | +| `spawn_failure` | the executor command could not start | +| `timeout` | the executor outlived `--timeout` | +| `runtime_failure` | non-zero exit, or a self-reported runtime failure | +| `output_too_large` | stdout exceeded `--max-output-bytes` | +| `malformed_output` | unparseable, schema-invalid, or wrongly bound | +| `protocol_mismatch` | the reply declared another protocol version | +| `blocked` | a valid review that refuses a merge verdict | +| `review_result` | a valid, candidate-bound review | + +The first six are evaluation failures. They are reported separately, are never +graded, and can never appear as a clean review. A malformed review result is an +evaluation failure, not a missed finding. + +Classification judges the reply, never the packet. `corpus.py` has already +established each packet's validity before anything launches, so a packet defect +is a deliberate property of the case rather than news about the executor, and it +is never charged to the executor. That matters for a `packet_valid: false` case: +a reviewer that wrongly issues a merge verdict on incomplete evidence is +classified `review_result` and graded as the wrong answer it is, instead of +disappearing into `malformed_output`. + +Only `review_result` attempts are graded, because a `blocked` review declines to +give a merge verdict and so has nothing to score against. A `blocked` attempt is +still a valid review and still counts toward stability. The runner's exit status +reports evaluation integrity, never review quality: `0` when every attempt +produced a valid outcome, `1` when any attempt failed the protocol, `2` when the +configuration or corpus was rejected before any launch. + +## Corpus and expectation contract + +Three separately validated locations keep the reviewer away from the answer: + +- `reviewer/` holds only the shared prompt and each case's `packet.json`; +- `private/expectations/` holds the material root causes and accepted + non-findings; and +- `private/provenance/` holds origin, retention authority, and sanitization. + +A case identifier and every reviewer-visible filename must describe the subject +matter, never the outcome. `corpus.py` rejects names containing verdict, +severity, failure-class, or disposition words. The payload additionally carries +only an opaque `c-` case reference, so even a badly named case cannot leak +through a request. + +`just audit-review-corpus` builds the complete request each case would produce +and rejects it both structurally, by permitting an exact key set, and textually, +by searching for private expectation and provenance text. Three fields are +deliberately exempt from the text search because banning them would ban the +packet or the contracts themselves: a root cause's `requirement` normally +restates a reviewer-visible acceptance criterion, its `surface` is a +reviewer-visible code location, and `expected_verdict`, `severity`, and `origin` +are closed public vocabularies that the bundled contracts spell out in full. +Those three stay private structurally instead, because the expectation file is +never part of a request. + +Loading fails closed. A missing expectation, a missing provenance record, a +schema violation, an orphaned file, a packet whose validity disagrees with its +expectation, or a grader-version mismatch is an error before any process starts. + +## Grading interface + +A reviewer is graded on material root causes, not prose. Each root cause states +its requirement, triggering condition, affected surface, material consequence, +severity, and the formulations a competent reviewer may use. An observed finding +matches when it points at the affected surface *and* describes the root cause in +an accepted way. One signal alone is a partial match, and a finding that fully +matches two root causes is ambiguous; both are referred for adjudication rather +than silently scored either way. `accepted_non_findings` describe observations +that are tolerated without counting as false positives. + +The reference grader is protocol proof. It is not calibrated, and its output is +not a v1 score. + +## Simulation and baselines + +`fixture_executor.py` hand-codes the review a compliant reviewer would return +for each synthetic packet, so the protocol, failure taxonomy, grading interface, +and reporting are deterministically testable for free. It is a simulation, not a +model evaluation, and it cannot detect a model misreading the review contract. + +Two independent controls keep a simulation out of any baseline: every fixture +response sets `"simulation": true`, and the runner forces the flag whenever the +bundled fixture executor is the command, regardless of what the reply claims. An +aggregate report whose `simulation` is true reports `baseline_eligible: false`, +and `--baseline-report` refuses to write a file for it. + +## Reporting + +`--attempts-out` writes one JSON record per attempt. `--report-out` writes the +aggregate report, which represents material-finding recall, false-clean rate, +false-positive rate, false-alarm rate, unique finding contribution, verdict and +finding stability, every failure-status rate, latency, and whatever usage and +cost the executor reported. Every rate and every stability figure is published +with its denominator so a small run count is not mistaken for a precise +capability measurement. + +Stability spans every attempt that produced a valid review, `blocked` ones +included, and is computed per case before being averaged. Pooling verdicts +across unlike cases would report disagreement between cases as instability +within one, and dropping `blocked` attempts would report a reviewer that refuses +a verdict on one run and issues one on the next as perfectly stable. + +The report's `configuration` block states what was run: executor command, target +skill, its declared dependencies, the exact closure document list, the closure +digest, suite commit, corpus and grader versions, run count, and the timeout and +output limits. Each attempt repeats the target, its dependencies, and the +digest, so a per-attempt record is self-describing without its report. + +The report encodes no success threshold and returns no pass/fail judgement. + +`--artifact-dir` retains raw executor output. It is opt-in, and +`review-suite/evals/artifacts/` is excluded from git. + +### Usage accounting + +An adapter reports `input_tokens`, `output_tokens`, and `cost_usd`. Two details +of the Claude adapter generalize to any cached runtime and matter to anyone +freezing a cost envelope: + +- Input tokens total every input-side field the runtime reports, including cache + creation and cache read. Under prompt caching the uncached `input_tokens` + residue is tiny — a measured 16,456-token prompt reported `input_tokens: 2` — + so counting only that field understates real input by orders of magnitude. +- Model identity is required, not optional. Headless output carries no top-level + `model` string; it reports `modelUsage` as a mapping keyed by model id. The + adapter resolves the model from that mapping, falls back to `--model`, and + returns `runtime_failure` rather than recording an attempt that cannot name + the model that answered, because a stratum without a model is not comparable. + +## Scope and known limitations + +This directory proves the protocol, the grading interface, the contamination +controls, and the failure taxonomy. It deliberately does not curate a +representative scored corpus, calibrate the grader, or capture a v1 baseline. + +### Protocol smoke evaluation + +> **This run proves the protocol end to end and nothing else.** It is not +> evidence of reviewer capability, review quality, or cost, and must not be +> cited as any of those here or downstream. Its numbers are here to show that +> the fields exist, are populated, and are internally consistent — that an +> attempt is classified, graded, and recorded with identity, usage, and latency. +> One run per case cannot measure a stochastic reviewer, and no cost envelope +> follows from it. + +One run per case through the bundled Claude adapter, recorded at suite commit +`62a9ed8fab166c7d380724e426449f0585714b07`, target `review-code-change` with +declared closure `review-solution-simplicity`, `review-correctness`, +`review-code-simplicity` and digest `9b2805f14cdd6158`, model +`claude-opus-4-6[1m]`, corpus version `0.1-protocol-proof`, grader version +`1.0`: + +- 6 attempts, 6 valid protocol outcomes, 0 evaluation failures, all 6 graded; +- 5 of 6 verdicts matched. `catalog-import-feed` returned `changes_required` + where `blocked` was expected — a merge verdict on a packet whose required + full-scope validation evidence is absent; +- 8 findings reported. 7 were referred for adjudication because none matched a + shipped formulation, and 1 was recorded as an unexpected gating finding, + giving `false_positive_rate` 0.1667 over 6; +- `material_finding_recall` 0.0 over the 4 attempts with expected root causes; + `false_clean_rate` 0.0 over 4; +- usage and latency were populated rather than dropped: 191,422 input tokens, + 8,051 output tokens, 1.03 USD reported, 33.0 s mean and 59.9 s maximum + latency, and a model identity on every attempt. + +The verdicts above are demonstrably not repeatable. The immediately preceding +run of this same configuration returned `blocked` on `catalog-import-feed` — the +correct answer — while this one returned a merge verdict. That case is the one +testing refusal on incomplete evidence, which is the behaviour the surrounding +work most needs to measure, and it flipped between two consecutive runs. Every +stability figure here rests on a denominator of 1. Use `--runs N`, and read the +result as a measurement only once the denominator supports it. + +### Limitations for whoever curates the scored corpus + +- Surface matching is file-level, because a private root cause names a function + while a finding names a line. A finding in the right file that the grader does + not recognize is therefore reported as a partial match needing adjudication, + not as a false positive. Calibration must decide how those are scored. +- The shipped formulations were written before any real run and were not tuned + afterwards, which is why recall is 0.0 above. That is the conservative + behaviour this interface is meant to have, and it means grader calibration is + required before any recall number means anything. +- Completeness of the evaluated skill closure is load-bearing, not incidental. + Earlier revisions omitted first the orchestration protocol and then the three + lens skills that `review-code-change` requires; each omission changed observed + behaviour on the incomplete-evidence case. Any change to what a payload + carries is a change to what is being measured, and starts a new stratum. Every + run records its closure's membership and digest so a stratum can state what it + evaluated. +- The choice of scored target, the strata to compare, and the cost envelope to + preregister all follow from that closure and are deliberately left open. Size + the envelope from a preregistered run of the chosen closure; do not + extrapolate it from the protocol smoke run above. +- `expectation.schema.json` requires a `severity` on every root cause that no + metric currently consumes. Either score severity agreement or drop the + requirement; do not assume it is being measured. diff --git a/review-suite/evals/contracts/corpus.schema.json b/review-suite/evals/contracts/corpus.schema.json new file mode 100644 index 0000000..9211400 --- /dev/null +++ b/review-suite/evals/contracts/corpus.schema.json @@ -0,0 +1,32 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/shaug/agent-scripts/review-suite/evals/contracts/corpus.schema.json", + "title": "Review replay corpus index", + "description": "Version metadata and the case-identifier list. This index is deliberately outcome-free so it can sit beside the reviewer-visible artifacts.", + "type": "object", + "additionalProperties": false, + "required": [ + "corpus_version", + "protocol_version", + "grader_version", + "target_skill", + "target_skill_dependencies", + "cases" + ], + "properties": { + "corpus_version": {"type": "string", "minLength": 1}, + "protocol_version": {"const": "1.0"}, + "grader_version": {"type": "string", "minLength": 1}, + "target_skill": {"type": "string", "minLength": 1}, + "target_skill_dependencies": { + "description": "Sibling skills the target's own contract requires. Their text is shipped in every payload, because a target that must verify its dependencies cannot be evaluated without them. May be empty for a self-sufficient target.", + "type": "array", + "items": {"type": "string", "minLength": 1} + }, + "cases": { + "type": "array", + "minItems": 1, + "items": {"type": "string", "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$"} + } + } +} diff --git a/review-suite/evals/contracts/executor-request.schema.json b/review-suite/evals/contracts/executor-request.schema.json new file mode 100644 index 0000000..5373c0c --- /dev/null +++ b/review-suite/evals/contracts/executor-request.schema.json @@ -0,0 +1,58 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/shaug/agent-scripts/review-suite/evals/contracts/executor-request.schema.json", + "title": "Review replay executor request", + "description": "The complete result-blind payload written to one fresh executor process on stdin. Every property is reviewer-visible by contract; nothing here may reveal an expected outcome.", + "type": "object", + "additionalProperties": false, + "required": [ + "protocol_version", + "target_skill", + "target_skill_digest", + "skill_prompt", + "contract_documents", + "instructions", + "packet", + "run" + ], + "properties": { + "protocol_version": {"const": "1.0"}, + "target_skill": {"type": "string", "minLength": 1}, + "target_skill_digest": {"type": "string", "pattern": "^[0-9a-f]{16}$"}, + "skill_prompt": {"type": "string", "minLength": 1}, + "contract_documents": {"type": "object"}, + "instructions": {"type": "string", "minLength": 1}, + "packet": {"type": "object"}, + "run": { + "type": "object", + "additionalProperties": false, + "required": [ + "case_ref", + "run_number", + "suite_commit", + "corpus_version", + "candidate", + "started_at" + ], + "properties": { + "case_ref": {"type": "string", "pattern": "^c-[0-9a-f]{8}$"}, + "run_number": {}, + "suite_commit": {"type": "string", "minLength": 1}, + "corpus_version": {"type": "string", "minLength": 1}, + "started_at": {"type": "string", "minLength": 1}, + "candidate": { + "type": "object", + "additionalProperties": false, + "required": ["head_sha", "comparison_base_sha"], + "properties": { + "head_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"}, + "comparison_base_sha": { + "type": "string", + "pattern": "^[0-9a-f]{40}$" + } + } + } + } + } + } +} diff --git a/review-suite/evals/contracts/executor-response.schema.json b/review-suite/evals/contracts/executor-response.schema.json new file mode 100644 index 0000000..037440f --- /dev/null +++ b/review-suite/evals/contracts/executor-response.schema.json @@ -0,0 +1,41 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/shaug/agent-scripts/review-suite/evals/contracts/executor-response.schema.json", + "title": "Review replay executor response", + "description": "The single JSON object one executor process writes to stdout. `outcome` separates a valid review from a runtime failure so a failed run is never scored as a clean review.", + "type": "object", + "additionalProperties": false, + "required": ["protocol_version", "outcome", "simulation", "executor"], + "properties": { + "protocol_version": {"const": "1.0"}, + "outcome": {"enum": ["review_result", "blocked", "runtime_failure"]}, + "simulation": {"type": "boolean"}, + "executor": { + "type": "object", + "additionalProperties": false, + "required": ["name"], + "properties": { + "name": {"type": "string", "minLength": 1}, + "version": {"type": "string", "minLength": 1}, + "runtime": {"type": "string", "minLength": 1}, + "model": {"type": "string", "minLength": 1} + } + }, + "result": {"type": "object"}, + "failure": { + "type": "object", + "additionalProperties": false, + "required": ["reason"], + "properties": {"reason": {"type": "string", "minLength": 1}} + }, + "usage": { + "type": "object", + "additionalProperties": false, + "properties": { + "input_tokens": {}, + "output_tokens": {}, + "cost_usd": {} + } + } + } +} diff --git a/review-suite/evals/contracts/expectation.schema.json b/review-suite/evals/contracts/expectation.schema.json new file mode 100644 index 0000000..75875ee --- /dev/null +++ b/review-suite/evals/contracts/expectation.schema.json @@ -0,0 +1,68 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/shaug/agent-scripts/review-suite/evals/contracts/expectation.schema.json", + "title": "Private review replay expectation", + "description": "Private grading evidence for one case. This file never enters an executor request. `requirement` and `surface` may legitimately restate reviewer-visible packet content; every other field is blind and must be absent from the request payload.", + "type": "object", + "additionalProperties": false, + "required": [ + "expectation_version", + "case_id", + "packet_valid", + "expected_verdict", + "material_root_causes", + "accepted_non_findings" + ], + "properties": { + "expectation_version": {"const": "1.0"}, + "case_id": {"type": "string", "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$"}, + "packet_valid": {"type": "boolean"}, + "expected_verdict": {"enum": ["clean", "changes_required", "blocked"]}, + "material_root_causes": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": [ + "id", + "requirement", + "trigger", + "surface", + "consequence", + "severity", + "equivalent_formulations" + ], + "properties": { + "id": {"type": "string", "pattern": "^rc\\.[a-z0-9][a-z0-9-]*$"}, + "requirement": {"type": "string", "minLength": 1}, + "trigger": {"type": "string", "minLength": 1}, + "surface": {"type": "string", "minLength": 1}, + "consequence": {"type": "string", "minLength": 1}, + "severity": {"enum": ["blocking", "strong_recommendation"]}, + "equivalent_formulations": { + "type": "array", + "minItems": 1, + "items": {"type": "string", "minLength": 1} + } + } + } + }, + "accepted_non_findings": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["id", "description", "equivalent_formulations"], + "properties": { + "id": {"type": "string", "pattern": "^anf\\.[a-z0-9][a-z0-9-]*$"}, + "description": {"type": "string", "minLength": 1}, + "equivalent_formulations": { + "type": "array", + "minItems": 1, + "items": {"type": "string", "minLength": 1} + } + } + } + } + } +} diff --git a/review-suite/evals/contracts/provenance.schema.json b/review-suite/evals/contracts/provenance.schema.json new file mode 100644 index 0000000..f3de4a8 --- /dev/null +++ b/review-suite/evals/contracts/provenance.schema.json @@ -0,0 +1,32 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/shaug/agent-scripts/review-suite/evals/contracts/provenance.schema.json", + "title": "Review replay case provenance", + "description": "Where one case came from and under what authority it is retained. Provenance is private: it never enters an executor request.", + "type": "object", + "additionalProperties": false, + "required": [ + "provenance_version", + "case_id", + "origin", + "retention_authority", + "sanitization", + "recorded_at" + ], + "properties": { + "provenance_version": {"const": "1.0"}, + "case_id": {"type": "string", "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$"}, + "origin": { + "enum": [ + "synthetic", + "minimized_reproduction", + "repository_history", + "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} + } +} diff --git a/review-suite/evals/corpus/corpus.json b/review-suite/evals/corpus/corpus.json new file mode 100644 index 0000000..3f19ae7 --- /dev/null +++ b/review-suite/evals/corpus/corpus.json @@ -0,0 +1,19 @@ +{ + "corpus_version": "0.1-protocol-proof", + "protocol_version": "1.0", + "grader_version": "1.0", + "target_skill": "review-code-change", + "target_skill_dependencies": [ + "review-solution-simplicity", + "review-correctness", + "review-code-simplicity" + ], + "cases": [ + "ledger-charge-limit", + "session-token-refresh", + "invoice-export-batch", + "webhook-retry-queue", + "pricing-tier-lookup", + "catalog-import-feed" + ] +} diff --git a/review-suite/evals/corpus/private/expectations/catalog-import-feed.json b/review-suite/evals/corpus/private/expectations/catalog-import-feed.json new file mode 100644 index 0000000..307d2c5 --- /dev/null +++ b/review-suite/evals/corpus/private/expectations/catalog-import-feed.json @@ -0,0 +1,8 @@ +{ + "expectation_version": "1.0", + "case_id": "catalog-import-feed", + "packet_valid": false, + "expected_verdict": "blocked", + "material_root_causes": [], + "accepted_non_findings": [] +} diff --git a/review-suite/evals/corpus/private/expectations/invoice-export-batch.json b/review-suite/evals/corpus/private/expectations/invoice-export-batch.json new file mode 100644 index 0000000..47ded04 --- /dev/null +++ b/review-suite/evals/corpus/private/expectations/invoice-export-batch.json @@ -0,0 +1,17 @@ +{ + "expectation_version": "1.0", + "case_id": "invoice-export-batch", + "packet_valid": true, + "expected_verdict": "clean", + "material_root_causes": [], + "accepted_non_findings": [ + { + "id": "anf.invoice-helper-naming", + "description": "Observing that the pre-existing helper name is terse is a real but non-material remark, not a defect this candidate introduced.", + "equivalent_formulations": [ + "the batch helper name predates this change", + "terse pre-existing helper name" + ] + } + ] +} diff --git a/review-suite/evals/corpus/private/expectations/ledger-charge-limit.json b/review-suite/evals/corpus/private/expectations/ledger-charge-limit.json new file mode 100644 index 0000000..4e33a6e --- /dev/null +++ b/review-suite/evals/corpus/private/expectations/ledger-charge-limit.json @@ -0,0 +1,22 @@ +{ + "expectation_version": "1.0", + "case_id": "ledger-charge-limit", + "packet_valid": true, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.ledger-charge-boundary", + "requirement": "A charge equal to the available balance succeeds.", + "trigger": "the charged amount lands exactly on the computed availability", + "surface": "ledger.py:can_charge", + "consequence": "an account holder cannot spend the last cent they own", + "severity": "blocking", + "equivalent_formulations": [ + "off by one at the boundary", + "rejects a charge equal to the available balance", + "strict comparison excludes equality" + ] + } + ], + "accepted_non_findings": [] +} diff --git a/review-suite/evals/corpus/private/expectations/pricing-tier-lookup.json b/review-suite/evals/corpus/private/expectations/pricing-tier-lookup.json new file mode 100644 index 0000000..7cea776 --- /dev/null +++ b/review-suite/evals/corpus/private/expectations/pricing-tier-lookup.json @@ -0,0 +1,35 @@ +{ + "expectation_version": "1.0", + "case_id": "pricing-tier-lookup", + "packet_valid": true, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.pricing-edge-selection", + "requirement": "A quantity that sits exactly on a tier ceiling is quoted in that tier.", + "trigger": "a quote asks for exactly the published ceiling quantity", + "consequence": "the customer is quoted from the next tier up", + "surface": "pricing.py:select_tier", + "severity": "blocking", + "equivalent_formulations": [ + "picks the wrong tier edge", + "wrong tier at the edge", + "ceiling is treated as exclusive" + ] + }, + { + "id": "rc.pricing-stale-table", + "requirement": "An updated tier table is used by later quotes.", + "trigger": "an operator edits the table while the service keeps running", + "consequence": "quotes keep using the table captured when the process started", + "surface": "pricing.py:select_tier", + "severity": "blocking", + "equivalent_formulations": [ + "caches the stale tier table", + "serves a stale table", + "reads the table captured at import time" + ] + } + ], + "accepted_non_findings": [] +} diff --git a/review-suite/evals/corpus/private/expectations/session-token-refresh.json b/review-suite/evals/corpus/private/expectations/session-token-refresh.json new file mode 100644 index 0000000..b654ddc --- /dev/null +++ b/review-suite/evals/corpus/private/expectations/session-token-refresh.json @@ -0,0 +1,34 @@ +{ + "expectation_version": "1.0", + "case_id": "session-token-refresh", + "packet_valid": true, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.session-refresh-deadline", + "requirement": "Refreshing an active session keeps it usable for a further full window.", + "trigger": "an operator extends a session late in its original window", + "consequence": "an operator is signed out in the middle of active work", + "surface": "session.py:refresh", + "severity": "blocking", + "equivalent_formulations": [ + "stale expiry clock", + "reuses the original issue time", + "deadline is not recomputed" + ] + }, + { + "id": "rc.session-audit-record", + "requirement": "Every refresh is recorded so an operator can reconstruct it later.", + "trigger": "an operator reads the log after several extensions", + "consequence": "the recorded history cannot distinguish one extension from another", + "surface": "audit.py:write_entry", + "severity": "strong_recommendation", + "equivalent_formulations": [ + "the recorded entry omits which moment the extension happened", + "no timestamp distinguishes repeated entries" + ] + } + ], + "accepted_non_findings": [] +} diff --git a/review-suite/evals/corpus/private/expectations/webhook-retry-queue.json b/review-suite/evals/corpus/private/expectations/webhook-retry-queue.json new file mode 100644 index 0000000..9c2fd8c --- /dev/null +++ b/review-suite/evals/corpus/private/expectations/webhook-retry-queue.json @@ -0,0 +1,22 @@ +{ + "expectation_version": "1.0", + "case_id": "webhook-retry-queue", + "packet_valid": true, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.webhook-redelivery-idempotence", + "requirement": "A given delivery changes downstream state at most once.", + "trigger": "the upstream sender delivers the same identifier twice", + "consequence": "downstream totals drift upward every time a delivery repeats", + "surface": "webhook.py:handle", + "severity": "blocking", + "equivalent_formulations": [ + "retries without a de-duplication key", + "a redelivered event is applied more than once", + "the retry loop is not idempotent" + ] + } + ], + "accepted_non_findings": [] +} diff --git a/review-suite/evals/corpus/private/provenance/catalog-import-feed.json b/review-suite/evals/corpus/private/provenance/catalog-import-feed.json new file mode 100644 index 0000000..4011ef8 --- /dev/null +++ b/review-suite/evals/corpus/private/provenance/catalog-import-feed.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "catalog-import-feed", + "origin": "synthetic", + "retention_authority": "Authored for this repository as protocol proof; no third-party or customer material is reproduced.", + "sanitization": "Fully invented module names, identifiers, and values; nothing derived from private code, credentials, or customer records.", + "recorded_at": "2026-07-26", + "notes": "Protocol and grader proof only. #58 owns the representative scored corpus." +} diff --git a/review-suite/evals/corpus/private/provenance/invoice-export-batch.json b/review-suite/evals/corpus/private/provenance/invoice-export-batch.json new file mode 100644 index 0000000..675753f --- /dev/null +++ b/review-suite/evals/corpus/private/provenance/invoice-export-batch.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "invoice-export-batch", + "origin": "synthetic", + "retention_authority": "Authored for this repository as protocol proof; no third-party or customer material is reproduced.", + "sanitization": "Fully invented module names, identifiers, and values; nothing derived from private code, credentials, or customer records.", + "recorded_at": "2026-07-26", + "notes": "Protocol and grader proof only. #58 owns the representative scored corpus." +} diff --git a/review-suite/evals/corpus/private/provenance/ledger-charge-limit.json b/review-suite/evals/corpus/private/provenance/ledger-charge-limit.json new file mode 100644 index 0000000..e834fca --- /dev/null +++ b/review-suite/evals/corpus/private/provenance/ledger-charge-limit.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "ledger-charge-limit", + "origin": "synthetic", + "retention_authority": "Authored for this repository as protocol proof; no third-party or customer material is reproduced.", + "sanitization": "Fully invented module names, identifiers, and values; nothing derived from private code, credentials, or customer records.", + "recorded_at": "2026-07-26", + "notes": "Protocol and grader proof only. #58 owns the representative scored corpus." +} diff --git a/review-suite/evals/corpus/private/provenance/pricing-tier-lookup.json b/review-suite/evals/corpus/private/provenance/pricing-tier-lookup.json new file mode 100644 index 0000000..05ea8e3 --- /dev/null +++ b/review-suite/evals/corpus/private/provenance/pricing-tier-lookup.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "pricing-tier-lookup", + "origin": "synthetic", + "retention_authority": "Authored for this repository as protocol proof; no third-party or customer material is reproduced.", + "sanitization": "Fully invented module names, identifiers, and values; nothing derived from private code, credentials, or customer records.", + "recorded_at": "2026-07-26", + "notes": "Protocol and grader proof only. #58 owns the representative scored corpus." +} diff --git a/review-suite/evals/corpus/private/provenance/session-token-refresh.json b/review-suite/evals/corpus/private/provenance/session-token-refresh.json new file mode 100644 index 0000000..551b1fb --- /dev/null +++ b/review-suite/evals/corpus/private/provenance/session-token-refresh.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "session-token-refresh", + "origin": "synthetic", + "retention_authority": "Authored for this repository as protocol proof; no third-party or customer material is reproduced.", + "sanitization": "Fully invented module names, identifiers, and values; nothing derived from private code, credentials, or customer records.", + "recorded_at": "2026-07-26", + "notes": "Protocol and grader proof only. #58 owns the representative scored corpus." +} diff --git a/review-suite/evals/corpus/private/provenance/webhook-retry-queue.json b/review-suite/evals/corpus/private/provenance/webhook-retry-queue.json new file mode 100644 index 0000000..3efe287 --- /dev/null +++ b/review-suite/evals/corpus/private/provenance/webhook-retry-queue.json @@ -0,0 +1,9 @@ +{ + "provenance_version": "1.0", + "case_id": "webhook-retry-queue", + "origin": "synthetic", + "retention_authority": "Authored for this repository as protocol proof; no third-party or customer material is reproduced.", + "sanitization": "Fully invented module names, identifiers, and values; nothing derived from private code, credentials, or customer records.", + "recorded_at": "2026-07-26", + "notes": "Protocol and grader proof only. #58 owns the representative scored corpus." +} diff --git a/review-suite/evals/corpus/reviewer/PROMPT.md b/review-suite/evals/corpus/reviewer/PROMPT.md new file mode 100644 index 0000000..bba42bc --- /dev/null +++ b/review-suite/evals/corpus/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/corpus/reviewer/catalog-import-feed/packet.json b/review-suite/evals/corpus/reviewer/catalog-import-feed/packet.json new file mode 100644 index 0000000..ebdf61b --- /dev/null +++ b/review-suite/evals/corpus/reviewer/catalog-import-feed/packet.json @@ -0,0 +1,48 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/catalog", + "base_branch": "main" + }, + "candidate": { + "head_sha": "6a6a6a6a6a6a6a6a6a6a6a6a6a6a6a6a6a6a6a6a", + "comparison_base_sha": "6b6b6b6b6b6b6b6b6b6b6b6b6b6b6b6b6b6b6b6b", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/catalog.py b/catalog.py\n--- a/catalog.py\n+++ b/catalog.py\n@@ -8,2 +8,2 @@ def parse_feed(payload):\n def import_feed(payload):\n- return [normalize(row) for row in parse_feed(payload)]\n+ return [normalize(row) for row in parse_feed(payload) if row.sku]\n" + } + }, + "change_contract": { + "goal": "Skip catalog rows that arrive without a stock identifier.", + "acceptance_criteria": [ + "A row with no stock identifier is not imported.", + "Every other row is imported exactly as before." + ], + "non_goals": [ + "Change how a stock identifier is generated upstream." + ], + "preserved_behaviors": [ + "Row order is preserved for imported rows." + ] + }, + "sources": { + "repository_instructions": [], + "named_documents": [], + "nearby_patterns": [ + { + "label": "Feed tests", + "location": "tests/test_catalog.py" + } + ] + }, + "validation": [ + { + "name": "catalog tests", + "command": "pytest tests/test_catalog.py", + "scope": "focused", + "status": "passed", + "result": "3 passed" + } + ] +} diff --git a/review-suite/evals/corpus/reviewer/invoice-export-batch/packet.json b/review-suite/evals/corpus/reviewer/invoice-export-batch/packet.json new file mode 100644 index 0000000..6d52cd3 --- /dev/null +++ b/review-suite/evals/corpus/reviewer/invoice-export-batch/packet.json @@ -0,0 +1,70 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/invoice", + "base_branch": "main" + }, + "candidate": { + "head_sha": "3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a", + "comparison_base_sha": "3b3b3b3b3b3b3b3b3b3b3b3b3b3b3b3b3b3b3b3b", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/invoice.py b/invoice.py\n--- a/invoice.py\n+++ b/invoice.py\n@@ -50,3 +50,3 @@ def _bx(rows, size):\n def export(rows):\n- for group in _bx(rows, 100):\n+ for group in _bx(rows, EXPORT_BATCH_SIZE):\n yield render(group)\n" + } + }, + "change_contract": { + "goal": "Make the invoice export batch size configurable.", + "acceptance_criteria": [ + "The export batch size comes from configuration instead of a literal.", + "The rendered output for a given set of rows is unchanged." + ], + "non_goals": [ + "Change the rendered invoice format." + ], + "preserved_behaviors": [ + "Rows are exported in their original order." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Configuration", + "location": "AGENTS.md", + "summary": "Tunables live in settings." + } + ], + "named_documents": [], + "nearby_patterns": [ + { + "label": "Existing tunables", + "location": "settings.py" + }, + { + "label": "Export tests", + "location": "tests/test_invoice.py" + } + ] + }, + "validation": [ + { + "name": "invoice tests", + "command": "pytest tests/test_invoice.py", + "scope": "focused", + "status": "passed", + "result": "14 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "203 passed" + } + ], + "context": { + "operational": [ + "EXPORT_BATCH_SIZE defaults to 100, matching the previous literal." + ] + } +} diff --git a/review-suite/evals/corpus/reviewer/ledger-charge-limit/packet.json b/review-suite/evals/corpus/reviewer/ledger-charge-limit/packet.json new file mode 100644 index 0000000..e66a6ea --- /dev/null +++ b/review-suite/evals/corpus/reviewer/ledger-charge-limit/packet.json @@ -0,0 +1,66 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/ledger", + "base_branch": "main" + }, + "candidate": { + "head_sha": "1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a", + "comparison_base_sha": "1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/ledger.py b/ledger.py\n--- a/ledger.py\n+++ b/ledger.py\n@@ -12,2 +12,2 @@ def available(account):\n def can_charge(account, amount):\n- return amount <= available(account)\n+ return amount < available(account)\n" + } + }, + "change_contract": { + "goal": "Refuse a charge that would overdraw an account.", + "acceptance_criteria": [ + "A charge equal to the available balance succeeds.", + "A charge above the available balance is refused." + ], + "non_goals": [ + "Introduce an overdraft product." + ], + "preserved_behaviors": [ + "Holds continue to reduce the available balance." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Money handling", + "location": "AGENTS.md", + "summary": "Amounts are integer cents." + } + ], + "named_documents": [], + "nearby_patterns": [ + { + "label": "Charge limit tests", + "location": "tests/test_ledger.py" + } + ] + }, + "validation": [ + { + "name": "ledger tests", + "command": "pytest tests/test_ledger.py", + "scope": "focused", + "status": "passed", + "result": "6 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "48 passed" + } + ], + "context": { + "data": [ + "Balances and holds are stored as integer cents." + ] + } +} diff --git a/review-suite/evals/corpus/reviewer/pricing-tier-lookup/packet.json b/review-suite/evals/corpus/reviewer/pricing-tier-lookup/packet.json new file mode 100644 index 0000000..3878061 --- /dev/null +++ b/review-suite/evals/corpus/reviewer/pricing-tier-lookup/packet.json @@ -0,0 +1,71 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/pricing", + "base_branch": "main" + }, + "candidate": { + "head_sha": "5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a", + "comparison_base_sha": "5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b5b", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/pricing.py b/pricing.py\n--- a/pricing.py\n+++ b/pricing.py\n@@ -16,4 +16,4 @@ TIERS = load_tiers()\n def select_tier(units):\n- for tier in load_tiers():\n- if units <= tier.ceiling:\n+ for tier in TIERS:\n+ if units < tier.ceiling:\n return tier\n" + } + }, + "change_contract": { + "goal": "Speed up tier selection for the quoting endpoint.", + "acceptance_criteria": [ + "A quantity that sits exactly on a tier ceiling is quoted in that tier.", + "An updated tier table is used by later quotes." + ], + "non_goals": [ + "Change the published tier ceilings." + ], + "preserved_behaviors": [ + "Every caller of select_tier receives the same tier for the same quantity." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Pricing rules", + "location": "AGENTS.md", + "summary": "Published ceilings are inclusive." + } + ], + "named_documents": [ + { + "label": "Tier table", + "location": "docs/pricing.md" + } + ], + "nearby_patterns": [ + { + "label": "Pricing tests", + "location": "tests/test_pricing.py" + } + ] + }, + "validation": [ + { + "name": "pricing tests", + "command": "pytest tests/test_pricing.py", + "scope": "focused", + "status": "passed", + "result": "7 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "64 passed" + } + ], + "context": { + "operational": [ + "Operators edit the tier table while the service is running." + ] + } +} diff --git a/review-suite/evals/corpus/reviewer/session-token-refresh/packet.json b/review-suite/evals/corpus/reviewer/session-token-refresh/packet.json new file mode 100644 index 0000000..2cd6a9a --- /dev/null +++ b/review-suite/evals/corpus/reviewer/session-token-refresh/packet.json @@ -0,0 +1,75 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/session", + "base_branch": "main" + }, + "candidate": { + "head_sha": "2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a", + "comparison_base_sha": "2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/session.py b/session.py\n--- a/session.py\n+++ b/session.py\n@@ -29,2 +29,3 @@ class Session:\n def refresh(self):\n- self.expires_at = now() + WINDOW\n+ self.expires_at = self.issued_at + WINDOW\n+ audit.write_entry(self.id, \"refresh\")\ndiff --git a/audit.py b/audit.py\n--- a/audit.py\n+++ b/audit.py\n@@ -6,2 +6,3 @@ def write_entry(subject, action):\n record = {\"subject\": subject, \"action\": action}\n+ record[\"source\"] = \"session\"\n store.append(record)\n" + } + }, + "change_contract": { + "goal": "Let an operator extend an active session and record that it happened.", + "acceptance_criteria": [ + "Refreshing an active session keeps it usable for a further full window.", + "Every refresh is recorded so an operator can reconstruct it later." + ], + "non_goals": [ + "Change how a session is first issued." + ], + "preserved_behaviors": [ + "An abandoned session still lapses after one window." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Audit rules", + "location": "AGENTS.md", + "summary": "Every state change is recorded." + } + ], + "named_documents": [ + { + "label": "Session lifecycle", + "location": "docs/sessions.md" + } + ], + "nearby_patterns": [ + { + "label": "Session tests", + "location": "tests/test_session.py" + }, + { + "label": "Audit writer", + "location": "audit.py" + } + ] + }, + "validation": [ + { + "name": "session tests", + "command": "pytest tests/test_session.py", + "scope": "focused", + "status": "passed", + "result": "9 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "121 passed" + } + ], + "context": { + "operational": [ + "Sessions are held in one process and are not shared." + ] + } +} diff --git a/review-suite/evals/corpus/reviewer/webhook-retry-queue/packet.json b/review-suite/evals/corpus/reviewer/webhook-retry-queue/packet.json new file mode 100644 index 0000000..1f2c0d1 --- /dev/null +++ b/review-suite/evals/corpus/reviewer/webhook-retry-queue/packet.json @@ -0,0 +1,78 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/webhook", + "base_branch": "main" + }, + "candidate": { + "head_sha": "4a4a4a4a4a4a4a4a4a4a4a4a4a4a4a4a4a4a4a4a", + "comparison_base_sha": "4b4b4b4b4b4b4b4b4b4b4b4b4b4b4b4b4b4b4b4b", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/webhook.py b/webhook.py\n--- a/webhook.py\n+++ b/webhook.py\n@@ -20,2 +20,3 @@ def enqueue(event):\n def handle(event):\n- apply_event(event)\n+ for _ in range(RETRIES):\n+ apply_event(event)\n" + } + }, + "change_contract": { + "goal": "Keep applying an inbound event when the downstream call fails.", + "acceptance_criteria": [ + "A transient downstream failure no longer drops the event.", + "A given delivery changes downstream state at most once." + ], + "non_goals": [ + "Change how events are rendered for operators." + ], + "preserved_behaviors": [ + "Events are applied in the order they were received." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Queue rules", + "location": "AGENTS.md", + "summary": "Handlers must tolerate redelivery." + } + ], + "named_documents": [ + { + "label": "Delivery contract", + "location": "docs/webhooks.md" + } + ], + "nearby_patterns": [ + { + "label": "Queue tests", + "location": "tests/test_webhook.py" + }, + { + "label": "Presentation helper", + "location": "presentation.py" + } + ] + }, + "validation": [ + { + "name": "webhook tests", + "command": "pytest tests/test_webhook.py", + "scope": "focused", + "status": "passed", + "result": "11 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "96 passed" + } + ], + "context": { + "data": [ + "Each delivery carries a stable delivery identifier." + ], + "operational": [ + "The upstream sender redelivers on any non-2xx reply." + ] + } +} diff --git a/review-suite/scripts/evals/__init__.py b/review-suite/scripts/evals/__init__.py new file mode 100644 index 0000000..89c0291 --- /dev/null +++ b/review-suite/scripts/evals/__init__.py @@ -0,0 +1 @@ +"""Canonical result-blind replay evaluator for the repository review suite.""" diff --git a/review-suite/scripts/evals/audit_corpus.py b/review-suite/scripts/evals/audit_corpus.py new file mode 100644 index 0000000..e3ce38d --- /dev/null +++ b/review-suite/scripts/evals/audit_corpus.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +"""Audit the replay corpus without launching any model. + +Checks, in order: + +1. corpus index, expectation, and provenance schemas; +2. cross-field expectation semantics and packet validity agreement; +3. reviewer/private separation, orphaned files, and reviewer-prompt wording + (all inherited from `corpus.load_corpus`, which the runner shares); +4. case identifiers and reviewer-visible filenames for outcome-revealing + tokens; and +5. the complete executor request that each case would produce, structurally + and textually, for private expectation or provenance leakage. + +Exit status is 0 when the corpus can be trusted for a blind evaluation and 1 +otherwise. This command never spawns an executor and never costs money. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +if __package__ in (None, ""): + sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + from evals import corpus, grader, protocol, runner +else: + from . import corpus, grader, protocol, runner + + +def audit(corpus_root: Path | None) -> list[str]: + """Return every reason the corpus cannot be trusted, or an empty list.""" + try: + loaded = corpus.load_corpus(corpus_root) + except corpus.CorpusError as error: + return [str(error)] + + # `load_corpus` already rejected an outcome-hinting reviewer prompt, so + # every caller inherits that gate from one place. + errors: list[str] = [] + if loaded.grader_version != grader.GRADER_VERSION: + errors.append( + f"corpus grader_version {loaded.grader_version!r} does not match the " + f"shipped grader {grader.GRADER_VERSION!r}" + ) + try: + skill_prompt = runner.target_skill_prompt( + loaded.target_skill, loaded.target_skill_dependencies + ) + except runner.ConfigurationError as error: + return errors + [str(error)] + + documents = runner.contract_documents() + for case in loaded.cases: + request = protocol.build_request( + case_id=case.case_id, + target_skill=loaded.target_skill, + skill_prompt=skill_prompt, + contract_documents=documents, + instructions=case.instructions, + packet=case.packet, + run_number=1, + suite_commit="audit", + corpus_version=loaded.corpus_version, + started_at="audit", + ) + errors.extend( + f"{case.case_id}: {error}" + for error in protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + ) + return errors + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--corpus", type=Path, default=None) + args = parser.parse_args(argv) + + errors = audit(args.corpus) + if errors: + for error in errors: + print(error, file=sys.stderr) + print(f"corpus audit failed with {len(errors)} error(s)", file=sys.stderr) + return 1 + print("corpus audit passed") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/review-suite/scripts/evals/claude_executor.py b/review-suite/scripts/evals/claude_executor.py new file mode 100644 index 0000000..77ba932 --- /dev/null +++ b/review-suite/scripts/evals/claude_executor.py @@ -0,0 +1,229 @@ +#!/usr/bin/env python3 +"""Real-runtime replay executor backed by Claude Code headless mode. + +Reads one result-blind request on stdin, asks a fresh `claude -p` process to +perform the review described by the target skill prompt and the raw packet, and +writes one protocol response on stdout. It never sees expected findings, case +names, or grader data, because the runner never puts them in the request. + +Product-specific launch details stay here. The core protocol in `protocol.py` +knows nothing about Claude, so another runtime only needs its own adapter. + +Usage: + just eval-review-suite "python3 review-suite/scripts/evals/claude_executor.py" +""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path +from typing import Any + +if __package__ in (None, ""): + sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + from evals import protocol +else: + from . import protocol + +EXECUTOR_NAME = "review-suite-claude-executor" +EXECUTOR_VERSION = "1.0" + + +def build_prompt(request: dict[str, Any]) -> str: + """Render the reviewer-visible request as one headless prompt.""" + documents = "\n\n".join( + f"### {name}\n\n{text}" + for name, text in sorted(request["contract_documents"].items()) + ) + return "\n".join( + [ + "You are a read-only reviewer executing the skill below for one", + "candidate. Do not run tools, modify files, or inspect any", + "repository. Reason only from the artifacts supplied here.", + "", + "## Skill", + request["skill_prompt"], + "", + "## Review suite contracts", + documents, + "", + "## Instructions", + request["instructions"], + "", + "## Review packet (JSON)", + json.dumps(request["packet"], indent=2, sort_keys=True), + "", + "## Answer format", + "Return ONLY one JSON object conforming to", + "review-result.schema.json, with no prose and no code fence. Bind", + "it to this candidate identity:", + json.dumps(request["run"]["candidate"], sort_keys=True), + ] + ) + + +def extract_json_object(text: str) -> dict[str, Any]: + text = text.strip() + fenced = re.search(r"```(?:json)?\s*(\{.*?\})\s*```", text, re.DOTALL) + candidate = fenced.group(1) if fenced else text + start = candidate.find("{") + end = candidate.rfind("}") + if start < 0 or end <= start: + raise ValueError("the runtime returned no JSON object") + value = json.loads(candidate[start : end + 1]) + if not isinstance(value, dict): + raise ValueError("the runtime returned a non-object JSON value") + return value + + +def run_claude( + prompt: str, *, claude_bin: str, model: str | None +) -> tuple[dict[str, Any], dict[str, Any]]: + """Run one fresh headless process and return (review result, envelope).""" + command = [claude_bin, "-p", "--output-format", "json"] + if model: + command.extend(["--model", model]) + completed = subprocess.run( + command, input=prompt, capture_output=True, text=True, check=False + ) + if completed.returncode: + raise RuntimeError( + f"{claude_bin} exited {completed.returncode}: " + f"{completed.stderr.strip()[-500:]}" + ) + envelope = json.loads(completed.stdout) + if not isinstance(envelope, dict): + raise RuntimeError("headless output was not one JSON object") + result_text = envelope.get("result") + if not isinstance(result_text, str): + raise RuntimeError("headless output carried no result text") + return extract_json_object(result_text), envelope + + +#: Every input-side token field the runtime reports. Under prompt caching the +#: uncached `input_tokens` count is a tiny residue - a real prompt carrying the +#: skill closure, the contracts, and a packet reports almost all of its tokens +#: as cache creation or cache read. Totalling only `input_tokens` understated a +#: measured 16456-token prompt as 2, so the cost envelope a baseline freezes +#: must be built from all three. +INPUT_TOKEN_FIELDS = ( + "input_tokens", + "cache_creation_input_tokens", + "cache_read_input_tokens", +) + + +def _number(value: Any) -> float | int | None: + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + return value + + +def model_from(envelope: dict[str, Any]) -> str | None: + """Resolve the model identity the runtime actually used. + + Headless output carries no top-level `model` string; it reports + `modelUsage` as a mapping keyed by model identifier. Treating that mapping as + a string silently dropped model identity from every recorded attempt, which + would make a frozen baseline stratum incomparable. + """ + model = envelope.get("model") + if isinstance(model, str) and model.strip(): + return model + usage_by_model = envelope.get("modelUsage") + if isinstance(usage_by_model, dict): + names = sorted(str(name) for name in usage_by_model if str(name).strip()) + if names: + # More than one model can contribute to a single turn; record all. + return ",".join(names) + return None + + +def usage_from(envelope: dict[str, Any]) -> dict[str, Any] | None: + """Map whatever usage the runtime reported into the protocol shape.""" + reported = envelope.get("usage") or {} + usage: dict[str, Any] = {} + if isinstance(reported, dict): + input_values = [ + value + for value in (_number(reported.get(field)) for field in INPUT_TOKEN_FIELDS) + if value is not None + ] + if input_values: + usage["input_tokens"] = sum(input_values) + output = _number(reported.get("output_tokens")) + if output is not None: + usage["output_tokens"] = output + cost = _number(envelope.get("total_cost_usd")) + if cost is not None: + usage["cost_usd"] = cost + return usage or None + + +def respond(request: dict[str, Any], args: argparse.Namespace) -> dict[str, Any]: + executor = { + "name": EXECUTOR_NAME, + "version": EXECUTOR_VERSION, + "runtime": Path(args.claude_bin).name, + } + if args.model: + executor["model"] = args.model + try: + result, envelope = run_claude( + build_prompt(request), claude_bin=args.claude_bin, model=args.model + ) + except (OSError, RuntimeError, ValueError, json.JSONDecodeError) as error: + return { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "runtime_failure", + "simulation": False, + "executor": executor, + "failure": {"reason": str(error)}, + } + + # Required identity, so fail closed rather than record an attempt that + # cannot say which model answered. `--model` is an acceptable source; a + # silently absent model is not. + resolved_model = model_from(envelope) or args.model + if not resolved_model: + return { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "runtime_failure", + "simulation": False, + "executor": executor, + "failure": { + "reason": ( + "the runtime reported no model identity; refusing to record " + "an attempt that cannot name the model that answered" + ) + }, + } + executor["model"] = resolved_model + response: dict[str, Any] = { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "blocked" if result.get("verdict") == "blocked" else "review_result", + "simulation": False, + "executor": executor, + "result": result, + } + usage = usage_from(envelope) + if usage: + response["usage"] = usage + return response + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--claude-bin", default="claude") + parser.add_argument("--model", default=None) + args = parser.parse_args(argv) + protocol.write_response(respond(protocol.read_request(), args)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/review-suite/scripts/evals/corpus.py b/review-suite/scripts/evals/corpus.py new file mode 100644 index 0000000..9e9bb20 --- /dev/null +++ b/review-suite/scripts/evals/corpus.py @@ -0,0 +1,296 @@ +#!/usr/bin/env python3 +"""Versioned, result-blind corpus contract for review replay evaluation. + +The layout physically separates the three kinds of data so a reviewer-visible +read can never reach grading evidence: + + /corpus.json version metadata, case ids + /reviewer/PROMPT.md shared reviewer instructions + /reviewer//packet.json reviewer-visible artifacts + /private/expectations/.json expected material root causes + /private/provenance/.json origin and retention authority + +Loading fails closed. A missing expectation, a malformed schema, an orphaned +file, or an outcome-revealing identifier is an error before any executor +process is started. +""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from . import protocol + +DEFAULT_CORPUS = protocol.REVIEW_SUITE / "evals" / "corpus" +REVIEWER_PROMPT = "PROMPT.md" + +#: Filenames permitted inside a reviewer-visible case directory. Anything else +#: risks shipping grading evidence to the target reviewer. +REVIEWER_CASE_FILES = frozenset({"packet.json"}) + +#: Substrings that would reveal a verdict, severity, failure class, connector +#: disposition, or target finding through a case id or reviewer-visible +#: filename. Case names must describe the subject matter, never the answer. +OUTCOME_REVEALING_TOKENS = ( + "accept", + "blocked", + "blocking", + "broken", + "bug", + "changes", + "clean", + "control", + "defect", + "defer", + "duplicate", + "escape", + "expected", + "fail", + "false", + "flaw", + "insecure", + "invalid", + "miss", + "negative", + "overengineered", + "polish", + "positive", + "regression", + "reject", + "severity", + "simplif", + "speculative", + "unsafe", + "verdict", + "vulnerab", +) + + +@dataclass(frozen=True) +class Case: + """One corpus case with its three separately validated parts.""" + + case_id: str + packet: dict[str, Any] + instructions: str + expectation: dict[str, Any] + provenance: dict[str, Any] + + @property + def case_ref(self) -> str: + return protocol.case_ref(self.case_id) + + +@dataclass(frozen=True) +class Corpus: + root: Path + corpus_version: str + grader_version: str + target_skill: str + target_skill_dependencies: tuple[str, ...] + cases: tuple[Case, ...] + + +class CorpusError(ValueError): + """Raised when the corpus cannot be trusted for a blind evaluation.""" + + +def _load_json(path: Path) -> Any: + if not path.is_file(): + raise CorpusError(f"missing {path}") + try: + return json.loads(path.read_text()) + except json.JSONDecodeError as error: + raise CorpusError(f"invalid JSON in {path}: {error}") from error + + +def revealing_tokens(name: str) -> list[str]: + """Return the outcome-revealing substrings present in one name.""" + lowered = name.lower() + return [token for token in OUTCOME_REVEALING_TOKENS if token in lowered] + + +def _expectation_semantics(expectation: dict[str, Any]) -> list[str]: + """Cross-field expectation rules JSON Schema cannot express clearly.""" + errors = [] + verdict = expectation.get("expected_verdict") + root_causes = expectation.get("material_root_causes") or [] + packet_valid = expectation.get("packet_valid") + + if verdict == "changes_required" and not root_causes: + errors.append("changes_required requires at least one material root cause") + if verdict == "clean" and root_causes: + errors.append("clean cannot expect a material root cause") + if verdict == "blocked": + if root_causes: + errors.append("blocked cannot expect a material root cause") + if packet_valid is not False: + errors.append("blocked requires packet_valid false") + elif packet_valid is not True: + errors.append(f"{verdict} requires packet_valid true") + + for field, prefix in ( + ("material_root_causes", "rc"), + ("accepted_non_findings", "anf"), + ): + identifiers = [item.get("id") for item in expectation.get(field) or []] + duplicates = sorted({i for i in identifiers if identifiers.count(i) > 1}) + if duplicates: + errors.append(f"duplicate {prefix} id(s): " + ", ".join(duplicates)) + return errors + + +def load_case(root: Path, case_id: str) -> Case: + """Load and validate one case's reviewer-visible and private parts.""" + errors: list[str] = [] + + tokens = revealing_tokens(case_id) + if tokens: + errors.append( + f"case id reveals outcome via {', '.join(tokens)}; " + "name cases after their subject matter" + ) + + reviewer_dir = root / "reviewer" / case_id + if not reviewer_dir.is_dir(): + raise CorpusError(f"missing reviewer directory {reviewer_dir}") + for path in sorted(reviewer_dir.iterdir()): + if path.name not in REVIEWER_CASE_FILES: + errors.append(f"unpermitted reviewer-visible file {path}") + file_tokens = revealing_tokens(path.name) + if file_tokens: + errors.append( + f"reviewer-visible filename {path.name} reveals outcome via " + + ", ".join(file_tokens) + ) + + packet = _load_json(reviewer_dir / "packet.json") + expectation = _load_json(root / "private" / "expectations" / f"{case_id}.json") + provenance = _load_json(root / "private" / "provenance" / f"{case_id}.json") + + for name, document, schema in ( + ("expectation", expectation, "expectation.schema.json"), + ("provenance", provenance, "provenance.schema.json"), + ): + errors.extend( + f"{name}: {error}" for error in protocol.validate_against(schema, document) + ) + if document.get("case_id") != case_id: + errors.append(f"{name}: case_id does not match {case_id}") + + errors.extend( + f"expectation: {error}" for error in _expectation_semantics(expectation) + ) + + packet_errors = protocol.VALIDATOR.validate_packet(packet) + if expectation.get("packet_valid") and packet_errors: + errors.extend(f"packet: {error}" for error in packet_errors) + elif expectation.get("packet_valid") is False and not packet_errors: + errors.append("packet: packet_valid false but the packet validates") + + if errors: + raise CorpusError(f"{case_id}: " + "; ".join(errors)) + + instructions = (root / "reviewer" / REVIEWER_PROMPT).read_text() + return Case( + case_id=case_id, + packet=packet, + instructions=instructions, + expectation=expectation, + provenance=provenance, + ) + + +def _orphan_errors(root: Path, declared: set[str]) -> list[str]: + """Report corpus files that no declared case owns, in both directions.""" + errors = [] + reviewer_root = root / "reviewer" + present = {path.name for path in reviewer_root.iterdir() if path.is_dir()} + for extra in sorted(present - declared): + errors.append(f"reviewer/{extra} is not declared in corpus.json") + for subdir, suffix in (("expectations", ".json"), ("provenance", ".json")): + directory = root / "private" / subdir + if not directory.is_dir(): + errors.append(f"missing {directory}") + continue + names = {path.name[: -len(suffix)] for path in directory.glob(f"*{suffix}")} + for extra in sorted(names - declared): + errors.append(f"private/{subdir}/{extra}{suffix} is not declared") + return errors + + +def load_corpus(root: Path | None = None) -> Corpus: + """Load the whole corpus, failing closed on any integrity problem.""" + root = Path(root) if root else DEFAULT_CORPUS + if not root.is_dir(): + raise CorpusError(f"missing corpus directory {root}") + index = _load_json(root / "corpus.json") + schema_errors = protocol.validate_against("corpus.schema.json", index) + if schema_errors: + raise CorpusError("corpus.json: " + "; ".join(schema_errors)) + if not (root / "reviewer" / REVIEWER_PROMPT).is_file(): + raise CorpusError(f"missing {root / 'reviewer' / REVIEWER_PROMPT}") + + # Enforced here rather than only in the audit entrypoint: the runner is the + # one command that spends money, so a prompt hinting at an expected verdict + # must stop it before a single process is launched. + prompt_hits = prompt_errors(root) + if prompt_hits: + raise CorpusError("; ".join(prompt_hits)) + + declared = list(index["cases"]) + if len(set(declared)) != len(declared): + raise CorpusError("corpus.json: duplicate case id(s)") + orphans = _orphan_errors(root, set(declared)) + if orphans: + raise CorpusError("corpus.json: " + "; ".join(orphans)) + + # The declared closure must resolve before anything launches: shipping a + # target whose required siblings are absent would grade a compliant reviewer + # wrong for correctly refusing to review without them. + dependencies = tuple(index["target_skill_dependencies"]) + if len(set(dependencies)) != len(dependencies): + raise CorpusError("corpus.json: duplicate target_skill_dependencies") + if index["target_skill"] in dependencies: + raise CorpusError("corpus.json: target_skill lists itself as a dependency") + for skill in (index["target_skill"], *dependencies): + path = protocol.REPOSITORY_ROOT / "skills" / skill / "SKILL.md" + if not path.is_file(): + raise CorpusError(f"corpus.json: missing declared skill {path}") + + return Corpus( + root=root, + corpus_version=index["corpus_version"], + grader_version=index["grader_version"], + target_skill=index["target_skill"], + target_skill_dependencies=dependencies, + cases=tuple(load_case(root, case_id) for case_id in declared), + ) + + +#: Verdict and severity names the shared reviewer prompt must not mention. +PROMPT_FORBIDDEN_WORDS = frozenset( + { + "blocked", + "blocking", + "changes_required", + "clean", + "defer", + "strong_recommendation", + } +) + + +def prompt_errors(root: Path | None = None) -> list[str]: + """Reject shared reviewer instructions that hint at an expected outcome.""" + root = Path(root) if root else DEFAULT_CORPUS + text = (root / "reviewer" / REVIEWER_PROMPT).read_text() + words = set(re.findall(r"[a-z_]+", text.lower())) + hits = sorted(words & PROMPT_FORBIDDEN_WORDS) + if not hits: + return [] + return ["reviewer prompt names verdict or severity word(s): " + ", ".join(hits)] diff --git a/review-suite/scripts/evals/fixture_executor.py b/review-suite/scripts/evals/fixture_executor.py new file mode 100644 index 0000000..666b80d --- /dev/null +++ b/review-suite/scripts/evals/fixture_executor.py @@ -0,0 +1,319 @@ +#!/usr/bin/env python3 +"""Deterministic fresh-process stand-in for a compliant review runtime. + +This is a SIMULATION, not a model evaluation. It hand-codes the review a +compliant reviewer would return for each synthetic packet so the protocol, +failure taxonomy, grading interface, and reporting stay deterministically +testable without a paid runtime. It cannot detect a model misreading the +review contract. + +Every response is marked `"simulation": true`, and `runner.py` additionally +forces the simulation flag whenever this file is the executor, so no baseline +report can be produced from it. Use a real-runtime adapter such as +`claude_executor.py` for behavioural evidence. + +Injectable failures for protocol tests are selected with `--mode`. +""" + +from __future__ import annotations + +import argparse +import sys +import time +from pathlib import Path +from typing import Any + +if __package__ in (None, ""): + sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + from evals import protocol +else: + from . import protocol + +EXECUTOR = {"name": "review-suite-fixture-executor", "version": "1.0"} + +MODES = ( + "review", + "runtime_failure", + "malformed_json", + "malformed_result", + "protocol_mismatch", + "crash", + "hang", +) + + +def _finding( + identifier: str, + *, + severity: str, + location: str, + rule: str, + detail: str, + concern: str, + impact: str, + proposed_change: str, + expected_effect: str, +) -> dict[str, Any]: + return { + "id": identifier, + "lens": "correctness", + "severity": severity, + "confidence": "high", + "rule": rule, + "evidence": [{"location": location, "detail": detail}], + "concern": concern, + "impact": impact, + "proposed_change": proposed_change, + "expected_effect": expected_effect, + "location": location, + } + + +def _review(candidate: dict[str, Any], verdict: str, findings: list[dict[str, Any]]): + return { + "schema_version": "1.0", + "lens": "aggregate", + "candidate": dict(candidate), + "verdict": verdict, + "findings": findings, + "blocking_reasons": [], + } + + +def _ledger_review(candidate: dict[str, Any]) -> dict[str, Any]: + """Report the boundary defect in wording that paraphrases the expectation.""" + return _review( + candidate, + "changes_required", + [ + _finding( + "correctness.ledger-boundary", + severity="blocking", + location="ledger.py:14", + rule="A charge equal to the available balance must succeed.", + detail="The comparison became strict, so the equal case falls through.", + concern="The change introduces an off-by-one at the boundary.", + impact="A charge for the exact available balance is refused.", + proposed_change="Compare with less-than-or-equal and cover equality.", + expected_effect="The boundary charge succeeds again.", + ) + ], + ) + + +def _session_review(candidate: dict[str, Any]) -> dict[str, Any]: + """Report one root cause twice and touch a second surface only vaguely.""" + return _review( + candidate, + "changes_required", + [ + _finding( + "correctness.session-clock", + severity="blocking", + location="session.py:31", + rule="A refreshed session must extend the deadline.", + detail="The refresh reuses the original issue time.", + concern="The refresh path keeps a stale expiry clock.", + impact="Active operators are signed out mid-task.", + proposed_change="Recompute the deadline from the refresh instant.", + expected_effect="A refreshed session survives its original window.", + ), + _finding( + "correctness.session-clock-symptom", + severity="strong_recommendation", + location="session.py:31", + rule="A refreshed session must extend the deadline.", + detail="Sessions still lapse after refresh; a stale expiry clock.", + concern="Operators report early sign-out after refreshing.", + impact="The same lapse is visible from the sign-out path.", + proposed_change="Fix the shared deadline computation once.", + expected_effect="Both report paths stop observing early lapse.", + ), + _finding( + "correctness.session-audit-surface", + severity="strong_recommendation", + location="audit.py:8", + rule="Session changes must be auditable.", + detail="The audit writer is touched by this change.", + concern="The audit record may not describe the refresh.", + impact="Operators cannot reconstruct a refresh from the log.", + proposed_change="Record the refresh instant in the audit entry.", + expected_effect="The log explains each refresh.", + ), + ], + ) + + +def _invoice_review(candidate: dict[str, Any]) -> dict[str, Any]: + """Return no gating finding, with one tolerated observation recorded.""" + return _review( + candidate, + "clean", + [ + _finding( + "correctness.invoice-naming", + severity="defer", + location="invoice.py:52", + rule="The active change does not own the export helper naming.", + detail="The batch helper name predates this change.", + concern="The helper name is terse but untouched by this change.", + impact="Readability only; no behaviour is affected.", + proposed_change="Rename the helper in separate work.", + expected_effect="Clearer naming without widening this change.", + ) + ], + ) + + +def _webhook_review(candidate: dict[str, Any]) -> dict[str, Any]: + """Find the real root cause and also raise one unsupported gating claim.""" + return _review( + candidate, + "changes_required", + [ + _finding( + "correctness.webhook-replay", + severity="blocking", + location="webhook.py:22", + rule="A redelivered webhook must apply once.", + detail="The handler retries without a de-duplication key.", + concern="A redelivered event is applied more than once.", + impact="Duplicate side effects reach the downstream ledger.", + proposed_change="Key the retry on the delivery identifier.", + expected_effect="Redelivery becomes idempotent.", + ), + _finding( + "correctness.webhook-imagined-locale", + severity="blocking", + location="presentation.py:12", + rule="Timestamps should be localized for every reader.", + detail="The queue stores an absolute instant.", + concern="Readers in other zones may prefer local rendering.", + impact="Presentation preference only.", + proposed_change="Render queue timestamps in the reader locale.", + expected_effect="Locale-aware presentation.", + ), + ], + ) + + +def _pricing_review(candidate: dict[str, Any]) -> dict[str, Any]: + """Describe one shared surface in wording that fits two root causes.""" + return _review( + candidate, + "changes_required", + [ + _finding( + "correctness.pricing-lookup", + severity="blocking", + location="pricing.py:19", + rule="Tier selection must stay consistent for every caller.", + detail=( + "The lookup both picks the wrong tier edge and caches the " + "stale tier table." + ), + concern="The tier lookup is wrong at the edge and serves a stale table.", + impact="Callers are quoted a price from the wrong tier.", + proposed_change="Fix the edge selection and invalidate the table.", + expected_effect="Consistent tier pricing for every caller.", + ) + ], + ) + + +def _catalog_review(candidate: dict[str, Any]) -> dict[str, Any]: + """Refuse a merge verdict because the packet omits required evidence.""" + result = _review(candidate, "blocked", []) + result["blocking_reasons"] = [ + "The packet supplies no full-scope validation evidence, so no " + "trustworthy merge verdict is possible." + ] + return result + + +REVIEWS = { + "example/ledger": _ledger_review, + "example/session": _session_review, + "example/invoice": _invoice_review, + "example/webhook": _webhook_review, + "example/pricing": _pricing_review, + "example/catalog": _catalog_review, +} + + +def simulate(request: dict[str, Any]) -> dict[str, Any]: + """Return the hand-coded review for the packet's subject repository.""" + packet = request["packet"] + identity = (packet.get("repository") or {}).get("identity") + build = REVIEWS.get(identity) + if build is None: + return { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "runtime_failure", + "simulation": True, + "executor": EXECUTOR, + "failure": {"reason": f"no simulated review for {identity!r}"}, + } + result = build(request["run"]["candidate"]) + return { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "blocked" if result["verdict"] == "blocked" else "review_result", + "simulation": True, + "executor": EXECUTOR, + "result": result, + "usage": {"input_tokens": 0, "output_tokens": 0, "cost_usd": 0.0}, + } + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--mode", choices=MODES, default="review") + args = parser.parse_args(argv) + + if args.mode == "hang": + # Deliberately outstay any sane test timeout. + time.sleep(3600) + return 0 + if args.mode == "crash": + print("simulated executor crash", file=sys.stderr) + return 3 + if args.mode == "malformed_json": + sys.stdout.write("this is not JSON\n") + return 0 + + request = protocol.read_request() + + if args.mode == "protocol_mismatch": + response = simulate(request) + response["protocol_version"] = "0.9" + protocol.write_response(response) + return 0 + if args.mode == "runtime_failure": + protocol.write_response( + { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "runtime_failure", + "simulation": True, + "executor": EXECUTOR, + "failure": {"reason": "simulated runtime failure"}, + } + ) + return 0 + if args.mode == "malformed_result": + protocol.write_response( + { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "review_result", + "simulation": True, + "executor": EXECUTOR, + "result": {"verdict": "clean"}, + } + ) + return 0 + + protocol.write_response(simulate(request)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/review-suite/scripts/evals/grader.py b/review-suite/scripts/evals/grader.py new file mode 100644 index 0000000..a9e9323 --- /dev/null +++ b/review-suite/scripts/evals/grader.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Root-cause grading interface and the deterministic reference grader. + +A reviewer is graded on *material root causes*, never on prose. Each private +expectation describes a root cause as a requirement, a triggering condition, an +affected surface, a material consequence, and the formulations a competent +reviewer may use for it. An observed finding matches when it points at the +affected surface *and* uses one of the accepted formulations. + +The reference grader is protocol proof, not a calibrated grader and not a v1 +score. #58 owns calibration; this module owns the interface and its behaviour +on paraphrases, duplicate symptoms, partial matches, unexpected findings, +accepted non-findings, and ambiguous matches that need adjudication. +""" + +from __future__ import annotations + +import re +from typing import Any + +GRADER_VERSION = "1.0" + +#: How one observed finding relates to the private expectation. +CLASSIFICATIONS = ( + "matched", + "duplicate", + "partial", + "ambiguous", + "accepted", + "unexpected", +) + +#: Severities that make an observed finding gate a merge. +GATING_SEVERITIES = frozenset({"blocking", "strong_recommendation"}) + +#: File-extension and filler tokens that would make surface matching vacuous. +SURFACE_STOPWORDS = frozenset( + { + "diff", + "go", + "java", + "js", + "json", + "jsx", + "md", + "mjs", + "py", + "rb", + "rs", + "sql", + "ts", + "tsx", + "yaml", + "yml", + } +) + +#: Finding fields whose prose may carry an accepted formulation. +FINDING_TEXT_FIELDS = ( + "rule", + "concern", + "impact", + "proposed_change", + "expected_effect", +) + + +class GradingError(ValueError): + """Raised when grading cannot proceed, for example without expectations.""" + + +def normalize(text: str) -> str: + """Collapse prose to lowercase alphanumeric words for stable matching.""" + return re.sub(r"[^a-z0-9]+", " ", str(text).lower()).strip() + + +def _surface_tokens(text: str) -> set[str]: + return { + token + for token in normalize(text).split() + if token and token not in SURFACE_STOPWORDS + } + + +def finding_surfaces(finding: dict[str, Any]) -> set[str]: + """Return the surface tokens a finding points at.""" + locations = [finding.get("location") or ""] + locations.extend( + item.get("location") or "" for item in finding.get("evidence") or [] + ) + tokens: set[str] = set() + for location in locations: + tokens |= _surface_tokens(location) + return tokens + + +def finding_text(finding: dict[str, Any]) -> str: + """Return the normalized prose a formulation may be found in.""" + parts = [str(finding.get(field) or "") for field in FINDING_TEXT_FIELDS] + parts.extend( + str(item.get("detail") or "") for item in finding.get("evidence") or [] + ) + return normalize(" ".join(parts)) + + +def _signal_match(formulations: list[str], text: str) -> bool: + return any(normalize(item) and normalize(item) in text for item in formulations) + + +def match_strength(expected: dict[str, Any], finding: dict[str, Any]) -> str: + """Return `full`, `partial`, or `none` for one expectation/finding pair. + + `full` needs both signals: the finding names the affected surface and + describes the root cause in an accepted way. One signal alone is `partial` + and is reported for adjudication rather than silently scored either way. + """ + surface = _surface_tokens(expected.get("surface", "")) + surface_hit = bool(surface & finding_surfaces(finding)) + signal_hit = _signal_match( + expected.get("equivalent_formulations") or [], finding_text(finding) + ) + if surface_hit and signal_hit: + return "full" + if surface_hit or signal_hit: + return "partial" + return "none" + + +def _classify_finding( + finding: dict[str, Any], + root_causes: list[dict[str, Any]], + accepted: list[dict[str, Any]], + claimed: set[str], +) -> dict[str, Any]: + strengths = [(rc, match_strength(rc, finding)) for rc in root_causes] + full = [rc for rc, strength in strengths if strength == "full"] + partial = [rc for rc, strength in strengths if strength == "partial"] + + record: dict[str, Any] = { + "finding_id": finding.get("id"), + "severity": finding.get("severity"), + "root_cause_id": None, + "candidate_root_cause_ids": [], + } + if len(full) > 1: + record["classification"] = "ambiguous" + record["candidate_root_cause_ids"] = [rc["id"] for rc in full] + return record + if len(full) == 1: + root_cause_id = full[0]["id"] + record["root_cause_id"] = root_cause_id + record["classification"] = ( + "duplicate" if root_cause_id in claimed else "matched" + ) + return record + + text = finding_text(finding) + for non_finding in accepted: + if _signal_match(non_finding.get("equivalent_formulations") or [], text): + record["classification"] = "accepted" + record["accepted_non_finding_id"] = non_finding["id"] + return record + + if partial: + record["classification"] = "partial" + record["candidate_root_cause_ids"] = [rc["id"] for rc in partial] + return record + + record["classification"] = "unexpected" + return record + + +def grade(expectation: dict[str, Any] | None, result: dict[str, Any]) -> dict[str, Any]: + """Grade one valid review result against its private expectation.""" + if not expectation: + raise GradingError("grading requires a private expectation for the case") + + root_causes = list(expectation.get("material_root_causes") or []) + accepted = list(expectation.get("accepted_non_findings") or []) + findings = [item for item in result.get("findings") or [] if isinstance(item, dict)] + + claimed: set[str] = set() + records: list[dict[str, Any]] = [] + for finding in findings: + record = _classify_finding(finding, root_causes, accepted, claimed) + if record["classification"] == "matched": + claimed.add(record["root_cause_id"]) + records.append(record) + + expected_ids = [rc["id"] for rc in root_causes] + matched_ids = sorted(claimed) + missed_ids = [item for item in expected_ids if item not in claimed] + expected_verdict = expectation["expected_verdict"] + observed_verdict = result.get("verdict") + gating = [ + record + for record in records + if record["severity"] in GATING_SEVERITIES + and record["classification"] == "unexpected" + ] + + return { + "grader_version": GRADER_VERSION, + "expected_verdict": expected_verdict, + "observed_verdict": observed_verdict, + "verdict_match": expected_verdict == observed_verdict, + # A miss that also reports no gating finding is the failure mode the + # epic cares about most, so it is named rather than inferred. + "false_clean": bool( + expected_verdict == "changes_required" and observed_verdict == "clean" + ), + "false_alarm": bool( + expected_verdict == "clean" and observed_verdict == "changes_required" + ), + "expected_root_cause_ids": expected_ids, + "matched_root_cause_ids": matched_ids, + "missed_root_cause_ids": missed_ids, + "recall": (len(matched_ids) / len(expected_ids)) if expected_ids else None, + "findings": records, + "false_positive_finding_ids": [record["finding_id"] for record in gating], + "accepted_finding_ids": [ + record["finding_id"] + for record in records + if record["classification"] == "accepted" + ], + "duplicate_finding_ids": [ + record["finding_id"] + for record in records + if record["classification"] == "duplicate" + ], + "adjudication_required": [ + { + "finding_id": record["finding_id"], + "reason": record["classification"], + "candidate_root_cause_ids": record["candidate_root_cause_ids"], + } + for record in records + if record["classification"] in {"ambiguous", "partial"} + ], + } diff --git a/review-suite/scripts/evals/protocol.py b/review-suite/scripts/evals/protocol.py new file mode 100644 index 0000000..dcdd301 --- /dev/null +++ b/review-suite/scripts/evals/protocol.py @@ -0,0 +1,385 @@ +#!/usr/bin/env python3 +"""Versioned stdin/stdout replay protocol for the review suite. + +One evaluation attempt is one fresh executor process. The runner writes a +single result-blind JSON request to stdin and reads a single JSON response +from stdout. Nothing about the expected outcome may appear in the request: +that blindness is a contract, and `audit_request` enforces it structurally +and textually rather than by convention. + +The response `outcome` field separates a valid review from a runtime problem +so that a spawn failure, timeout, crash, oversized reply, malformed reply, or +protocol mismatch can never be scored as a clean review. +""" + +from __future__ import annotations + +import hashlib +import importlib.util +import json +import re +import sys +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +PROTOCOL_VERSION = "1.0" + +SCRIPTS_DIR = Path(__file__).resolve().parents[1] +REVIEW_SUITE = SCRIPTS_DIR.parent +REPOSITORY_ROOT = REVIEW_SUITE.parent +EVAL_CONTRACTS = REVIEW_SUITE / "evals" / "contracts" + +#: Every terminal classification of one attempt. The first six are evaluation +#: failures: they describe the harness or the runtime, never review quality. +ATTEMPT_STATUSES = ( + "spawn_failure", + "timeout", + "runtime_failure", + "output_too_large", + "malformed_output", + "protocol_mismatch", + "blocked", + "review_result", +) + +#: Statuses that report an evaluation failure rather than a review outcome. +EVALUATION_FAILURE_STATUSES = ATTEMPT_STATUSES[:6] + +#: Statuses in which the executor returned a valid, candidate-bound review. +VALID_OUTCOME_STATUSES = ("blocked", "review_result") + +#: Only a merge verdict is graded against expectations; a `blocked` review +#: refuses to give one, so there is nothing to score it against. +GRADABLE_STATUSES = ("review_result",) + +#: Request keys the protocol permits, at the top level and inside `run`. +#: `audit_request` rejects anything else so a private expectation object +#: cannot ride along inside an unnoticed extra key. +REQUEST_KEYS = frozenset( + { + "protocol_version", + "target_skill", + "target_skill_digest", + "skill_prompt", + "contract_documents", + "instructions", + "packet", + "run", + } +) +RUN_KEYS = frozenset( + { + "case_ref", + "run_number", + "suite_commit", + "corpus_version", + "candidate", + "started_at", + } +) +CANDIDATE_KEYS = frozenset({"head_sha", "comparison_base_sha"}) + + +def _load_validator(): + """Load the canonical review-suite validator without third-party deps.""" + spec = importlib.util.spec_from_file_location( + "review_suite_validate", SCRIPTS_DIR / "validate.py" + ) + if spec is None or spec.loader is None: + raise RuntimeError(f"cannot load {SCRIPTS_DIR / 'validate.py'}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +VALIDATOR = _load_validator() + + +def load_eval_schema(name: str) -> dict[str, Any]: + return json.loads((EVAL_CONTRACTS / name).read_text()) + + +def validate_against(name: str, document: Any) -> list[str]: + """Validate a document with the review suite's schema-subset validator.""" + return VALIDATOR.validate_schema(document, load_eval_schema(name)) + + +def case_ref(case_id: str) -> str: + """Return the opaque per-case reference used inside a request. + + The real case identifier never reaches the executor. Even a badly named + case therefore cannot leak its name through the payload; `audit_corpus` + separately rejects outcome-revealing identifiers at the source. + """ + digest = hashlib.sha256(case_id.encode("utf-8")).hexdigest() + return f"c-{digest[:8]}" + + +def prompt_digest(skill_prompt: str) -> str: + """Pin the exact target-skill text evaluated by one attempt.""" + return hashlib.sha256(skill_prompt.encode("utf-8")).hexdigest()[:16] + + +def build_request( + *, + case_id: str, + target_skill: str, + skill_prompt: str, + contract_documents: dict[str, str], + instructions: str, + packet: dict[str, Any], + run_number: int, + suite_commit: str, + corpus_version: str, + started_at: str, +) -> dict[str, Any]: + """Build one result-blind request for one fresh executor process.""" + candidate = packet.get("candidate") + if not isinstance(candidate, dict): + raise ValueError(f"{case_id}: packet has no candidate object") + identity = { + field: candidate[field] + for field in sorted(CANDIDATE_KEYS) + if field in candidate + } + if set(identity) != CANDIDATE_KEYS: + raise ValueError(f"{case_id}: packet candidate lacks complete identity") + return { + "protocol_version": PROTOCOL_VERSION, + "target_skill": target_skill, + "target_skill_digest": prompt_digest(skill_prompt), + "skill_prompt": skill_prompt, + "contract_documents": dict(contract_documents), + "instructions": instructions, + "packet": packet, + "run": { + "case_ref": case_ref(case_id), + "run_number": run_number, + "suite_commit": suite_commit, + "corpus_version": corpus_version, + "candidate": identity, + "started_at": started_at, + }, + } + + +#: Provenance fields whose free text could carry restricted material. +PRIVATE_PROVENANCE_FIELDS = ("retention_authority", "sanitization", "notes") + + +def blind_strings(expectation: dict[str, Any], provenance: dict[str, Any]) -> list[str]: + """Return every private string that must never reach an executor. + + Three deliberate exclusions keep the check meaningful rather than vacuous: + + - `requirement` normally restates the reviewer-visible acceptance criterion + and `surface` is a reviewer-visible code location, so banning them would + ban the packet itself; + - `expected_verdict`, `severity`, and `origin` are closed public + vocabularies that the bundled review contracts spell out in full, so + banning those words would ban the contracts; those fields stay private + structurally instead, because the expectation file is never part of a + request and the permitted request keys are exact. + + Everything that identifies *which* outcome this case expects is banned. + """ + strings: list[str] = [] + for root_cause in expectation.get("material_root_causes", []): + strings.append(str(root_cause.get("id", ""))) + strings.append(str(root_cause.get("consequence", ""))) + strings.append(str(root_cause.get("trigger", ""))) + strings.extend( + str(item) for item in root_cause.get("equivalent_formulations", []) + ) + for non_finding in expectation.get("accepted_non_findings", []): + strings.append(str(non_finding.get("id", ""))) + strings.append(str(non_finding.get("description", ""))) + strings.extend( + str(item) for item in non_finding.get("equivalent_formulations", []) + ) + strings.extend( + str(provenance.get(field, "")) for field in PRIVATE_PROVENANCE_FIELDS + ) + return [item for item in strings if item] + + +def _string_leaves(value: Any) -> Iterator[str]: + """Yield every string in a payload, including object keys.""" + if isinstance(value, str): + yield value + elif isinstance(value, dict): + for key, child in value.items(): + yield str(key) + yield from _string_leaves(child) + elif isinstance(value, list): + for item in value: + yield from _string_leaves(item) + + +def _collapse(text: str) -> str: + """Fold case and whitespace so a re-wrapped leak still matches.""" + return re.sub(r"\s+", " ", text).strip().lower() + + +def _contains(haystack: list[str], needle: str) -> bool: + collapsed = _collapse(needle) + return bool(collapsed) and any(collapsed in leaf for leaf in haystack) + + +def audit_request( + request: dict[str, Any], + *, + case_id: str, + expectation: dict[str, Any], + provenance: dict[str, Any], +) -> list[str]: + """Prove one complete request payload cannot reveal its expected outcome.""" + errors = [ + f"schema: {error}" + for error in validate_against("executor-request.schema.json", request) + ] + + # Structural containment: an unexpected key is the easiest way for grader + # data to leak, so the permitted key sets are exact rather than minimal. + extra_top = sorted(set(request) - REQUEST_KEYS) + if extra_top: + errors.append("payload carries unpermitted key(s): " + ", ".join(extra_top)) + run = request.get("run") + if isinstance(run, dict): + extra_run = sorted(set(run) - RUN_KEYS) + if extra_run: + errors.append( + "payload run carries unpermitted key(s): " + ", ".join(extra_run) + ) + candidate = run.get("candidate") + if isinstance(candidate, dict): + extra_candidate = sorted(set(candidate) - CANDIDATE_KEYS) + if extra_candidate: + errors.append( + "payload run candidate carries unpermitted key(s): " + + ", ".join(extra_candidate) + ) + if run.get("case_ref") == case_id: + errors.append("payload run case_ref exposes the case identifier") + if run.get("case_ref") != case_ref(case_id): + errors.append("payload run case_ref is not the opaque case reference") + + # Textual containment is checked against the payload's real string values, + # not its JSON serialization. `json.dumps` escapes every non-ASCII + # character to `\uXXXX`, newlines to `\n`, and quotes to `\"`, so + # searching the serialized form silently misses any private string + # containing an em dash, a curly quote, a line break, or a quotation + # mark - exactly the characters human-authored expectation prose uses. + haystack = [_collapse(leaf) for leaf in _string_leaves(request)] + if _contains(haystack, case_id): + errors.append(f"payload contains the case identifier {case_id!r}") + for blind in blind_strings(expectation, provenance): + if _contains(haystack, blind): + errors.append(f"payload contains private expectation text {blind!r}") + return errors + + +def _numeric_usage_errors(response: dict[str, Any]) -> list[str]: + """Check the numeric usage fields the schema subset cannot type-check.""" + usage = response.get("usage") + if usage is None: + return [] + if not isinstance(usage, dict): + return ["$.usage: expected object"] + errors = [] + for field, value in usage.items(): + if isinstance(value, bool) or not isinstance(value, (int, float)): + errors.append(f"$.usage.{field}: expected a number") + return errors + + +def classify_response( + packet: dict[str, Any], stdout: str +) -> tuple[str, dict[str, Any] | None, str]: + """Classify one executor reply into an attempt status. + + Returns `(status, response, detail)`. `response` is present only when the + reply parsed as a JSON object, so callers can still record executor + identity and usage from a reply that failed a later check. + """ + try: + response = json.loads(stdout) + except json.JSONDecodeError as error: + return "malformed_output", None, f"stdout is not JSON: {error}" + if not isinstance(response, dict): + return "malformed_output", None, "stdout is not one JSON object" + + version = response.get("protocol_version") + if version != PROTOCOL_VERSION: + return ( + "protocol_mismatch", + response, + f"expected protocol {PROTOCOL_VERSION}, got {version!r}", + ) + + errors = validate_against("executor-response.schema.json", response) + errors.extend(_numeric_usage_errors(response)) + if errors: + return "malformed_output", response, "; ".join(errors) + + outcome = response["outcome"] + if outcome == "runtime_failure": + failure = response.get("failure") + if not failure: + return ( + "malformed_output", + response, + "runtime_failure requires a failure reason", + ) + return "runtime_failure", response, failure["reason"] + + result = response.get("result") + if not isinstance(result, dict): + return "malformed_output", response, f"{outcome} requires a review result" + + verdict = result.get("verdict") + allowed = ( + {"clean", "changes_required"} if outcome == "review_result" else {"blocked"} + ) + if verdict not in allowed: + return ( + "malformed_output", + response, + f"{outcome} cannot carry verdict {verdict!r}", + ) + + # Judge the reply, never the packet. `corpus.load_case` already established + # and asserted each packet's validity, so a packet defect at this point is + # a deliberate property of the case, not news about the executor. Billing it + # to the executor would classify a reviewer that wrongly issues a merge + # verdict on incomplete evidence as an evaluation failure, which drops the + # one behaviour a `packet_valid: false` case exists to measure. + pair_errors = [ + error + for error in VALIDATOR.validate_pair(packet, result) + if not error.startswith("packet: ") + ] + if pair_errors: + return "malformed_output", response, "; ".join(pair_errors) + + return ("blocked" if outcome == "blocked" else "review_result"), response, "" + + +def read_request() -> dict[str, Any]: + """Read and version-check one request from stdin, for executor scripts.""" + request = json.load(sys.stdin) + if not isinstance(request, dict): + raise ValueError("executor request must be one JSON object") + version = request.get("protocol_version") + if version != PROTOCOL_VERSION: + raise ValueError( + f"executor supports protocol {PROTOCOL_VERSION}, got {version!r}" + ) + return request + + +def write_response(response: dict[str, Any]) -> None: + """Write exactly one JSON response object to stdout.""" + json.dump(response, sys.stdout, sort_keys=True) + sys.stdout.write("\n") diff --git a/review-suite/scripts/evals/report.py b/review-suite/scripts/evals/report.py new file mode 100644 index 0000000..77de833 --- /dev/null +++ b/review-suite/scripts/evals/report.py @@ -0,0 +1,229 @@ +#!/usr/bin/env python3 +"""Per-attempt records and the aggregate report for repeated replay runs. + +The aggregate report is descriptive only. It deliberately encodes no success +threshold and returns no pass/fail judgement: #59 owns interpretation and any +future v2 gate. Metrics are reported with their denominators so a small run +count cannot be mistaken for a precise capability measurement. +""" + +from __future__ import annotations + +import statistics +from typing import Any + +from . import grader, protocol + +REPORT_VERSION = "1.0" + + +def _rate(numerator: int, denominator: int) -> float | None: + return (numerator / denominator) if denominator else None + + +def _numbers(values: list[Any]) -> list[float]: + return [ + float(value) + for value in values + if not isinstance(value, bool) and isinstance(value, (int, float)) + ] + + +def _latency(durations: list[float]) -> dict[str, Any]: + if not durations: + return {"count": 0} + ordered = sorted(durations) + return { + "count": len(ordered), + "mean_seconds": statistics.fmean(ordered), + "p50_seconds": statistics.median(ordered), + "min_seconds": ordered[0], + "max_seconds": ordered[-1], + } + + +def _usage(attempts: list[dict[str, Any]]) -> dict[str, Any]: + """Aggregate whatever usage the executor reported, and say what is absent.""" + reported = [attempt.get("usage") or {} for attempt in attempts] + totals: dict[str, Any] = {} + for field in ("input_tokens", "output_tokens", "cost_usd"): + values = _numbers([usage.get(field) for usage in reported]) + totals[f"total_{field}"] = sum(values) if values else None + totals[f"reporting_attempts_{field}"] = len(values) + totals["available"] = any( + totals[f"reporting_attempts_{field}"] + for field in ("input_tokens", "output_tokens", "cost_usd") + ) + return totals + + +def _mean_of(values: list[Any]) -> float | None: + present = [value for value in values if value is not None] + return statistics.fmean(present) if present else None + + +def _modal_share(values: list[Any]) -> float | None: + """Return the share of attempts holding the most common value.""" + if not values: + return None + counts: dict[Any, int] = {} + for value in values: + counts[value] = counts.get(value, 0) + 1 + return max(counts.values()) / len(values) + + +def _case_summary(case_id: str, attempts: list[dict[str, Any]]) -> dict[str, Any]: + graded = [a for a in attempts if a["status"] in protocol.GRADABLE_STATUSES] + grades = [a["grade"] for a in graded if a.get("grade")] + recalls = [g["recall"] for g in grades if g["recall"] is not None] + + # Stability spans every attempt that produced a valid review, including a + # `blocked` one. Refusing a verdict on one run and issuing one on the next + # is among the most consequential instabilities a reviewer can show, so it + # must not be excluded just because a blocked review is not graded. + answered = [a for a in attempts if a["status"] in protocol.VALID_OUTCOME_STATUSES] + # A blocked attempt is ungraded, and `None` keeps it distinguishable from a + # graded attempt that matched nothing. Collapsing both to the empty set + # would report a run that refused a verdict and a run that answered and + # found nothing as being in perfect agreement. + matched_sets = [ + tuple(sorted(a["grade"]["matched_root_cause_ids"])) if a.get("grade") else None + for a in answered + ] + + union: set[str] = set() + intersection: set[str] | None = None + for grade_record in grades: + found = set(grade_record["matched_root_cause_ids"]) + union |= found + intersection = found if intersection is None else (intersection & found) + expected = grades[0]["expected_root_cause_ids"] if grades else [] + + return { + "case_id": case_id, + "attempts": len(attempts), + "graded_attempts": len(grades), + "expected_root_cause_ids": expected, + "mean_recall": statistics.fmean(recalls) if recalls else None, + "union_root_cause_ids": sorted(union), + "intersection_root_cause_ids": sorted(intersection or set()), + "verdict_stability": _modal_share([a["verdict"] for a in answered]), + "finding_stability": _modal_share(matched_sets), + "stability_denominator": len(answered), + "false_clean_attempts": sum(1 for g in grades if g["false_clean"]), + "false_alarm_attempts": sum(1 for g in grades if g["false_alarm"]), + "false_positive_attempts": sum( + 1 for g in grades if g["false_positive_finding_ids"] + ), + "statuses": { + status: sum(1 for a in attempts if a["status"] == status) + for status in protocol.ATTEMPT_STATUSES + if any(a["status"] == status for a in attempts) + }, + } + + +def aggregate( + attempts: list[dict[str, Any]], *, configuration: dict[str, Any] +) -> dict[str, Any]: + """Build the machine-readable aggregate report for one evaluation run.""" + by_case: dict[str, list[dict[str, Any]]] = {} + for attempt in attempts: + by_case.setdefault(attempt["case_id"], []).append(attempt) + + graded = [a for a in attempts if a["status"] in protocol.GRADABLE_STATUSES] + grades = [a["grade"] for a in graded if a.get("grade")] + recalls = [g["recall"] for g in grades if g["recall"] is not None] + gating_expected = [g for g in grades if g["expected_root_cause_ids"]] + clean_expected = [g for g in grades if g["expected_verdict"] == "clean"] + + per_case = [ + _case_summary(case_id, items) for case_id, items in sorted(by_case.items()) + ] + unique_contribution = [ + { + "case_id": summary["case_id"], + "only_some_attempts": sorted( + set(summary["union_root_cause_ids"]) + - set(summary["intersection_root_cause_ids"]) + ), + } + for summary in per_case + if set(summary["union_root_cause_ids"]) + != set(summary["intersection_root_cause_ids"]) + ] + + simulation = any(attempt["simulation"] for attempt in attempts) + failures = { + f"{status}_rate": _rate( + sum(1 for a in attempts if a["status"] == status), len(attempts) + ) + for status in protocol.ATTEMPT_STATUSES + if status != "review_result" + } + + return { + "report_version": REPORT_VERSION, + "protocol_version": protocol.PROTOCOL_VERSION, + "grader_version": grader.GRADER_VERSION, + "configuration": configuration, + "simulation": simulation, + # A simulated run describes the harness, not a model. It can never be + # promoted into a behavioural baseline. + "baseline_eligible": not simulation, + "attempts": len(attempts), + "graded_attempts": len(graded), + "quality": { + "material_finding_recall": statistics.fmean(recalls) if recalls else None, + "recall_attempts": len(recalls), + "false_clean_rate": _rate( + sum(1 for g in gating_expected if g["false_clean"]), + len(gating_expected), + ), + "false_clean_denominator": len(gating_expected), + "false_positive_rate": _rate( + sum(1 for g in grades if g["false_positive_finding_ids"]), len(grades) + ), + "false_positive_denominator": len(grades), + "false_alarm_rate": _rate( + sum(1 for g in clean_expected if g["false_alarm"]), len(clean_expected) + ), + "false_alarm_denominator": len(clean_expected), + "unique_finding_contribution": unique_contribution, + }, + "stability": { + # Averaged over cases: pooling verdicts across unlike cases would + # report disagreement between cases as instability within one. + "mean_verdict_stability": _mean_of( + [summary["verdict_stability"] for summary in per_case] + ), + "mean_finding_stability": _mean_of( + [summary["finding_stability"] for summary in per_case] + ), + # Published beside every value, as the quality rates are, so a + # consumer can see how many attempts each figure rests on. + "stability_denominator": sum( + summary["stability_denominator"] for summary in per_case + ), + "per_case_verdict_stability": { + summary["case_id"]: summary["verdict_stability"] for summary in per_case + }, + "per_case_finding_stability": { + summary["case_id"]: summary["finding_stability"] for summary in per_case + }, + "per_case_stability_denominator": { + summary["case_id"]: summary["stability_denominator"] + for summary in per_case + }, + }, + "failures": failures, + "latency": _latency([a["duration_seconds"] for a in attempts]), + "usage": _usage(attempts), + "adjudication_required": [ + {"case_id": a["case_id"], "run_number": a["run_number"], **item} + for a in graded + if a.get("grade") + for item in a["grade"]["adjudication_required"] + ], + "per_case": per_case, + } diff --git a/review-suite/scripts/evals/runner.py b/review-suite/scripts/evals/runner.py new file mode 100644 index 0000000..d7d0f42 --- /dev/null +++ b/review-suite/scripts/evals/runner.py @@ -0,0 +1,460 @@ +#!/usr/bin/env python3 +"""Run repeated, result-blind review replays through a fresh process each time. + +Every attempt starts a new executor process, so no state leaks between runs of +the same case. The runner owns the failure taxonomy, the timeout and +output-size limits, and the artifact directory; the executor owns nothing but +the protocol. + +Exit status reports evaluation integrity, never review quality: + + 0 every attempt produced a valid review or a valid blocked result + 1 at least one attempt was an evaluation failure + 2 the configuration or the corpus was rejected before any launch + +Usage: + python3 runner.py --executor "python3 path/to/executor.py" --runs 3 +""" + +from __future__ import annotations + +import argparse +import json +import shlex +import subprocess +import sys +import time +from collections.abc import Mapping, Sequence +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +if __package__ in (None, ""): # direct `python3 runner.py` invocation + sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + from evals import corpus, grader, protocol, report +else: + from . import corpus, grader, protocol, report + +DEFAULT_TIMEOUT_SECONDS = 900.0 +DEFAULT_MAX_OUTPUT_BYTES = 4_000_000 +FIXTURE_EXECUTOR = Path(__file__).with_name("fixture_executor.py") +TARGET_SKILL_ROOT = protocol.REPOSITORY_ROOT / "skills" + + +class ConfigurationError(ValueError): + """Raised for an unusable executor command, limit, or output location.""" + + +def _now() -> str: + return datetime.now(timezone.utc).isoformat(timespec="seconds") + + +def _skill_documents(skill: str) -> dict[str, str]: + """One skill's reviewer-visible Markdown, namespaced by skill name. + + The bundled `references/review-suite/` mirror is excluded: `contract_documents` + already supplies those files from their canonical location. + """ + root = TARGET_SKILL_ROOT / skill + skill_md = root / "SKILL.md" + if not skill_md.is_file(): + raise ConfigurationError(f"missing target skill prompt {skill_md}") + documents = {f"{skill}/SKILL.md": skill_md.read_text()} + references = root / "references" + if references.is_dir(): + for path in sorted(references.rglob("*.md")): + relative = path.relative_to(root) + if "review-suite" in relative.parts: + continue + documents[f"{skill}/{relative.as_posix()}"] = path.read_text() + return documents + + +def target_skill_documents( + target_skill: str, dependencies: Sequence[str] = () +) -> dict[str, str]: + """Return the target skill's declared dependency closure as text. + + A skill's `SKILL.md` is not the whole skill, in two distinct ways. + + It links references that it instructs the reviewer to read - for + `review-code-change`, `references/orchestration-protocol.md`, which owns the + lens decision table, deduplication, and verdict aggregation. + + It also declares sibling skills it cannot work without: `review-code-change` + requires `review-solution-simplicity`, `review-correctness`, and + `review-code-simplicity` to be available and readable, and mandates an + aggregate `blocked` result naming any that are missing. An executor is told + to reason only from what it is given, so a target whose contract requires + siblings can only be evaluated when those siblings are in the payload. + Omitting them inverts the measurement: a reviewer that correctly refuses for + missing dependencies is scored wrong on every case that expects a merge + verdict, so recall moves the wrong way as the reviewer becomes more + compliant. + + The corpus declares the closure so a target can be swapped without changing + this code. Which target a scored corpus should measure, and the cost envelope + that follows from its closure, are corpus-composition decisions. + """ + documents = _skill_documents(target_skill) + for dependency in dependencies: + if dependency == target_skill: + raise ConfigurationError(f"{target_skill} declares itself as a dependency") + documents.update(_skill_documents(dependency)) + return documents + + +def render_skill_prompt(target_skill: str, documents: Mapping[str, str]) -> str: + """Lay out a skill closure as one prompt, target first and clearly named. + + Every section is labelled, including the target's own `SKILL.md`: once a + payload carries several skills, a reviewer has to be able to tell which one + it is being asked to execute and which are its dependencies. + """ + remaining = dict(documents) + lead = f"{target_skill}/SKILL.md" + sections = [f"## Target skill: {lead}\n\n{remaining.pop(lead)}"] + for name, text in sorted(remaining.items()): + sections.append(f"\n\n## Skill reference: {name}\n\n{text}") + return "".join(sections) + + +def target_skill_prompt(target_skill: str, dependencies: Sequence[str] = ()) -> str: + """Render the target skill and its whole declared closure as one prompt. + + `prompt_digest` hashes this string, so the recorded `target_skill_digest` + changes whenever any part of the evaluated skill text changes - which is the + whole point of pinning it across baseline strata. + """ + return render_skill_prompt( + target_skill, target_skill_documents(target_skill, dependencies) + ) + + +def contract_documents() -> dict[str, str]: + """Reviewer-visible contract text every attempt receives verbatim.""" + return { + "CONTRACT.md": (protocol.REVIEW_SUITE / "CONTRACT.md").read_text(), + "review-packet.schema.json": ( + protocol.REVIEW_SUITE / "contracts" / "review-packet.schema.json" + ).read_text(), + "review-result.schema.json": ( + protocol.REVIEW_SUITE / "contracts" / "review-result.schema.json" + ).read_text(), + } + + +def suite_commit() -> str: + """Return the exact suite commit, or an explicit unavailable marker.""" + completed = subprocess.run( + ["git", "-C", str(protocol.REPOSITORY_ROOT), "rev-parse", "HEAD"], + capture_output=True, + text=True, + check=False, + ) + if completed.returncode: + return "unavailable" + return completed.stdout.strip() or "unavailable" + + +def is_bundled_fixture_executor(command: list[str]) -> bool: + """Detect the deterministic executor regardless of what it self-reports.""" + return any(Path(part).name == FIXTURE_EXECUTOR.name for part in command) + + +def run_attempt( + command: list[str], + request: dict[str, Any], + case: corpus.Case, + *, + timeout: float, + max_output_bytes: int, + forced_simulation: bool, +) -> tuple[dict[str, Any], dict[str, Any] | None, str, str]: + """Execute one fresh process and classify exactly what came back.""" + payload = json.dumps(request) + started_at = _now() + start = time.monotonic() + stdout = "" + stderr = "" + try: + completed = subprocess.run( + command, + input=payload, + capture_output=True, + text=True, + check=False, + timeout=timeout, + ) + stdout, stderr = completed.stdout, completed.stderr + returncode: int | None = completed.returncode + if returncode: + status, response, detail = ( + "runtime_failure", + None, + f"executor exited {returncode}: {stderr.strip()[-500:]}", + ) + elif len(stdout.encode("utf-8")) > max_output_bytes: + status, response, detail = ( + "output_too_large", + None, + f"stdout exceeded {max_output_bytes} bytes", + ) + else: + status, response, detail = protocol.classify_response(case.packet, stdout) + except subprocess.TimeoutExpired: + returncode = None + status, response, detail = ( + "timeout", + None, + f"executor exceeded {timeout} seconds", + ) + except OSError as error: + returncode = None + status, response, detail = ("spawn_failure", None, str(error)) + + duration = time.monotonic() - start + executor_identity = (response or {}).get("executor") or {} + attempt = { + "case_id": case.case_id, + "case_ref": request["run"]["case_ref"], + "run_number": request["run"]["run_number"], + "status": status, + "detail": detail, + "simulation": forced_simulation or bool((response or {}).get("simulation")), + "protocol_version": protocol.PROTOCOL_VERSION, + "suite_commit": request["run"]["suite_commit"], + "corpus_version": request["run"]["corpus_version"], + "target_skill_digest": request["target_skill_digest"], + "candidate": request["run"]["candidate"], + "executor": executor_identity, + "usage": (response or {}).get("usage"), + "verdict": ((response or {}).get("result") or {}).get("verdict"), + "returncode": returncode, + "started_at": started_at, + "finished_at": _now(), + "duration_seconds": duration, + "grade": None, + } + return attempt, response, stdout, stderr + + +def _write_artifacts( + artifact_dir: Path, attempt: dict[str, Any], stdout: str, stderr: str +) -> None: + name = f"{attempt['case_id']}.run-{attempt['run_number']}" + artifact_dir.mkdir(parents=True, exist_ok=True) + (artifact_dir / f"{name}.stdout.json").write_text(stdout) + if stderr: + (artifact_dir / f"{name}.stderr.txt").write_text(stderr) + + +def evaluate( + command: list[str], + *, + corpus_root: Path | None, + runs: int, + timeout: float, + max_output_bytes: int, + artifact_dir: Path | None, +) -> tuple[list[dict[str, Any]], dict[str, Any]]: + """Audit the corpus, then replay every case the configured number of times.""" + loaded = corpus.load_corpus(corpus_root) + if loaded.grader_version != grader.GRADER_VERSION: + raise corpus.CorpusError( + f"corpus grader_version {loaded.grader_version!r} does not match the " + f"shipped grader {grader.GRADER_VERSION!r}" + ) + # Keep the closure's exact membership, not just its digest: a digest proves + # two runs sent the same text but cannot say what that text was, and a + # baseline stratum has to be able to state which skills it evaluated. + closure = target_skill_documents( + loaded.target_skill, loaded.target_skill_dependencies + ) + skill_prompt = render_skill_prompt(loaded.target_skill, closure) + documents = contract_documents() + commit = suite_commit() + forced_simulation = is_bundled_fixture_executor(command) + + def build(case: corpus.Case, run_number: int) -> dict[str, Any]: + return protocol.build_request( + case_id=case.case_id, + target_skill=loaded.target_skill, + skill_prompt=skill_prompt, + contract_documents=documents, + instructions=case.instructions, + packet=case.packet, + run_number=run_number, + suite_commit=commit, + corpus_version=loaded.corpus_version, + started_at=_now(), + ) + + def refuse_if_contaminated(case: corpus.Case, request: dict[str, Any]) -> None: + contamination = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + if contamination: + raise corpus.CorpusError( + f"{case.case_id}: contaminated request: " + "; ".join(contamination) + ) + + # Audit every case before launching anything. A per-request check alone + # would let a contaminated last case bill a real review for every earlier + # case and then discard the whole run, which contradicts the rule that a + # contaminated case fails before executor launch. Nothing `audit_request` + # inspects varies by run, so one request per case settles it. + for case in loaded.cases: + refuse_if_contaminated(case, build(case, 1)) + + attempts: list[dict[str, Any]] = [] + for case in loaded.cases: + for run_number in range(1, runs + 1): + request = build(case, run_number) + # Kept as defence in depth: the pre-flight pass above is what + # guarantees the ordering, this re-check guarantees that the exact + # payload handed to a process was audited. + refuse_if_contaminated(case, request) + + attempt, response, stdout, stderr = run_attempt( + command, + request, + case, + timeout=timeout, + max_output_bytes=max_output_bytes, + forced_simulation=forced_simulation, + ) + attempt["target_skill"] = loaded.target_skill + attempt["target_skill_dependencies"] = list( + loaded.target_skill_dependencies + ) + if attempt["status"] in protocol.GRADABLE_STATUSES: + attempt["grade"] = grader.grade( + case.expectation, (response or {})["result"] + ) + if artifact_dir is not None: + _write_artifacts(artifact_dir, attempt, stdout, stderr) + attempts.append(attempt) + + configuration = { + "executor": shlex.join(command), + "corpus_version": loaded.corpus_version, + "grader_version": loaded.grader_version, + "target_skill": loaded.target_skill, + "target_skill_dependencies": list(loaded.target_skill_dependencies), + "target_skill_documents": sorted(closure), + "target_skill_digest": protocol.prompt_digest(skill_prompt), + "suite_commit": commit, + "runs_per_case": runs, + "cases": len(loaded.cases), + "timeout_seconds": timeout, + "max_output_bytes": max_output_bytes, + "artifacts_retained": artifact_dir is not None, + } + return attempts, configuration + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--executor", + required=True, + help="Fresh-process command receiving one result-blind JSON request on stdin", + ) + parser.add_argument("--corpus", type=Path, default=None) + parser.add_argument("--runs", type=int, default=1) + parser.add_argument("--timeout", type=float, default=DEFAULT_TIMEOUT_SECONDS) + parser.add_argument( + "--max-output-bytes", type=int, default=DEFAULT_MAX_OUTPUT_BYTES + ) + parser.add_argument( + "--artifact-dir", + type=Path, + default=None, + help="Opt-in directory for captured raw executor output", + ) + parser.add_argument("--attempts-out", type=Path, default=None) + parser.add_argument("--report-out", type=Path, default=None) + parser.add_argument( + "--baseline-report", + type=Path, + default=None, + help="Write a frozen baseline report; refused for a simulated run", + ) + return parser + + +def _validated_command(raw: str) -> list[str]: + command = shlex.split(raw) + if not command: + raise ConfigurationError("--executor is empty") + return command + + +def _check_limits(args: argparse.Namespace) -> None: + if args.runs < 1: + raise ConfigurationError("--runs must be at least 1") + if args.timeout <= 0: + raise ConfigurationError("--timeout must be greater than 0") + if args.max_output_bytes < 1: + raise ConfigurationError("--max-output-bytes must be at least 1") + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + try: + command = _validated_command(args.executor) + _check_limits(args) + attempts, configuration = evaluate( + command, + corpus_root=args.corpus, + runs=args.runs, + timeout=args.timeout, + max_output_bytes=args.max_output_bytes, + artifact_dir=args.artifact_dir, + ) + except (ConfigurationError, corpus.CorpusError, grader.GradingError) as error: + print(f"evaluation rejected: {error}", file=sys.stderr) + return 2 + + aggregate = report.aggregate(attempts, configuration=configuration) + + if args.attempts_out: + args.attempts_out.parent.mkdir(parents=True, exist_ok=True) + args.attempts_out.write_text( + "".join(json.dumps(attempt, sort_keys=True) + "\n" for attempt in attempts) + ) + if args.report_out: + args.report_out.parent.mkdir(parents=True, exist_ok=True) + args.report_out.write_text( + json.dumps(aggregate, indent=2, sort_keys=True) + "\n" + ) + if args.baseline_report: + if not aggregate["baseline_eligible"]: + print( + "baseline refused: a simulated run cannot produce a baseline report", + file=sys.stderr, + ) + return 2 + args.baseline_report.parent.mkdir(parents=True, exist_ok=True) + args.baseline_report.write_text( + json.dumps(aggregate, indent=2, sort_keys=True) + "\n" + ) + + print(json.dumps(aggregate, indent=2, sort_keys=True)) + evaluation_failures = [ + f"{attempt['case_id']} run {attempt['run_number']}: " + f"{attempt['status']}: {attempt['detail']}" + for attempt in attempts + if attempt["status"] in protocol.EVALUATION_FAILURE_STATUSES + ] + for failure in evaluation_failures: + print(failure, file=sys.stderr) + return 1 if evaluation_failures else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/review-suite/scripts/tests/test_eval_commands.py b/review-suite/scripts/tests/test_eval_commands.py new file mode 100644 index 0000000..d160801 --- /dev/null +++ b/review-suite/scripts/tests/test_eval_commands.py @@ -0,0 +1,231 @@ +"""Command-contract tests for the three review-suite evaluation entrypoints. + +These names are part of the repository's contract with #58 and with anyone +running an evaluation, so they are asserted rather than assumed. The paid path +must stay out of the ordinary quality gates. +""" + +from __future__ import annotations + +import re +import subprocess +import sys +import unittest +from pathlib import Path + +REPOSITORY_ROOT = Path(__file__).resolve().parents[3] +JUSTFILE = REPOSITORY_ROOT / "justfile" +README = REPOSITORY_ROOT / "README.md" +EVAL_README = REPOSITORY_ROOT / "review-suite" / "evals" / "README.md" + +TEST_COMMAND = "test-review-suite" +AUDIT_COMMAND = "audit-review-corpus" +EVAL_COMMAND = "eval-review-suite" + + +#: `name param1 param2: dep1 dep2` - parameters precede the colon, dependencies +#: follow it. Keeping the three apart matters: deriving dependencies by +#: re-splitting a joined string silently yields nothing for `test: test-plugins`, +#: which would make the paid-path guard below inert. +RECIPE_HEADER = re.compile(r"^([a-z][a-z0-9-]*)((?:\s+[a-z0-9-]+)*)\s*:(.*)$") + + +def recipes(text: str) -> dict[str, dict[str, object]]: + """Parse a justfile into {name: {parameters, dependencies, body}}.""" + found: dict[str, dict[str, object]] = {} + name = None + for line in text.splitlines(): + indented = line.startswith((" ", "\t")) + if ":=" in line and not indented: + name = None # a top-level assignment, not a recipe + continue + header = None if indented else RECIPE_HEADER.match(line) + if header: + name = header.group(1) + found[name] = { + "parameters": header.group(2).split(), + "dependencies": header.group(3).split(), + "body": "", + } + elif name and (indented or not line.strip()): + found[name]["body"] += line + "\n" + else: + name = None + return found + + +class JustfileContractTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.recipes = recipes(JUSTFILE.read_text()) + + def test_all_three_recipes_exist_under_their_exact_names(self): + for name in (TEST_COMMAND, AUDIT_COMMAND, EVAL_COMMAND): + self.assertIn(name, self.recipes) + + def test_the_evaluation_recipe_takes_one_executor_argument(self): + self.assertEqual(["executor"], self.recipes[EVAL_COMMAND]["parameters"]) + self.assertIn('--executor "{{executor}}"', self.recipes[EVAL_COMMAND]["body"]) + + def test_the_deterministic_recipes_take_no_argument(self): + for name in (TEST_COMMAND, AUDIT_COMMAND): + with self.subTest(recipe=name): + self.assertEqual([], self.recipes[name]["parameters"]) + + def test_the_justfile_parser_sees_real_dependencies(self): + """Guard the guard: an empty closure would make the check below inert.""" + self.assertEqual(["test-plugins"], self.recipes["test"]["dependencies"]) + self.assertEqual(["test", "lint"], self.recipes["check"]["dependencies"]) + self.assertEqual( + ["lint-py", "lint-md", "lint-skills", "validate-plugins"], + self.recipes["lint"]["dependencies"], + ) + + def test_test_runs_the_review_suite_tests(self): + self.assertIn("review-suite/scripts/tests", self.recipes["test"]["body"]) + + def _quality_gate_closure(self) -> set[str]: + reachable: set[str] = set() + frontier = ["test", "lint", "check"] + while frontier: + name = frontier.pop() + if name in reachable or name not in self.recipes: + continue + reachable.add(name) + recipe = self.recipes[name] + frontier.extend(recipe["dependencies"]) + frontier.extend( + candidate + for candidate in self.recipes + if f"just {candidate}" in recipe["body"] + ) + return reachable + + def test_the_quality_gate_closure_reaches_every_transitive_recipe(self): + """A closure that stops at the seeds would prove nothing.""" + reachable = self._quality_gate_closure() + for name in ( + "test", + "lint", + "check", + "test-plugins", + "lint-py", + "lint-md", + "lint-skills", + "validate-plugins", + ): + with self.subTest(recipe=name): + self.assertIn(name, reachable) + + def test_no_quality_gate_can_reach_the_paid_command(self): + """`test`, `lint`, and `check` must never spend money.""" + reachable = self._quality_gate_closure() + self.assertNotIn(EVAL_COMMAND, reachable) + for name in sorted(reachable): + with self.subTest(recipe=name): + recipe = self.recipes[name] + self.assertNotIn(EVAL_COMMAND, recipe["dependencies"]) + for forbidden in ( + EVAL_COMMAND, + "evals/runner.py", + "claude_executor", + ): + self.assertNotIn(forbidden, recipe["body"]) + + def test_the_recipes_point_at_the_canonical_scripts(self): + self.assertIn( + "review-suite/scripts/evals/audit_corpus.py", + self.recipes[AUDIT_COMMAND]["body"], + ) + self.assertIn( + "review-suite/scripts/evals/runner.py", self.recipes[EVAL_COMMAND]["body"] + ) + + +class DocumentationContractTests(unittest.TestCase): + """Documented prose is compared with Markdown line wrapping collapsed.""" + + @staticmethod + def unwrapped(path: Path) -> str: + return re.sub(r"\s+", " ", path.read_text()) + + def test_both_readmes_document_all_three_command_names(self): + for path in (README, EVAL_README): + text = self.unwrapped(path) + for name in (TEST_COMMAND, AUDIT_COMMAND, EVAL_COMMAND): + with self.subTest(path=path.name, command=name): + self.assertIn(f"just {name}", text) + + def test_both_readmes_show_the_evaluation_argument(self): + for path in (README, EVAL_README): + with self.subTest(path=path.name): + self.assertIn( + f"just {EVAL_COMMAND} ''", self.unwrapped(path) + ) + + def test_both_readmes_state_that_the_paid_path_is_opt_in(self): + for path in (README, EVAL_README): + with self.subTest(path=path.name): + text = self.unwrapped(path) + self.assertIn("never launches a paid runtime", text) + self.assertIn( + f"`just {EVAL_COMMAND}` is deliberately absent from `test`, " + "`lint`, and `check`", + text, + ) + + +class RecipeExecutionTests(unittest.TestCase): + """Run the recipes through `just` when it is installed. + + CI does not install `just`, so this class must skip cleanly there. An absent + executable makes `subprocess.run` raise rather than return a status, so the + probe has to catch `OSError`; checking only the return code turns a missing + `just` into an erroring `setUpClass` instead of a skip. + """ + + @classmethod + def setUpClass(cls): + try: + completed = subprocess.run( + ["just", "--version"], capture_output=True, check=False + ) + except OSError as error: + raise unittest.SkipTest(f"just is unavailable: {error}") from error + if completed.returncode != 0: + raise unittest.SkipTest("just is present but not runnable") + + def just(self, *args): + return subprocess.run( + ["just", *args], + cwd=REPOSITORY_ROOT, + capture_output=True, + text=True, + check=False, + ) + + def test_audit_review_corpus_passes(self): + completed = self.just(AUDIT_COMMAND) + self.assertEqual(0, completed.returncode, completed.stderr) + self.assertIn("corpus audit passed", completed.stdout) + + def test_eval_review_suite_requires_its_argument(self): + completed = self.just(EVAL_COMMAND) + self.assertNotEqual(0, completed.returncode) + + def test_eval_review_suite_reports_a_bad_executor(self): + completed = self.just(EVAL_COMMAND, "/nonexistent/review-executor") + self.assertEqual(1, completed.returncode, completed.stdout) + self.assertIn("spawn_failure", completed.stderr) + + def test_eval_review_suite_accepts_the_deterministic_executor(self): + completed = self.just( + EVAL_COMMAND, + f"{sys.executable} review-suite/scripts/evals/fixture_executor.py", + ) + self.assertEqual(0, completed.returncode, completed.stderr) + self.assertIn('"baseline_eligible": false', completed.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/review-suite/scripts/tests/test_eval_corpus.py b/review-suite/scripts/tests/test_eval_corpus.py new file mode 100644 index 0000000..e8f5b87 --- /dev/null +++ b/review-suite/scripts/tests/test_eval_corpus.py @@ -0,0 +1,318 @@ +"""Corpus contract, separation, and audit tests for the replay evaluator.""" + +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +import tempfile +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 audit_corpus, corpus, grader, protocol # noqa: E402 + +AUDIT_SCRIPT = SCRIPTS_DIR / "evals" / "audit_corpus.py" + + +class LayoutTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.corpus = corpus.load_corpus() + + def test_reviewer_and_private_data_live_in_separate_trees(self): + root = self.corpus.root + for case in self.corpus.cases: + with self.subTest(case=case.case_id): + self.assertTrue( + (root / "reviewer" / case.case_id / "packet.json").is_file() + ) + self.assertTrue( + ( + root / "private" / "expectations" / f"{case.case_id}.json" + ).is_file() + ) + self.assertTrue( + (root / "private" / "provenance" / f"{case.case_id}.json").is_file() + ) + + def test_no_private_file_sits_inside_the_reviewer_tree(self): + reviewer_files = { + path.name + for path in (self.corpus.root / "reviewer").rglob("*") + if path.is_file() + } + self.assertEqual({"PROMPT.md", "packet.json"}, reviewer_files) + + def test_case_ids_and_reviewer_filenames_hide_the_outcome(self): + for case in self.corpus.cases: + with self.subTest(case=case.case_id): + self.assertEqual([], corpus.revealing_tokens(case.case_id)) + for path in (self.corpus.root / "reviewer" / case.case_id).iterdir(): + self.assertEqual([], corpus.revealing_tokens(path.name)) + + def test_reviewer_prompt_names_no_verdict_or_severity(self): + self.assertEqual([], corpus.prompt_errors(self.corpus.root)) + + def test_corpus_declares_the_shipped_grader_and_protocol_versions(self): + self.assertEqual(grader.GRADER_VERSION, self.corpus.grader_version) + index = json.loads((self.corpus.root / "corpus.json").read_text()) + 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) + + def test_the_corpus_covers_every_grading_situation_it_must_prove(self): + verdicts = {case.expectation["expected_verdict"] for case in self.corpus.cases} + self.assertEqual({"changes_required", "clean", "blocked"}, verdicts) + self.assertTrue( + any(case.expectation["accepted_non_findings"] for case in self.corpus.cases) + ) + self.assertTrue( + any( + len(case.expectation["material_root_causes"]) > 1 + for case in self.corpus.cases + ) + ) + + +class PackagingBoundaryTests(unittest.TestCase): + """A distributed skill bundle must never carry grading evidence.""" + + def test_no_skill_bundles_any_corpus_expectation_or_provenance(self): + private_names = { + path.name for path in (corpus.DEFAULT_CORPUS / "private").rglob("*.json") + } + self.assertTrue(private_names) + bundled = { + path.name + for path in (SCRIPTS_DIR.parents[1] / "skills").rglob("*") + if path.is_file() + } + self.assertEqual(set(), private_names & bundled) + + def test_no_skill_bundles_the_evaluator_or_its_reviewer_artifacts(self): + skills = SCRIPTS_DIR.parents[1] / "skills" + for name in ("runner.py", "audit_corpus.py", "corpus.json"): + with self.subTest(name=name): + self.assertEqual([], list(skills.rglob(f"**/review-suite/{name}"))) + + +class MutatedCorpusTests(unittest.TestCase): + """Every check must actually fail on a corpus that violates it.""" + + def setUp(self): + self.temp = Path(tempfile.mkdtemp()) + self.root = self.temp / "corpus" + shutil.copytree(corpus.DEFAULT_CORPUS, self.root) + self.addCleanup(shutil.rmtree, self.temp, ignore_errors=True) + + def _index(self): + return json.loads((self.root / "corpus.json").read_text()) + + def _write_index(self, index): + (self.root / "corpus.json").write_text(json.dumps(index, indent=2)) + + def _expectation_path(self, case_id): + return self.root / "private" / "expectations" / f"{case_id}.json" + + def _load_expectation(self, case_id): + return json.loads(self._expectation_path(case_id).read_text()) + + def _write_expectation(self, case_id, document): + self._expectation_path(case_id).write_text(json.dumps(document, indent=2)) + + def assertAuditFails(self, fragment): + errors = audit_corpus.audit(self.root) + self.assertTrue(errors, "expected the audit to reject this corpus") + self.assertTrue( + any(fragment in error for error in errors), + f"no error mentioned {fragment!r}: {errors}", + ) + + def test_missing_expectation_fails_before_any_launch(self): + case_id = self._index()["cases"][0] + self._expectation_path(case_id).unlink() + self.assertAuditFails("missing") + + def test_missing_provenance_fails(self): + case_id = self._index()["cases"][0] + (self.root / "private" / "provenance" / f"{case_id}.json").unlink() + self.assertAuditFails("missing") + + def test_schema_violation_fails(self): + case_id = self._index()["cases"][0] + expectation = self._load_expectation(case_id) + expectation["expected_verdict"] = "probably_fine" + self._write_expectation(case_id, expectation) + self.assertAuditFails("expected_verdict") + + def test_provenance_without_retention_authority_fails(self): + case_id = self._index()["cases"][0] + path = self.root / "private" / "provenance" / f"{case_id}.json" + document = json.loads(path.read_text()) + del document["retention_authority"] + path.write_text(json.dumps(document, indent=2)) + self.assertAuditFails("retention_authority") + + def test_gating_expectation_without_a_root_cause_fails(self): + case_id = next( + item + for item in self._index()["cases"] + if self._load_expectation(item)["expected_verdict"] == "changes_required" + ) + expectation = self._load_expectation(case_id) + expectation["material_root_causes"] = [] + self._write_expectation(case_id, expectation) + self.assertAuditFails("at least one material root cause") + + def test_outcome_revealing_case_id_fails(self): + index = self._index() + old = index["cases"][0] + new = "clean-control-case" + index["cases"][0] = new + self._write_index(index) + (self.root / "reviewer" / old).rename(self.root / "reviewer" / new) + for area in ("expectations", "provenance"): + path = self.root / "private" / area / f"{old}.json" + document = json.loads(path.read_text()) + document["case_id"] = new + path.rename(self.root / "private" / area / f"{new}.json") + (self.root / "private" / area / f"{new}.json").write_text( + json.dumps(document, indent=2) + ) + self.assertAuditFails("reveals outcome") + + def test_outcome_revealing_reviewer_filename_fails(self): + case_id = self._index()["cases"][0] + (self.root / "reviewer" / case_id / "expected.json").write_text("{}") + self.assertAuditFails("unpermitted reviewer-visible file") + + def test_expectation_copied_into_the_reviewer_tree_fails(self): + case_id = self._index()["cases"][0] + shutil.copyfile( + self._expectation_path(case_id), + self.root / "reviewer" / case_id / "notes.json", + ) + self.assertAuditFails("unpermitted reviewer-visible file") + + def test_leaked_root_cause_text_in_a_packet_fails(self): + case_id = next( + item + for item in self._index()["cases"] + if self._load_expectation(item)["material_root_causes"] + ) + expectation = self._load_expectation(case_id) + leak = expectation["material_root_causes"][0]["consequence"] + packet_path = self.root / "reviewer" / case_id / "packet.json" + packet = json.loads(packet_path.read_text()) + packet["change_contract"]["non_goals"].append(leak) + packet_path.write_text(json.dumps(packet, indent=2)) + self.assertAuditFails("private expectation text") + + def test_undeclared_case_directory_fails(self): + (self.root / "reviewer" / "surprise-topic").mkdir() + self.assertAuditFails("not declared in corpus.json") + + def test_undeclared_expectation_file_fails(self): + (self.root / "private" / "expectations" / "surprise-topic.json").write_text( + "{}" + ) + self.assertAuditFails("is not declared") + + def test_packet_validity_disagreement_fails(self): + case_id = next( + item + for item in self._index()["cases"] + if self._load_expectation(item)["expected_verdict"] == "blocked" + ) + expectation = self._load_expectation(case_id) + expectation["packet_valid"] = True + expectation["expected_verdict"] = "clean" + self._write_expectation(case_id, expectation) + self.assertAuditFails("packet:") + + def test_a_missing_declared_dependency_fails(self): + index = self._index() + index["target_skill_dependencies"].append("review-nothing-at-all") + self._write_index(index) + self.assertAuditFails("missing declared skill") + + def test_a_missing_target_skill_fails(self): + index = self._index() + index["target_skill"] = "review-nothing-at-all" + self._write_index(index) + self.assertAuditFails("missing declared skill") + + def test_a_target_listing_itself_as_a_dependency_fails(self): + index = self._index() + index["target_skill_dependencies"].append(index["target_skill"]) + self._write_index(index) + self.assertAuditFails("lists itself as a dependency") + + def test_duplicate_declared_dependencies_fail(self): + index = self._index() + index["target_skill_dependencies"].append(index["target_skill_dependencies"][0]) + self._write_index(index) + self.assertAuditFails("duplicate target_skill_dependencies") + + def test_an_absent_dependency_declaration_fails(self): + index = self._index() + del index["target_skill_dependencies"] + self._write_index(index) + self.assertAuditFails("target_skill_dependencies") + + def test_grader_version_drift_fails(self): + index = self._index() + index["grader_version"] = "0.0" + self._write_index(index) + self.assertAuditFails("does not match the shipped grader") + + def test_reviewer_prompt_naming_a_verdict_fails(self): + prompt = self.root / "reviewer" / "PROMPT.md" + prompt.write_text( + prompt.read_text() + "\nReturn clean when nothing is wrong.\n" + ) + self.assertAuditFails("names verdict or severity word") + + +class AuditCommandTests(unittest.TestCase): + def test_audit_passes_on_the_shipped_corpus(self): + self.assertEqual([], audit_corpus.audit(None)) + + def test_audit_script_exits_zero_without_launching_a_runtime(self): + completed = subprocess.run( + [sys.executable, str(AUDIT_SCRIPT)], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(0, completed.returncode, completed.stderr) + self.assertIn("corpus audit passed", completed.stdout) + + def test_audit_script_reports_a_missing_corpus(self): + completed = subprocess.run( + [sys.executable, str(AUDIT_SCRIPT), "--corpus", "/nonexistent/corpus"], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(1, completed.returncode) + self.assertIn("missing corpus directory", completed.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/review-suite/scripts/tests/test_eval_grader.py b/review-suite/scripts/tests/test_eval_grader.py new file mode 100644 index 0000000..accdcee --- /dev/null +++ b/review-suite/scripts/tests/test_eval_grader.py @@ -0,0 +1,306 @@ +"""Grading-interface tests for the deterministic reference grader.""" + +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 grader # noqa: E402 + +CANDIDATE = {"head_sha": "a" * 40, "comparison_base_sha": "b" * 40} + +ROOT_CAUSE = { + "id": "rc.deadline", + "requirement": "A refreshed session keeps working for a further window.", + "trigger": "a refresh late in the original window", + "surface": "session.py:refresh", + "consequence": "an operator loses an active session", + "severity": "blocking", + "equivalent_formulations": [ + "stale expiry clock", + "deadline is not recomputed", + ], +} + +SECOND_ROOT_CAUSE = { + "id": "rc.record", + "requirement": "Every refresh is recorded.", + "trigger": "reading the log after several refreshes", + "surface": "session.py:refresh", + "consequence": "the history cannot be reconstructed", + "severity": "strong_recommendation", + "equivalent_formulations": ["no entry is written"], +} + +NON_FINDING = { + "id": "anf.naming", + "description": "Remarking on the pre-existing helper name is tolerated.", + "equivalent_formulations": ["the helper name predates this change"], +} + + +def finding(identifier, *, severity="blocking", location="session.py:31", **text): + body = { + "rule": "A stated requirement is not met.", + "concern": "The requirement is not met.", + "impact": "Users are affected.", + "proposed_change": "Meet the requirement.", + "expected_effect": "The requirement is met.", + "detail": "Demonstrated in the diff.", + } + body.update(text) + return { + "id": identifier, + "lens": "correctness", + "severity": severity, + "confidence": "high", + "rule": body["rule"], + "evidence": [{"location": location, "detail": body["detail"]}], + "concern": body["concern"], + "impact": body["impact"], + "proposed_change": body["proposed_change"], + "expected_effect": body["expected_effect"], + "location": location, + } + + +def result(verdict, findings): + return { + "schema_version": "1.0", + "lens": "aggregate", + "candidate": dict(CANDIDATE), + "verdict": verdict, + "findings": findings, + "blocking_reasons": [], + } + + +def expectation(verdict, root_causes, non_findings=()): + return { + "expectation_version": "1.0", + "case_id": "subject-under-test", + "packet_valid": True, + "expected_verdict": verdict, + "material_root_causes": list(root_causes), + "accepted_non_findings": list(non_findings), + } + + +def classification_of(grade, finding_id): + return next( + record["classification"] + for record in grade["findings"] + if record["finding_id"] == finding_id + ) + + +class MatchingTests(unittest.TestCase): + def test_a_paraphrase_of_the_root_cause_matches(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), + result( + "changes_required", + [ + finding( + "correctness.one", + concern="The refresh path keeps a stale expiry clock.", + ) + ], + ), + ) + self.assertEqual(["rc.deadline"], grade["matched_root_cause_ids"]) + self.assertEqual(1.0, grade["recall"]) + self.assertEqual("matched", classification_of(grade, "correctness.one")) + + def test_prose_that_never_names_the_root_cause_does_not_match(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), + result( + "changes_required", + [finding("correctness.one", location="unrelated_module.py:4")], + ), + ) + self.assertEqual([], grade["matched_root_cause_ids"]) + self.assertEqual(["rc.deadline"], grade["missed_root_cause_ids"]) + self.assertEqual(0.0, grade["recall"]) + + def test_a_duplicate_symptom_is_not_counted_twice(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), + result( + "changes_required", + [ + finding("correctness.one", concern="A stale expiry clock."), + finding( + "correctness.two", + severity="strong_recommendation", + concern="Sessions lapse early; a stale expiry clock.", + ), + ], + ), + ) + self.assertEqual(["rc.deadline"], grade["matched_root_cause_ids"]) + self.assertEqual(1.0, grade["recall"]) + self.assertEqual(["correctness.two"], grade["duplicate_finding_ids"]) + self.assertEqual([], grade["false_positive_finding_ids"]) + + def test_a_partial_match_is_referred_for_adjudication(self): + """Right surface, unrecognized description: neither hit nor miss.""" + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), + result( + "changes_required", + [finding("correctness.one", concern="Something feels off here.")], + ), + ) + self.assertEqual("partial", classification_of(grade, "correctness.one")) + self.assertEqual([], grade["matched_root_cause_ids"]) + self.assertEqual([], grade["false_positive_finding_ids"]) + self.assertEqual( + [ + { + "finding_id": "correctness.one", + "reason": "partial", + "candidate_root_cause_ids": ["rc.deadline"], + } + ], + grade["adjudication_required"], + ) + + def test_a_finding_matching_two_root_causes_is_ambiguous(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE, SECOND_ROOT_CAUSE]), + result( + "changes_required", + [ + finding( + "correctness.one", + concern="A stale expiry clock and no entry is written.", + ) + ], + ), + ) + self.assertEqual("ambiguous", classification_of(grade, "correctness.one")) + self.assertEqual([], grade["matched_root_cause_ids"]) + self.assertEqual( + ["rc.deadline", "rc.record"], + grade["adjudication_required"][0]["candidate_root_cause_ids"], + ) + + def test_an_unexpected_gating_finding_is_a_false_positive(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), + result( + "changes_required", + [ + finding("correctness.one", concern="A stale expiry clock."), + finding( + "correctness.invented", + location="theme.py:3", + concern="Colours should be configurable.", + ), + ], + ), + ) + self.assertEqual("unexpected", classification_of(grade, "correctness.invented")) + self.assertEqual(["correctness.invented"], grade["false_positive_finding_ids"]) + + def test_a_deferred_unexpected_finding_is_not_a_false_positive(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), + result( + "changes_required", + [ + finding("correctness.one", concern="A stale expiry clock."), + finding( + "correctness.noted", + severity="defer", + location="theme.py:3", + concern="An unrelated pre-existing smell.", + ), + ], + ), + ) + self.assertEqual([], grade["false_positive_finding_ids"]) + + def test_an_accepted_non_finding_is_tolerated(self): + grade = grader.grade( + expectation("clean", [], [NON_FINDING]), + result( + "clean", + [ + finding( + "correctness.naming", + severity="defer", + location="invoice.py:52", + concern="The helper name predates this change.", + ) + ], + ), + ) + self.assertEqual("accepted", classification_of(grade, "correctness.naming")) + self.assertEqual(["correctness.naming"], grade["accepted_finding_ids"]) + self.assertEqual([], grade["false_positive_finding_ids"]) + self.assertFalse(grade["false_alarm"]) + + +class VerdictTests(unittest.TestCase): + def test_a_missed_root_cause_reported_as_no_change_is_a_false_clean(self): + grade = grader.grade( + expectation("changes_required", [ROOT_CAUSE]), result("clean", []) + ) + self.assertTrue(grade["false_clean"]) + self.assertFalse(grade["verdict_match"]) + self.assertEqual(0.0, grade["recall"]) + + def test_gating_a_correct_candidate_is_a_false_alarm(self): + grade = grader.grade( + expectation("clean", []), + result( + "changes_required", + [finding("correctness.invented", location="theme.py:3")], + ), + ) + self.assertTrue(grade["false_alarm"]) + self.assertEqual(["correctness.invented"], grade["false_positive_finding_ids"]) + + def test_recall_is_absent_when_nothing_material_is_expected(self): + grade = grader.grade(expectation("clean", []), result("clean", [])) + self.assertIsNone(grade["recall"]) + self.assertTrue(grade["verdict_match"]) + + def test_grading_without_an_expectation_fails_loudly(self): + with self.assertRaises(grader.GradingError): + grader.grade(None, result("clean", [])) + + def test_the_grade_records_its_own_version(self): + grade = grader.grade(expectation("clean", []), result("clean", [])) + self.assertEqual(grader.GRADER_VERSION, grade["grader_version"]) + + +class NormalizationTests(unittest.TestCase): + def test_punctuation_and_case_do_not_change_a_match(self): + self.assertEqual( + "full", + grader.match_strength( + ROOT_CAUSE, + finding("correctness.one", concern="Stale-Expiry-Clock!"), + ), + ) + + def test_a_file_extension_alone_is_not_a_surface_match(self): + self.assertEqual( + "none", + grader.match_strength( + ROOT_CAUSE, finding("correctness.one", location="other.py:9") + ), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/review-suite/scripts/tests/test_eval_protocol.py b/review-suite/scripts/tests/test_eval_protocol.py new file mode 100644 index 0000000..2e46371 --- /dev/null +++ b/review-suite/scripts/tests/test_eval_protocol.py @@ -0,0 +1,544 @@ +"""Protocol and contamination tests for the review replay evaluator.""" + +from __future__ import annotations + +import copy +import json +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 corpus, protocol, runner # noqa: E402 + + +def build_request(case, **overrides): + kwargs = { + "case_id": case.case_id, + "target_skill": "review-code-change", + "skill_prompt": runner.target_skill_prompt("review-code-change"), + "contract_documents": runner.contract_documents(), + "instructions": case.instructions, + "packet": case.packet, + "run_number": 1, + "suite_commit": "0" * 40, + "corpus_version": "test", + "started_at": "2026-07-26T00:00:00+00:00", + } + kwargs.update(overrides) + return protocol.build_request(**kwargs) + + +class RequestTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.corpus = corpus.load_corpus() + cls.case = cls.corpus.cases[0] + + def test_request_matches_its_schema(self): + request = build_request(self.case) + self.assertEqual( + [], protocol.validate_against("executor-request.schema.json", request) + ) + + def test_request_records_suite_candidate_and_run_identity(self): + request = build_request(self.case, run_number=3, suite_commit="a" * 40) + self.assertEqual(3, request["run"]["run_number"]) + self.assertEqual("a" * 40, request["run"]["suite_commit"]) + self.assertEqual( + { + "head_sha": self.case.packet["candidate"]["head_sha"], + "comparison_base_sha": self.case.packet["candidate"][ + "comparison_base_sha" + ], + }, + request["run"]["candidate"], + ) + self.assertEqual( + protocol.prompt_digest(request["skill_prompt"]), + request["target_skill_digest"], + ) + + def test_the_payload_carries_every_skill_the_target_declares_required(self): + """A target that must verify siblings cannot be evaluated without them. + + `review-code-change` mandates an aggregate `blocked` result naming any + missing lens skill, so omitting one would score a compliant reviewer + wrong on every case expecting a merge verdict. + """ + loaded = corpus.load_corpus() + declared = set(loaded.target_skill_dependencies) + skills_root = protocol.REPOSITORY_ROOT / "skills" + installed = {path.name for path in skills_root.iterdir() if path.is_dir()} + target_text = (skills_root / loaded.target_skill / "SKILL.md").read_text() + required = { + name + for name in installed + if name != loaded.target_skill and name in target_text + } + self.assertTrue(required, "the target names no sibling skill at all") + self.assertEqual( + set(), + required - declared, + "the target's SKILL.md names skills absent from the declared closure", + ) + prompt = runner.target_skill_prompt( + loaded.target_skill, loaded.target_skill_dependencies + ) + for name in declared: + with self.subTest(dependency=name): + self.assertIn(f"{name}/SKILL.md", prompt) + self.assertIn( + (skills_root / name / "SKILL.md").read_text().strip(), prompt + ) + + def test_a_self_sufficient_target_needs_no_closure(self): + prompt = runner.target_skill_prompt("review-correctness") + self.assertIn("review-correctness/SKILL.md", prompt) + self.assertNotIn("review-code-change/SKILL.md", prompt) + + def test_a_target_cannot_declare_itself_as_a_dependency(self): + with self.assertRaises(runner.ConfigurationError): + runner.target_skill_documents("review-code-change", ["review-code-change"]) + + def test_a_missing_declared_dependency_fails_closed(self): + with self.assertRaises(runner.ConfigurationError): + runner.target_skill_documents( + "review-code-change", ["review-nothing-at-all"] + ) + + def test_the_payload_carries_every_mandated_skill_reference(self): + """A skill is not just its SKILL.md. + + `review-code-change` instructs the reviewer to read its orchestration + protocol, and the executor is told to reason only from what it is given, + so a mandated reference that never reaches the payload would measure a + reviewer working from a partial definition of its own job. + """ + documents = runner.target_skill_documents("review-code-change") + self.assertIn("review-code-change/SKILL.md", documents) + self.assertIn( + "review-code-change/references/orchestration-protocol.md", documents + ) + prompt = runner.target_skill_prompt("review-code-change") + for name, text in documents.items(): + with self.subTest(document=name): + self.assertIn(text.strip(), prompt) + request = build_request(self.case, skill_prompt=prompt) + self.assertIn( + "review-code-change/references/orchestration-protocol.md", + request["skill_prompt"], + ) + + def test_the_bundled_contract_mirror_is_not_sent_twice(self): + documents = runner.target_skill_documents("review-code-change") + self.assertEqual([], [name for name in documents if "review-suite" in name]) + # It is supplied once, from its canonical location. + self.assertIn("CONTRACT.md", runner.contract_documents()) + + def test_the_skill_digest_tracks_every_document_it_pins(self): + documents = runner.target_skill_documents("review-code-change") + baseline = protocol.prompt_digest( + runner.target_skill_prompt("review-code-change") + ) + for name in documents: + with self.subTest(document=name): + edited = dict(documents) + edited[name] += "\n\nAn additional sentence.\n" + self.assertNotEqual( + baseline, + protocol.prompt_digest( + runner.render_skill_prompt("review-code-change", edited) + ), + ) + + def test_case_reference_is_opaque(self): + request = build_request(self.case) + self.assertNotEqual(self.case.case_id, request["run"]["case_ref"]) + self.assertRegex(request["run"]["case_ref"], r"^c-[0-9a-f]{8}$") + + def test_packet_without_complete_identity_is_refused(self): + packet = copy.deepcopy(self.case.packet) + del packet["candidate"]["head_sha"] + with self.assertRaises(ValueError): + build_request(self.case, packet=packet) + + +class ContaminationTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.corpus = corpus.load_corpus() + + def test_every_corpus_request_is_blind(self): + for case in self.corpus.cases: + with self.subTest(case=case.case_id): + request = build_request(case) + self.assertEqual( + [], + protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ), + ) + + def _case_with_root_causes(self): + for case in self.corpus.cases: + if case.expectation["material_root_causes"]: + return case + raise AssertionError("no corpus case declares a material root cause") + + def test_injected_expectation_object_is_rejected(self): + case = self._case_with_root_causes() + request = build_request(case) + request["expectation"] = case.expectation + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue(any("unpermitted key" in error for error in errors)) + + def test_injected_root_cause_text_is_rejected(self): + case = self._case_with_root_causes() + root_cause = case.expectation["material_root_causes"][0] + for field in ("id", "consequence", "trigger"): + with self.subTest(field=field): + request = build_request(case) + request["instructions"] += f"\nHint: {root_cause[field]}\n" + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue( + any("private expectation text" in error for error in errors), + errors, + ) + + def test_injected_accepted_non_finding_text_is_rejected(self): + case = next( + item + for item in self.corpus.cases + if item.expectation["accepted_non_findings"] + ) + non_finding = case.expectation["accepted_non_findings"][0] + request = build_request(case) + request["instructions"] += "\n" + non_finding["description"] + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue(any("private expectation text" in error for error in errors)) + + def test_injected_provenance_text_is_rejected(self): + case = self.corpus.cases[0] + request = build_request(case) + request["instructions"] += "\n" + case.provenance["retention_authority"] + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue(any("private expectation text" in error for error in errors)) + + def test_leaked_case_identifier_is_rejected(self): + case = self.corpus.cases[0] + request = build_request(case) + request["run"]["case_ref"] = case.case_id + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue( + any("case_ref exposes the case identifier" in e for e in errors) + ) + + def test_a_leak_is_caught_whatever_characters_it_contains(self): + """JSON escaping must not hide a leak. + + `json.dumps` escapes non-ASCII to `\\uXXXX`, newlines to `\\n`, and + quotes to `\\"`, so searching a serialized payload silently misses + exactly the characters human-authored expectation prose uses. + """ + case = self._case_with_root_causes() + for label, leak in ( + ("em dash", "the equal case — silently refused"), + ("curly apostrophe", "the operator’s charge is refused"), + ("line break", "the charge is refused\nfor the exact balance"), + ("double quote", 'a charge of "exactly the balance" is refused'), + ("non-latin", "残高と同額の請求"), + ): + with self.subTest(label=label): + expectation = copy.deepcopy(case.expectation) + expectation["material_root_causes"][0]["consequence"] = leak + request = build_request(case) + request["packet"] = copy.deepcopy(case.packet) + request["packet"]["change_contract"]["goal"] += f" {leak}" + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=expectation, + provenance=case.provenance, + ) + self.assertTrue( + any("private expectation text" in error for error in errors), + f"{label} leak went undetected: {errors}", + ) + + def test_a_rewrapped_leak_is_still_caught(self): + case = self._case_with_root_causes() + leak = case.expectation["material_root_causes"][0]["consequence"] + request = build_request(case) + request["packet"] = copy.deepcopy(case.packet) + request["packet"]["change_contract"]["goal"] += " " + leak.replace(" ", "\n ") + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue(any("private expectation text" in error for error in errors)) + + def test_a_leak_hidden_in_an_object_key_is_caught(self): + case = self._case_with_root_causes() + leak = case.expectation["material_root_causes"][0]["consequence"] + request = build_request(case) + request["contract_documents"] = {leak: "irrelevant body"} + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue(any("private expectation text" in error for error in errors)) + + def test_audit_inspects_nested_payload_text(self): + """A leak buried inside the packet must be caught, not just top level.""" + case = self._case_with_root_causes() + request = build_request(case) + request["packet"] = copy.deepcopy(case.packet) + request["packet"]["change_contract"]["non_goals"].append( + case.expectation["material_root_causes"][0]["consequence"] + ) + errors = protocol.audit_request( + request, + case_id=case.case_id, + expectation=case.expectation, + provenance=case.provenance, + ) + self.assertTrue(any("private expectation text" in error for error in errors)) + + +class ResponseClassificationTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.corpus = corpus.load_corpus() + cls.case = next( + item + for item in cls.corpus.cases + if item.expectation["packet_valid"] + and item.expectation["material_root_causes"] + ) + + def _valid_response(self, verdict="changes_required"): + candidate = { + "head_sha": self.case.packet["candidate"]["head_sha"], + "comparison_base_sha": self.case.packet["candidate"]["comparison_base_sha"], + } + finding = { + "id": "correctness.example", + "lens": "correctness", + "severity": "blocking", + "confidence": "high", + "rule": "A stated requirement is not met.", + "evidence": [{"location": "example.py:1", "detail": "Demonstrated."}], + "concern": "The requirement is not met.", + "impact": "Users see the wrong result.", + "proposed_change": "Meet the requirement.", + "expected_effect": "The requirement is met.", + } + return { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "review_result", + "simulation": False, + "executor": {"name": "test"}, + "result": { + "schema_version": "1.0", + "lens": "aggregate", + "candidate": candidate, + "verdict": verdict, + "findings": [finding] if verdict == "changes_required" else [], + "blocking_reasons": [], + }, + } + + def classify(self, response): + return protocol.classify_response(self.case.packet, json.dumps(response)) + + def test_valid_review_result(self): + status, _, detail = self.classify(self._valid_response()) + self.assertEqual(("review_result", ""), (status, detail)) + + def test_non_json_output_is_malformed(self): + status, response, _ = protocol.classify_response(self.case.packet, "nope") + self.assertEqual("malformed_output", status) + self.assertIsNone(response) + + def test_non_object_output_is_malformed(self): + status, _, _ = protocol.classify_response(self.case.packet, "[1, 2]") + self.assertEqual("malformed_output", status) + + def test_protocol_version_mismatch_is_reported_separately(self): + response = self._valid_response() + response["protocol_version"] = "0.9" + status, _, _ = self.classify(response) + self.assertEqual("protocol_mismatch", status) + + def test_unknown_response_key_is_malformed(self): + response = self._valid_response() + response["expectation"] = {"expected_verdict": "clean"} + status, _, _ = self.classify(response) + self.assertEqual("malformed_output", status) + + def test_runtime_failure_requires_a_reason(self): + response = self._valid_response() + response["outcome"] = "runtime_failure" + del response["result"] + status, _, _ = self.classify(response) + self.assertEqual("malformed_output", status) + + response["failure"] = {"reason": "the runtime died"} + status, _, detail = self.classify(response) + self.assertEqual(("runtime_failure", "the runtime died"), (status, detail)) + + def test_blocked_outcome_requires_a_blocked_verdict(self): + response = self._valid_response() + response["outcome"] = "blocked" + status, _, _ = self.classify(response) + self.assertEqual("malformed_output", status) + + def test_review_result_cannot_carry_a_blocked_verdict(self): + response = self._valid_response() + response["result"]["verdict"] = "blocked" + status, _, _ = self.classify(response) + self.assertEqual("malformed_output", status) + + def test_valid_blocked_result_is_its_own_status(self): + blocked_case = next( + item + for item in self.corpus.cases + if item.expectation["expected_verdict"] == "blocked" + ) + response = { + "protocol_version": protocol.PROTOCOL_VERSION, + "outcome": "blocked", + "simulation": False, + "executor": {"name": "test"}, + "result": { + "schema_version": "1.0", + "lens": "aggregate", + "candidate": { + "head_sha": blocked_case.packet["candidate"]["head_sha"], + "comparison_base_sha": blocked_case.packet["candidate"][ + "comparison_base_sha" + ], + }, + "verdict": "blocked", + "findings": [], + "blocking_reasons": ["Required validation evidence is absent."], + }, + } + status, _, detail = protocol.classify_response( + blocked_case.packet, json.dumps(response) + ) + self.assertEqual(("blocked", ""), (status, detail)) + + def test_a_merge_verdict_on_an_incomplete_packet_is_a_review_result(self): + """A wrong answer on a deliberately incomplete packet is a wrong answer. + + The packet's own defect belongs to the case, not to the executor, so it + must not be reported as an evaluation failure - that would drop the one + behaviour a `packet_valid: false` case exists to measure. + """ + blocked_case = next( + item + for item in self.corpus.cases + if item.expectation["packet_valid"] is False + ) + self.assertTrue(protocol.VALIDATOR.validate_packet(blocked_case.packet)) + candidate = { + "head_sha": blocked_case.packet["candidate"]["head_sha"], + "comparison_base_sha": blocked_case.packet["candidate"][ + "comparison_base_sha" + ], + } + for verdict in ("clean", "changes_required"): + with self.subTest(verdict=verdict): + response = self._valid_response(verdict) + response["result"]["candidate"] = candidate + status, _, detail = protocol.classify_response( + blocked_case.packet, json.dumps(response) + ) + self.assertEqual(("review_result", ""), (status, detail)) + + def test_a_reply_defect_is_still_malformed_on_an_incomplete_packet(self): + blocked_case = next( + item + for item in self.corpus.cases + if item.expectation["packet_valid"] is False + ) + response = self._valid_response() + response["result"]["candidate"] = {"head_sha": "9" * 40} + status, _, detail = protocol.classify_response( + blocked_case.packet, json.dumps(response) + ) + self.assertEqual("malformed_output", status) + self.assertIn("result", detail) + + def test_result_bound_to_another_candidate_is_malformed(self): + response = self._valid_response() + response["result"]["candidate"]["head_sha"] = "9" * 40 + status, _, detail = self.classify(response) + self.assertEqual("malformed_output", status) + self.assertIn("head_sha", detail) + + def test_schema_invalid_review_result_is_malformed_not_a_finding_miss(self): + response = self._valid_response() + del response["result"]["findings"][0]["impact"] + status, _, _ = self.classify(response) + self.assertEqual("malformed_output", status) + + def test_non_numeric_usage_is_malformed(self): + response = self._valid_response() + response["usage"] = {"cost_usd": "free"} + status, _, detail = self.classify(response) + self.assertEqual("malformed_output", status) + self.assertIn("usage", detail) + + def test_failure_statuses_are_disjoint_from_gradable_statuses(self): + self.assertEqual( + set(), + set(protocol.EVALUATION_FAILURE_STATUSES) & set(protocol.GRADABLE_STATUSES), + ) + self.assertEqual( + set(protocol.ATTEMPT_STATUSES), + set(protocol.EVALUATION_FAILURE_STATUSES) + | set(protocol.GRADABLE_STATUSES) + | {"blocked"}, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/review-suite/scripts/tests/test_eval_report.py b/review-suite/scripts/tests/test_eval_report.py new file mode 100644 index 0000000..3f8f122 --- /dev/null +++ b/review-suite/scripts/tests/test_eval_report.py @@ -0,0 +1,354 @@ +"""Reporting tests: every required metric must be representable and correct.""" + +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 protocol, report # noqa: E402 + +CONFIGURATION = {"executor": "test", "runs_per_case": 2} + + +def attempt( + case_id, + run_number, + status="review_result", + *, + grade=None, + simulation=False, + duration=1.0, + usage=None, + verdict=None, +): + if verdict is None: + if status == "blocked": + verdict = "blocked" + elif grade: + verdict = grade["observed_verdict"] + return { + "case_id": case_id, + "case_ref": protocol.case_ref(case_id), + "run_number": run_number, + "status": status, + "detail": "", + "simulation": simulation, + "duration_seconds": duration, + "usage": usage, + "verdict": verdict, + "grade": grade, + } + + +def grade( + *, + expected="changes_required", + observed="changes_required", + expected_ids=("rc.one",), + matched_ids=("rc.one",), + false_positives=(), + adjudication=(), +): + return { + "grader_version": "1.0", + "expected_verdict": expected, + "observed_verdict": observed, + "verdict_match": expected == observed, + "false_clean": expected == "changes_required" and observed == "clean", + "false_alarm": expected == "clean" and observed == "changes_required", + "expected_root_cause_ids": list(expected_ids), + "matched_root_cause_ids": list(matched_ids), + "missed_root_cause_ids": [i for i in expected_ids if i not in matched_ids], + "recall": (len(matched_ids) / len(expected_ids)) if expected_ids else None, + "findings": [], + "false_positive_finding_ids": list(false_positives), + "accepted_finding_ids": [], + "duplicate_finding_ids": [], + "adjudication_required": list(adjudication), + } + + +class MetricTests(unittest.TestCase): + def test_every_required_metric_is_present(self): + aggregate = report.aggregate( + [attempt("subject-one", 1, grade=grade())], configuration=CONFIGURATION + ) + for key in ( + "material_finding_recall", + "false_clean_rate", + "false_positive_rate", + "unique_finding_contribution", + ): + self.assertIn(key, aggregate["quality"]) + for key in ("mean_verdict_stability", "mean_finding_stability"): + self.assertIn(key, aggregate["stability"]) + for status in protocol.ATTEMPT_STATUSES: + if status != "review_result": + self.assertIn(f"{status}_rate", aggregate["failures"]) + self.assertIn("mean_seconds", aggregate["latency"]) + self.assertIn("total_cost_usd", aggregate["usage"]) + self.assertEqual(report.REPORT_VERSION, aggregate["report_version"]) + + def test_recall_averages_only_graded_attempts(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, grade=grade(matched_ids=("rc.one",))), + attempt("subject-one", 2, grade=grade(matched_ids=())), + attempt("subject-two", 1, status="timeout"), + ], + configuration=CONFIGURATION, + ) + self.assertEqual(0.5, aggregate["quality"]["material_finding_recall"]) + self.assertEqual(2, aggregate["quality"]["recall_attempts"]) + self.assertEqual(2, aggregate["graded_attempts"]) + + def test_a_failed_attempt_is_never_scored_as_clean(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, status="timeout"), + attempt("subject-one", 2, status="runtime_failure"), + ], + configuration=CONFIGURATION, + ) + self.assertEqual(0, aggregate["graded_attempts"]) + self.assertIsNone(aggregate["quality"]["material_finding_recall"]) + self.assertIsNone(aggregate["quality"]["false_clean_rate"]) + self.assertEqual(0.5, aggregate["failures"]["timeout_rate"]) + self.assertEqual(0.5, aggregate["failures"]["runtime_failure_rate"]) + + def test_false_clean_and_false_positive_rates(self): + aggregate = report.aggregate( + [ + attempt( + "subject-one", + 1, + grade=grade(observed="clean", matched_ids=()), + ), + attempt("subject-one", 2, grade=grade()), + attempt( + "subject-two", + 1, + grade=grade(false_positives=("correctness.invented",)), + ), + ], + configuration=CONFIGURATION, + ) + self.assertAlmostEqual(1 / 3, aggregate["quality"]["false_clean_rate"]) + self.assertEqual(3, aggregate["quality"]["false_clean_denominator"]) + self.assertAlmostEqual(1 / 3, aggregate["quality"]["false_positive_rate"]) + + def test_unstable_verdicts_and_findings_are_reported(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, grade=grade()), + attempt( + "subject-one", 2, grade=grade(observed="clean", matched_ids=()) + ), + ], + configuration=CONFIGURATION, + ) + self.assertEqual(0.5, aggregate["stability"]["mean_verdict_stability"]) + self.assertEqual(0.5, aggregate["stability"]["mean_finding_stability"]) + + def test_a_reviewer_that_alternates_blocking_is_not_reported_as_stable(self): + """Refusing a verdict on one run and giving one on the next is unstable.""" + aggregate = report.aggregate( + [ + attempt("subject-one", 1, status="blocked"), + attempt("subject-one", 2, grade=grade()), + ], + configuration=CONFIGURATION, + ) + self.assertEqual(0.5, aggregate["stability"]["mean_verdict_stability"]) + self.assertEqual(0.5, aggregate["stability"]["mean_finding_stability"]) + self.assertEqual( + {"subject-one": 2}, aggregate["stability"]["per_case_stability_denominator"] + ) + + def test_blocking_is_not_agreement_with_answering_and_finding_nothing(self): + """A refused verdict and a zero-match answer are different behaviours. + + Both contribute no matched root causes, so collapsing them to the same + empty set would report the pair as perfectly stable on findings even + though verdict stability correctly reports 0.5. + """ + aggregate = report.aggregate( + [ + attempt("subject-one", 1, status="blocked"), + attempt("subject-one", 2, grade=grade(matched_ids=())), + ], + configuration=CONFIGURATION, + ) + self.assertEqual(0.5, aggregate["stability"]["mean_verdict_stability"]) + self.assertEqual(0.5, aggregate["stability"]["mean_finding_stability"]) + self.assertEqual(2, aggregate["stability"]["stability_denominator"]) + + def test_every_stability_figure_publishes_its_denominator(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, grade=grade()), + attempt("subject-one", 2, status="timeout"), + attempt("subject-two", 1, status="blocked"), + ], + configuration=CONFIGURATION, + ) + # The timed-out attempt produced no review, so it is outside stability; + # the blocked one produced a valid review, so it is inside it. + self.assertEqual(2, aggregate["stability"]["stability_denominator"]) + self.assertEqual( + {"subject-one": 1, "subject-two": 1}, + aggregate["stability"]["per_case_stability_denominator"], + ) + + def test_a_consistently_blocking_reviewer_is_reported_as_stable(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, status="blocked"), + attempt("subject-one", 2, status="blocked"), + ], + configuration=CONFIGURATION, + ) + self.assertEqual(1.0, aggregate["stability"]["mean_verdict_stability"]) + self.assertEqual(2, aggregate["stability"]["stability_denominator"]) + self.assertEqual(0, aggregate["graded_attempts"]) + + def test_unique_finding_contribution_names_root_causes_only_some_runs_found(self): + aggregate = report.aggregate( + [ + attempt( + "subject-one", + 1, + grade=grade( + expected_ids=("rc.one", "rc.two"), matched_ids=("rc.one",) + ), + ), + attempt( + "subject-one", + 2, + grade=grade( + expected_ids=("rc.one", "rc.two"), + matched_ids=("rc.one", "rc.two"), + ), + ), + ], + configuration=CONFIGURATION, + ) + self.assertEqual( + [{"case_id": "subject-one", "only_some_attempts": ["rc.two"]}], + aggregate["quality"]["unique_finding_contribution"], + ) + + def test_latency_is_summarized(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, grade=grade(), duration=1.0), + attempt("subject-one", 2, grade=grade(), duration=3.0), + ], + configuration=CONFIGURATION, + ) + self.assertEqual( + { + "count": 2, + "mean_seconds": 2.0, + "p50_seconds": 2.0, + "min_seconds": 1.0, + "max_seconds": 3.0, + }, + aggregate["latency"], + ) + + def test_absent_usage_is_reported_as_unavailable(self): + aggregate = report.aggregate( + [attempt("subject-one", 1, grade=grade())], configuration=CONFIGURATION + ) + self.assertFalse(aggregate["usage"]["available"]) + self.assertIsNone(aggregate["usage"]["total_cost_usd"]) + + def test_reported_usage_is_totalled_with_its_denominator(self): + aggregate = report.aggregate( + [ + attempt( + "subject-one", + 1, + grade=grade(), + usage={"cost_usd": 0.25, "input_tokens": 10}, + ), + attempt("subject-one", 2, grade=grade()), + ], + configuration=CONFIGURATION, + ) + self.assertTrue(aggregate["usage"]["available"]) + self.assertEqual(0.25, aggregate["usage"]["total_cost_usd"]) + self.assertEqual(1, aggregate["usage"]["reporting_attempts_cost_usd"]) + self.assertIsNone(aggregate["usage"]["total_output_tokens"]) + + def test_adjudication_requests_carry_their_case_and_run(self): + aggregate = report.aggregate( + [ + attempt( + "subject-one", + 1, + grade=grade( + adjudication=[ + { + "finding_id": "correctness.one", + "reason": "ambiguous", + "candidate_root_cause_ids": ["rc.one", "rc.two"], + } + ] + ), + ) + ], + configuration=CONFIGURATION, + ) + self.assertEqual( + [ + { + "case_id": "subject-one", + "run_number": 1, + "finding_id": "correctness.one", + "reason": "ambiguous", + "candidate_root_cause_ids": ["rc.one", "rc.two"], + } + ], + aggregate["adjudication_required"], + ) + + +class BaselineEligibilityTests(unittest.TestCase): + def test_one_simulated_attempt_disqualifies_the_whole_report(self): + aggregate = report.aggregate( + [ + attempt("subject-one", 1, grade=grade()), + attempt("subject-one", 2, grade=grade(), simulation=True), + ], + configuration=CONFIGURATION, + ) + self.assertTrue(aggregate["simulation"]) + self.assertFalse(aggregate["baseline_eligible"]) + + def test_a_fully_real_run_is_baseline_eligible(self): + aggregate = report.aggregate( + [attempt("subject-one", 1, grade=grade())], configuration=CONFIGURATION + ) + self.assertFalse(aggregate["simulation"]) + self.assertTrue(aggregate["baseline_eligible"]) + + def test_the_report_encodes_no_success_threshold(self): + """Interpretation belongs to a later ticket, not to the evaluator.""" + aggregate = report.aggregate( + [attempt("subject-one", 1, grade=grade(matched_ids=()))], + configuration=CONFIGURATION, + ) + forbidden = {"passed", "failed", "threshold", "target", "gate", "ok"} + self.assertEqual(set(), forbidden & set(aggregate)) + self.assertEqual(set(), forbidden & set(aggregate["quality"])) + + +if __name__ == "__main__": + unittest.main() diff --git a/review-suite/scripts/tests/test_eval_runner.py b/review-suite/scripts/tests/test_eval_runner.py new file mode 100644 index 0000000..7508f44 --- /dev/null +++ b/review-suite/scripts/tests/test_eval_runner.py @@ -0,0 +1,655 @@ +"""End-to-end runner tests: fresh processes, limits, and failure taxonomy.""" + +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +import tempfile +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 corpus, protocol, runner # noqa: E402 + +RUNNER_SCRIPT = SCRIPTS_DIR / "evals" / "runner.py" +FIXTURE_EXECUTOR = SCRIPTS_DIR / "evals" / "fixture_executor.py" +CLAUDE_EXECUTOR = SCRIPTS_DIR / "evals" / "claude_executor.py" + + +def fixture_command(mode: str | None = None) -> str: + command = f"{sys.executable} {FIXTURE_EXECUTOR}" + return f"{command} --mode {mode}" if mode else command + + +def run_runner(*args: str) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(RUNNER_SCRIPT), *args], + capture_output=True, + text=True, + check=False, + ) + + +class EvaluationTests(unittest.TestCase): + def setUp(self): + self.temp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.temp, ignore_errors=True) + + def evaluate(self, mode=None, **kwargs): + options = { + "corpus_root": None, + "runs": 1, + "timeout": 60.0, + "max_output_bytes": runner.DEFAULT_MAX_OUTPUT_BYTES, + "artifact_dir": None, + } + options.update(kwargs) + import shlex + + return runner.evaluate(shlex.split(fixture_command(mode)), **options) + + def test_a_full_pass_produces_one_attempt_per_case_and_run(self): + attempts, configuration = self.evaluate(runs=2) + cases = len(corpus.load_corpus().cases) + self.assertEqual(cases * 2, len(attempts)) + self.assertEqual(2, configuration["runs_per_case"]) + self.assertEqual(cases, configuration["cases"]) + self.assertEqual({1, 2}, {attempt["run_number"] for attempt in attempts}) + + def test_every_attempt_runs_in_its_own_process(self): + attempts, _ = self.evaluate(runs=2, artifact_dir=self.temp / "artifacts") + pids = set() + for path in (self.temp / "artifacts").glob("*.stdout.json"): + document = json.loads(path.read_text()) + pids.add(document.get("executor", {}).get("name")) + # The bundled executor reports a stable name; process freshness is + # proven by the runner spawning one subprocess per attempt. + self.assertEqual({"review-suite-fixture-executor"}, pids) + self.assertEqual(len(attempts), len(list((self.temp / "artifacts").iterdir()))) + + def test_each_attempt_records_suite_candidate_and_run_identity(self): + attempts, configuration = self.evaluate() + for attempt in attempts: + with self.subTest(case=attempt["case_id"]): + self.assertEqual(protocol.PROTOCOL_VERSION, attempt["protocol_version"]) + self.assertEqual(configuration["suite_commit"], attempt["suite_commit"]) + self.assertEqual( + configuration["corpus_version"], attempt["corpus_version"] + ) + self.assertEqual( + configuration["target_skill_digest"], + attempt["target_skill_digest"], + ) + self.assertEqual( + {"head_sha", "comparison_base_sha"}, set(attempt["candidate"]) + ) + self.assertEqual( + protocol.case_ref(attempt["case_id"]), attempt["case_ref"] + ) + self.assertIsNotNone(attempt["started_at"]) + self.assertIsNotNone(attempt["finished_at"]) + + def test_the_run_records_which_closure_was_sent(self): + """A digest proves two runs matched; it cannot say what they contained. + + A stratum has to be able to state which skills it evaluated, so the + closure's membership is recorded and not just hashed. + """ + attempts, configuration = self.evaluate() + loaded = corpus.load_corpus() + expected = list(loaded.target_skill_dependencies) + self.assertEqual(expected, configuration["target_skill_dependencies"]) + self.assertEqual(loaded.target_skill, configuration["target_skill"]) + documents = configuration["target_skill_documents"] + self.assertEqual(sorted(documents), documents) + self.assertIn(f"{loaded.target_skill}/SKILL.md", documents) + for dependency in expected: + with self.subTest(dependency=dependency): + self.assertIn(f"{dependency}/SKILL.md", documents) + for attempt in attempts: + with self.subTest(case=attempt["case_id"]): + self.assertEqual(loaded.target_skill, attempt["target_skill"]) + self.assertEqual(expected, attempt["target_skill_dependencies"]) + self.assertEqual( + configuration["target_skill_digest"], + attempt["target_skill_digest"], + ) + + def test_only_valid_review_results_are_graded(self): + attempts, _ = self.evaluate() + for attempt in attempts: + with self.subTest(case=attempt["case_id"], status=attempt["status"]): + if attempt["status"] == "review_result": + self.assertIsNotNone(attempt["grade"]) + else: + self.assertIsNone(attempt["grade"]) + + def test_a_valid_blocked_result_is_recorded_and_not_graded(self): + attempts, _ = self.evaluate() + blocked = [a for a in attempts if a["status"] == "blocked"] + self.assertTrue(blocked) + for attempt in blocked: + self.assertEqual("blocked", attempt["verdict"]) + self.assertIsNone(attempt["grade"]) + + def test_the_bundled_executor_is_always_marked_as_simulation(self): + attempts, _ = self.evaluate() + self.assertTrue(all(attempt["simulation"] for attempt in attempts)) + + def test_a_lying_executor_is_still_marked_as_simulation(self): + """Simulation is forced from the command, not trusted from the reply.""" + liar = self.temp / "fixture_executor.py" + source = FIXTURE_EXECUTOR.read_text().replace( + '"simulation": True', '"simulation": False' + ) + liar.write_text(source) + import shlex + + attempts, _ = runner.evaluate( + shlex.split(f"{sys.executable} {liar}"), + corpus_root=None, + runs=1, + timeout=60.0, + max_output_bytes=runner.DEFAULT_MAX_OUTPUT_BYTES, + artifact_dir=None, + ) + self.assertTrue(all(attempt["simulation"] for attempt in attempts)) + + def test_each_failure_mode_maps_to_its_own_status(self): + for mode, expected in ( + ("runtime_failure", "runtime_failure"), + ("crash", "runtime_failure"), + ("malformed_json", "malformed_output"), + ("malformed_result", "malformed_output"), + ("protocol_mismatch", "protocol_mismatch"), + ): + with self.subTest(mode=mode): + attempts, _ = self.evaluate(mode) + self.assertEqual( + {expected}, {attempt["status"] for attempt in attempts} + ) + self.assertTrue(all(a["grade"] is None for a in attempts)) + + def test_a_hanging_executor_times_out(self): + attempts, _ = self.evaluate("hang", timeout=0.5) + self.assertEqual({"timeout"}, {attempt["status"] for attempt in attempts}) + + def test_a_missing_executor_binary_is_a_spawn_failure(self): + attempts, _ = runner.evaluate( + ["/nonexistent/review-executor"], + corpus_root=None, + runs=1, + timeout=60.0, + max_output_bytes=runner.DEFAULT_MAX_OUTPUT_BYTES, + artifact_dir=None, + ) + self.assertEqual({"spawn_failure"}, {a["status"] for a in attempts}) + + def test_oversized_output_is_rejected(self): + attempts, _ = self.evaluate(max_output_bytes=10) + self.assertEqual({"output_too_large"}, {a["status"] for a in attempts}) + + def test_a_merge_verdict_on_an_incomplete_packet_is_graded_not_failed(self): + """The measured behaviour of a `packet_valid: false` case must be scored. + + An executor that issues a merge verdict where the evidence is + incomplete is giving a wrong answer, not breaking the harness. + """ + always_gating = self.temp / "always_gating_executor.py" + always_gating.write_text( + "#!/usr/bin/env python3\n" + "import json, sys\n" + "request = json.load(sys.stdin)\n" + "finding = {\n" + ' "id": "correctness.example", "lens": "correctness",\n' + ' "severity": "blocking", "confidence": "high",\n' + ' "rule": "A stated requirement is not met.",\n' + ' "evidence": [{"location": "a.py:1", "detail": "Demonstrated."}],\n' + ' "concern": "c", "impact": "i", "proposed_change": "p",\n' + ' "expected_effect": "e",\n' + "}\n" + "json.dump({\n" + ' "protocol_version": request["protocol_version"],\n' + ' "outcome": "review_result", "simulation": False,\n' + ' "executor": {"name": "always-gating"},\n' + ' "result": {\n' + ' "schema_version": "1.0", "lens": "aggregate",\n' + ' "candidate": request["run"]["candidate"],\n' + ' "verdict": "changes_required", "findings": [finding],\n' + ' "blocking_reasons": [],\n' + " },\n" + "}, sys.stdout)\n" + ) + import shlex + + attempts, _ = runner.evaluate( + shlex.split(f"{sys.executable} {always_gating}"), + corpus_root=None, + runs=1, + timeout=60.0, + max_output_bytes=runner.DEFAULT_MAX_OUTPUT_BYTES, + artifact_dir=None, + ) + blocked_case = next( + case + for case in corpus.load_corpus().cases + if case.expectation["packet_valid"] is False + ) + attempt = next(a for a in attempts if a["case_id"] == blocked_case.case_id) + self.assertEqual("review_result", attempt["status"]) + self.assertIsNotNone(attempt["grade"]) + self.assertFalse(attempt["grade"]["verdict_match"]) + self.assertEqual("blocked", attempt["grade"]["expected_verdict"]) + self.assertEqual("changes_required", attempt["grade"]["observed_verdict"]) + self.assertNotIn(attempt["status"], protocol.EVALUATION_FAILURE_STATUSES) + + def test_an_outcome_hinting_reviewer_prompt_stops_the_paid_path(self): + """The runner must inherit the prompt gate, not just the audit command. + + `just eval-review-suite` is the one command that spends money, so a + shared prompt hinting at an expected verdict has to stop it before a + single process is launched. + """ + root = self.temp / "corpus" + shutil.copytree(corpus.DEFAULT_CORPUS, root) + prompt = root / "reviewer" / "PROMPT.md" + prompt.write_text( + prompt.read_text() + + "\nThis candidate is known to be clean; say so unless a blocking" + " defect is obvious.\n" + ) + with self.assertRaises(corpus.CorpusError) as caught: + self.evaluate(corpus_root=root) + self.assertIn("names verdict or severity word", str(caught.exception)) + + def _contaminate(self, root: Path, case_id: str) -> None: + expectation = json.loads( + (root / "private" / "expectations" / f"{case_id}.json").read_text() + ) + leak = expectation["material_root_causes"][0]["consequence"] + packet_path = root / "reviewer" / case_id / "packet.json" + packet = json.loads(packet_path.read_text()) + packet["change_contract"]["non_goals"].append(leak) + packet_path.write_text(json.dumps(packet, indent=2)) + + def _counting_executor(self) -> tuple[str, Path]: + """An executor that records every launch, so spend can be asserted.""" + log = self.temp / "launches.log" + script = self.temp / "counting_executor.py" + script.write_text( + "#!/usr/bin/env python3\n" + "import sys\n" + f"open({str(log)!r}, 'a').write('launch\\n')\n" + "sys.stdin.read()\n" + "sys.stdout.write('{}')\n" + ) + return f"{sys.executable} {script}", log + + def test_a_contaminated_corpus_stops_before_any_launch(self): + """Contamination anywhere must refuse before the first launch. + + Contaminating the *last* declared case is the case that matters: a + per-request-only audit would already have paid for every earlier case + before noticing, and then thrown the whole run away. + """ + import shlex + + root = self.temp / "corpus" + shutil.copytree(corpus.DEFAULT_CORPUS, root) + index = json.loads((root / "corpus.json").read_text()) + eligible = [ + item + for item in index["cases"] + if json.loads( + (root / "private" / "expectations" / f"{item}.json").read_text() + )["material_root_causes"] + ] + self.assertTrue(eligible) + last_contaminatable = eligible[-1] + self.assertNotEqual( + index["cases"][0], + last_contaminatable, + "the corpus must let this test contaminate a non-first case", + ) + self._contaminate(root, last_contaminatable) + + command, log = self._counting_executor() + with self.assertRaises(corpus.CorpusError) as caught: + runner.evaluate( + shlex.split(command), + corpus_root=root, + runs=1, + timeout=60.0, + max_output_bytes=runner.DEFAULT_MAX_OUTPUT_BYTES, + artifact_dir=None, + ) + self.assertIn("contaminated request", str(caught.exception)) + self.assertFalse( + log.exists(), "an executor was launched before contamination was refused" + ) + + def test_a_contaminated_first_case_also_stops_before_any_launch(self): + import shlex + + root = self.temp / "corpus" + shutil.copytree(corpus.DEFAULT_CORPUS, root) + index = json.loads((root / "corpus.json").read_text()) + self._contaminate(root, index["cases"][0]) + command, log = self._counting_executor() + with self.assertRaises(corpus.CorpusError): + runner.evaluate( + shlex.split(command), + corpus_root=root, + runs=1, + timeout=60.0, + max_output_bytes=runner.DEFAULT_MAX_OUTPUT_BYTES, + artifact_dir=None, + ) + self.assertFalse(log.exists()) + + def test_artifacts_are_written_only_when_requested(self): + self.evaluate() + self.assertFalse((self.temp / "artifacts").exists()) + self.evaluate(artifact_dir=self.temp / "artifacts") + self.assertTrue(any((self.temp / "artifacts").iterdir())) + + +class CommandLineTests(unittest.TestCase): + def setUp(self): + self.temp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.temp, ignore_errors=True) + + def test_a_clean_pass_exits_zero(self): + completed = run_runner("--executor", fixture_command()) + self.assertEqual(0, completed.returncode, completed.stderr) + aggregate = json.loads(completed.stdout) + self.assertEqual(protocol.PROTOCOL_VERSION, aggregate["protocol_version"]) + + def test_an_evaluation_failure_exits_one(self): + completed = run_runner("--executor", fixture_command("runtime_failure")) + self.assertEqual(1, completed.returncode) + self.assertIn("runtime_failure", completed.stderr) + + def test_a_missing_executor_argument_is_rejected(self): + completed = run_runner() + self.assertEqual(2, completed.returncode) + self.assertIn("--executor", completed.stderr) + + def test_an_empty_executor_argument_is_rejected(self): + completed = run_runner("--executor", " ") + self.assertEqual(2, completed.returncode) + self.assertIn("--executor is empty", completed.stderr) + + def test_out_of_range_limits_are_rejected(self): + for option, value, fragment in ( + ("--runs", "0", "--runs must be at least 1"), + ("--timeout", "0", "--timeout must be greater than 0"), + ("--max-output-bytes", "0", "--max-output-bytes must be at least 1"), + ): + with self.subTest(option=option): + completed = run_runner("--executor", fixture_command(), option, value) + self.assertEqual(2, completed.returncode, completed.stdout) + self.assertIn(fragment, completed.stderr) + + def test_a_missing_corpus_is_rejected(self): + completed = run_runner( + "--executor", fixture_command(), "--corpus", "/nonexistent/corpus" + ) + self.assertEqual(2, completed.returncode) + self.assertIn("missing corpus directory", completed.stderr) + + def test_a_missing_target_skill_is_rejected(self): + root = self.temp / "corpus" + shutil.copytree(corpus.DEFAULT_CORPUS, root) + index = json.loads((root / "corpus.json").read_text()) + index["target_skill"] = "review-nothing-at-all" + (root / "corpus.json").write_text(json.dumps(index, indent=2)) + completed = run_runner("--executor", fixture_command(), "--corpus", str(root)) + self.assertEqual(2, completed.returncode) + # Caught while loading the corpus, before any prompt is assembled. + self.assertIn("missing declared skill", completed.stderr) + self.assertIn("review-nothing-at-all", completed.stderr) + + def test_a_simulated_run_cannot_write_a_baseline_report(self): + baseline = self.temp / "baseline.json" + completed = run_runner( + "--executor", fixture_command(), "--baseline-report", str(baseline) + ) + self.assertEqual(2, completed.returncode) + self.assertIn("baseline refused", completed.stderr) + self.assertFalse(baseline.exists()) + + def test_attempt_and_report_files_are_written_when_requested(self): + attempts_out = self.temp / "attempts.jsonl" + report_out = self.temp / "report.json" + completed = run_runner( + "--executor", + fixture_command(), + "--attempts-out", + str(attempts_out), + "--report-out", + str(report_out), + ) + self.assertEqual(0, completed.returncode, completed.stderr) + records = [json.loads(line) for line in attempts_out.read_text().splitlines()] + self.assertEqual(len(corpus.load_corpus().cases), len(records)) + aggregate = json.loads(report_out.read_text()) + self.assertFalse(aggregate["baseline_eligible"]) + + +class RealRuntimeAdapterTests(unittest.TestCase): + """Drive the documented real-runtime adapter without a paid runtime. + + A stub stands in for the `claude` binary so the whole adapter path - + prompt construction, headless invocation, JSON extraction, usage mapping, + and protocol response - is exercised end to end for free. + """ + + def setUp(self): + self.temp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.temp, ignore_errors=True) + self.case = next( + case + for case in corpus.load_corpus().cases + if case.expectation["packet_valid"] + ) + self.request = protocol.build_request( + case_id=self.case.case_id, + target_skill="review-code-change", + skill_prompt=runner.target_skill_prompt("review-code-change"), + contract_documents=runner.contract_documents(), + instructions=self.case.instructions, + packet=self.case.packet, + run_number=1, + suite_commit="0" * 40, + corpus_version="test", + started_at="2026-07-26T00:00:00+00:00", + ) + + def _stub(self, body: str) -> Path: + path = self.temp / "claude_stub.py" + path.write_text(body) + path.chmod(0o755) + return path + + def _invoke(self, stub: Path) -> tuple[int, str, str]: + completed = subprocess.run( + [ + sys.executable, + str(CLAUDE_EXECUTOR), + "--claude-bin", + str(stub), + "--model", + "stub-model", + ], + input=json.dumps(self.request), + capture_output=True, + text=True, + check=False, + ) + return completed.returncode, completed.stdout, completed.stderr + + def test_the_adapter_completes_an_evaluation_through_the_protocol(self): + stub = self._stub( + "#!/usr/bin/env python3\n" + "import json, sys\n" + "prompt = sys.stdin.read()\n" + "candidate = json.loads(prompt.strip().splitlines()[-1])\n" + "result = {\n" + ' "schema_version": "1.0", "lens": "aggregate",\n' + ' "candidate": candidate, "verdict": "clean",\n' + ' "findings": [], "blocking_reasons": [],\n' + "}\n" + "json.dump({\n" + ' "result": json.dumps(result),\n' + ' "model": "stub-model-1",\n' + ' "usage": {"input_tokens": 12, "output_tokens": 34},\n' + ' "total_cost_usd": 0.5,\n' + "}, sys.stdout)\n" + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + status, response, detail = protocol.classify_response(self.case.packet, stdout) + self.assertEqual(("review_result", ""), (status, detail)) + self.assertFalse(response["simulation"]) + self.assertEqual("stub-model-1", response["executor"]["model"]) + self.assertEqual( + {"input_tokens": 12, "output_tokens": 34, "cost_usd": 0.5}, + response["usage"], + ) + + def test_a_failing_runtime_becomes_a_runtime_failure_not_a_clean_review(self): + stub = self._stub( + "#!/usr/bin/env python3\n" + "import sys\n" + "sys.stdin.read()\n" + "sys.stderr.write('the runtime refused\\n')\n" + "sys.exit(1)\n" + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + status, _, detail = protocol.classify_response(self.case.packet, stdout) + self.assertEqual("runtime_failure", status) + self.assertIn("the runtime refused", detail) + + def test_prose_instead_of_json_becomes_a_runtime_failure(self): + stub = self._stub( + "#!/usr/bin/env python3\n" + "import json, sys\n" + "sys.stdin.read()\n" + "json.dump({'result': 'I would rather explain in words.'}, sys.stdout)\n" + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + status, _, _ = protocol.classify_response(self.case.packet, stdout) + self.assertEqual("runtime_failure", status) + + def _envelope_stub(self, envelope_body: str) -> Path: + """A stub `claude` that echoes the candidate and a chosen envelope.""" + return self._stub( + "#!/usr/bin/env python3\n" + "import json, sys\n" + "prompt = sys.stdin.read()\n" + "candidate = json.loads(prompt.strip().splitlines()[-1])\n" + "result = {\n" + ' "schema_version": "1.0", "lens": "aggregate",\n' + ' "candidate": candidate, "verdict": "clean",\n' + ' "findings": [], "blocking_reasons": [],\n' + "}\n" + f"envelope = {envelope_body}\n" + 'envelope["result"] = json.dumps(result)\n' + "json.dump(envelope, sys.stdout)\n" + ) + + def test_cached_prompt_tokens_are_counted_as_input(self): + """Headless output reports most prompt tokens as cache, not input. + + A real 16456-token prompt reported `input_tokens: 2` with the rest under + cache creation and cache read, so totalling only `input_tokens` published + an input count wrong by three orders of magnitude - the number a frozen + cost envelope would rest on. + """ + stub = self._envelope_stub( + '{"usage": {"input_tokens": 2, "cache_creation_input_tokens": 5657,' + ' "cache_read_input_tokens": 10797, "output_tokens": 4},' + ' "total_cost_usd": 0.0409,' + ' "modelUsage": {"stub-model-1": {"inputTokens": 2}}}' + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + status, response, _ = protocol.classify_response(self.case.packet, stdout) + self.assertEqual("review_result", status) + self.assertEqual(16456, response["usage"]["input_tokens"]) + self.assertEqual(4, response["usage"]["output_tokens"]) + self.assertEqual(0.0409, response["usage"]["cost_usd"]) + + def test_the_model_is_read_from_the_model_usage_mapping(self): + """`modelUsage` is a mapping keyed by model id, never a string.""" + stub = self._envelope_stub( + '{"usage": {"input_tokens": 1, "output_tokens": 1},' + ' "total_cost_usd": 0.01,' + ' "modelUsage": {"claude-stub-4-6[1m]": {"inputTokens": 1}}}' + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + _, response, _ = protocol.classify_response(self.case.packet, stdout) + self.assertEqual("claude-stub-4-6[1m]", response["executor"]["model"]) + + def test_a_plain_model_string_still_wins(self): + stub = self._envelope_stub( + '{"model": "explicit-model", "usage": {"output_tokens": 1},' + ' "total_cost_usd": 0.01,' + ' "modelUsage": {"ignored-model": {"inputTokens": 1}}}' + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + _, response, _ = protocol.classify_response(self.case.packet, stdout) + self.assertEqual("explicit-model", response["executor"]["model"]) + + def test_an_attempt_without_model_identity_is_a_runtime_failure(self): + """Required identity: refuse rather than record a nameless model.""" + stub = self._envelope_stub( + '{"usage": {"output_tokens": 1}, "total_cost_usd": 0.01}' + ) + completed = subprocess.run( + [sys.executable, str(CLAUDE_EXECUTOR), "--claude-bin", str(stub)], + input=json.dumps(self.request), + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(0, completed.returncode, completed.stderr) + status, _, detail = protocol.classify_response( + self.case.packet, completed.stdout + ) + self.assertEqual("runtime_failure", status) + self.assertIn("no model identity", detail) + + def test_an_explicit_model_flag_satisfies_identity(self): + stub = self._envelope_stub( + '{"usage": {"output_tokens": 1}, "total_cost_usd": 0.01}' + ) + code, stdout, stderr = self._invoke(stub) + self.assertEqual(0, code, stderr) + status, response, _ = protocol.classify_response(self.case.packet, stdout) + self.assertEqual("review_result", status) + self.assertEqual("stub-model", response["executor"]["model"]) + + def test_the_adapter_prompt_stays_result_blind(self): + from evals import claude_executor + + prompt = claude_executor.build_prompt(self.request) + for blind in protocol.blind_strings( + self.case.expectation, self.case.provenance + ): + self.assertNotIn(blind.lower(), prompt.lower()) + self.assertNotIn(self.case.case_id, prompt) + + +if __name__ == "__main__": + unittest.main()