From 8386abb6641be4bd6e431f70fd9c336b8be617c6 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Wed, 29 Jul 2026 15:04:04 -0700 Subject: [PATCH 1/3] feat(review-suite): source two harder discriminating cases and preregister their validation ceiling (#89) - Add `artifact-promotion-environment-shortcut`, a harder sibling to `dependency-strictness-propagation`: the untouched sibling decision path is two calls deep and lives in a module named for staging/rollout defaults, not dependency or promotion checking. - Add `audit-log-flush-keyword-probe`, a harder sibling to `stale-claim-release-guard`: the test insufficiency is carried by a mocked session that grants success on a keyword match in the request text, rather than by the test's initial state. - Both cases are sourced from real, adjudicated, public review findings in shaug/atelier (PR 315 and PR 755 respectively), minimized to fictional subjects per #58's sourcing/sanitization discipline, and each ships an executable oracle that independently confirms the stated root cause. - Register both cases in the existing `s1-correctness-orchestrator` corpus (`corpus_version` 0.1 -> 0.2) and freeze the validation configuration (suite commit, grader version, run count, $5.00 cost ceiling) before any scored output is examined. - #57's ablation matrix found neither existing target case demonstrates unique causal contribution to the pass meant to fix it, because both sit within the reviewer's default competence even without the pass. This sources genuinely harder cases so the corpus can actually discriminate, or honestly report that it still cannot. --- CHANGELOG.md | 6 +- .../s1-correctness-orchestrator/corpus.json | 6 +- ...tifact-promotion-environment-shortcut.json | 34 +++++ .../audit-log-flush-keyword-probe.json | 34 +++++ ...tifact-promotion-environment-shortcut.json | 13 ++ .../audit-log-flush-keyword-probe.json | 13 ++ .../packet.json | 90 ++++++++++++ .../audit-log-flush-keyword-probe/packet.json | 86 +++++++++++ ...ng-case-validation-frozen-configuration.md | 131 +++++++++++++++++ ...artifact_promotion_environment_shortcut.py | 137 ++++++++++++++++++ .../oracles/audit_log_flush_keyword_probe.py | 133 +++++++++++++++++ 11 files changed, 680 insertions(+), 3 deletions(-) create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/artifact-promotion-environment-shortcut.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/audit-log-flush-keyword-probe.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/artifact-promotion-environment-shortcut.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/audit-log-flush-keyword-probe.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json create mode 100644 review-suite/evals/strata/s1-correctness-orchestrator/reviewer/audit-log-flush-keyword-probe/packet.json create mode 100644 review-suite/evals/v2/discriminating-case-validation-frozen-configuration.md create mode 100644 review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py create mode 100644 review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 95166e9..e2e5159 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,13 @@ summary: Chronological history of repository and skill changes. # Changelog -## 2026-07-29 — Designed the review-fix-loop skill, migrated implement-ticket and babysit-pr to consume the final review-result contract, rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect, added connector-outcome curation and promotion tooling, added a skill-root override for mechanism ablation runs, ran the preregistered v2 ablation and integration closeout, and confirmed the session-continuation-summary verification-only regression with an independent rerun +## 2026-07-29 — Sourced two harder discriminating cases for the traversal and verification-sufficiency passes, designed the review-fix-loop skill, migrated implement-ticket and babysit-pr to consume the final review-result contract, rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect, added connector-outcome curation and promotion tooling, added a skill-root override for mechanism ablation runs, ran the preregistered v2 ablation and integration closeout, and confirmed the session-continuation-summary verification-only regression with an independent rerun +- feat(review-suite): source two harder discriminating + `s1-correctness-orchestrator` cases for the traversal and + verification-sufficiency passes and preregister their validation ceiling (#89) - docs: design the review-fix-loop skill + (`06538e5c097ff8e6ef15b12d5fbf61b3d959abf7`) - docs(review-suite): add a confirming rerun of the session-continuation-summary verification-only regression (#57 follow-up) (`cd8efa444018d036a5749a1955e1f34ebe06b51f`) diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json b/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json index 456993d..2628775 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/corpus.json @@ -1,5 +1,5 @@ { - "corpus_version": "0.1-s1-populated", + "corpus_version": "0.2-s1-populated", "protocol_version": "1.0", "grader_version": "1.1", "target_skill": "review-code-change", @@ -13,9 +13,11 @@ "ground_truth": "human-review", "scored": true, "grading_is_signal": true, - "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss." + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss. `0.2` adds two cases (#89): a harder, disguised sibling to `dependency-strictness-propagation` and to `stale-claim-release-guard`, sourced after #57's ablation matrix found neither original target case demonstrates unique causal contribution to the pass meant to fix it." }, "cases": [ + "artifact-promotion-environment-shortcut", + "audit-log-flush-keyword-probe", "dependency-hint-parser-coverage", "dependency-strictness-propagation", "optional-tool-probe", diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/artifact-promotion-environment-shortcut.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/artifact-promotion-environment-shortcut.json new file mode 100644 index 0000000..0a653ba --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/artifact-promotion-environment-shortcut.json @@ -0,0 +1,34 @@ +{ + "expectation_version": "1.0", + "case_id": "artifact-promotion-environment-shortcut", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.rollout-path-keeps-marker-only-promotion-check", + "requirement": "Every path that decides whether an artifact counts as promoted applies the same mandatory-verification rule once verification is required for that artifact.", + "trigger": "An artifact requires mandatory verification, carries a promotion marker from the release tool, but has no ledger-confirmed proof, and enters the staging environment-selection step.", + "surface": "_pick_rollout_defaults", + "consequence": "`resolve_rollout_environment` reaches `artifact_promotable` through `_pick_rollout_defaults` without ever passing `external_proof`, so the environment-selection worker still treats the artifact as promoted for staging-defaults purposes even though `decide_promotion` would hold its release. The diff only hardens the gate's own direct call; the two-hop rollout path keeps deciding the same question permissively, and the change contract's non-goal about rollout defaults reads as license to leave it alone rather than as a sign it was missed.", + "severity": "blocking", + "equivalent_formulations": [ + "_pick_rollout_defaults still calls artifact_promotable without external_proof", + "the rollout path was not updated to require ledger proof", + "resolve_rollout_environment can still treat an unverified artifact as promoted", + "a marker-only artifact can still get promoted-tier rollout defaults", + "the verification requirement is applied inconsistently between the gate and the rollout path" + ] + } + ], + "accepted_non_findings": [ + { + "id": "anf.optional-proof-parameter-shape", + "description": "A reviewer may prefer a distinct function over an optional `external_proof` parameter. Reasonable as design commentary; the packet's preserved-behaviour list keeps the permissive result reachable when verification is not required, so an optional parameter satisfies the contract.", + "equivalent_formulations": [ + "prefer a separate function to an optional proof parameter", + "external_proof as an optional keyword argument is a design smell" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/audit-log-flush-keyword-probe.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/audit-log-flush-keyword-probe.json new file mode 100644 index 0000000..9c2b230 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/expectations/audit-log-flush-keyword-probe.json @@ -0,0 +1,34 @@ +{ + "expectation_version": "1.0", + "case_id": "audit-log-flush-keyword-probe", + "packet_valid": true, + "calibrated": false, + "expected_verdict": "changes_required", + "material_root_causes": [ + { + "id": "rc.retry-reuses-stale-optimistic-concurrency-script", + "requirement": "A retried flush re-derives its script from a fresh read of the entry's current version before resending.", + "trigger": "The first flush attempt reports a transient session error, and a concurrent flush advances the entry's stored version before the retry runs.", + "surface": "flush_entry", + "consequence": "The retry resends the exact same already-rendered script, whose `WHERE version = ...` clause now names a version older than the row's current one, so the session reports the script as syntactically successful while it matches zero rows and nothing is actually persisted on retry; `flush_entry` still records the entry as flushed. The added `_FakeSession` treats any script containing `COMMIT` as a persisted success and never models whether the version clause would actually match a row, so it exercises the retry path in name only and cannot tell a real persisted retry from this silent no-op.", + "severity": "blocking", + "equivalent_formulations": [ + "the retry resends the same stale script instead of re-rendering it", + "the version used on retry is not refreshed from a fresh read", + "a concurrent flush between attempts can make the retry a silent no-op", + "the fake session only checks for COMMIT in the script text, not whether it actually persisted", + "the added test cannot distinguish a real persisted retry from a no-op retry" + ] + } + ], + "accepted_non_findings": [ + { + "id": "anf.template-rendered-sql", + "description": "Building the script from a string template rather than a parameterized query API is unusual, but it is how this session boundary's script interface is used everywhere else in this module; not a defect here.", + "equivalent_formulations": [ + "string templating is used to build the script", + "the script is built via string formatting instead of bound parameters" + ] + } + ] +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/artifact-promotion-environment-shortcut.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/artifact-promotion-environment-shortcut.json new file mode 100644 index 0000000..a5b6e95 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/artifact-promotion-environment-shortcut.json @@ -0,0 +1,13 @@ +{ + "provenance_version": "1.0", + "case_id": "artifact-promotion-environment-shortcut", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 315, review comment 2866874599, authored by the repository owner and accepted in the reply (comment 2866898151, \"Good catch; this is now aligned\") naming the tests added to cover it. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional artifact-release system, deliberately reshaped to be less salient than this stratum's existing `dependency-strictness-propagation` case: the untouched sibling path is reached two calls deep (`resolve_rollout_environment` -> `_pick_rollout_defaults` -> `artifact_promotable`) rather than by one direct call, and it lives in a module (`rollout/targeting.py`) whose name and docstrings describe staging-environment defaults, not dependency or promotion checking. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 315 review comment 2866874599, accepted in reply 2866898151 (tests named: test_changeset_base_branch_uses_same_dependency_integration_signal_as_pr_gate, test_attempt_create_pr_passes_repo_slug_to_base_resolver).", + "second": "oracle" + }, + "recorded_at": "2026-07-29", + "notes": "Sourced for #89 as a harder discriminating sibling to `dependency-strictness-propagation`, per #57's ablation matrix finding that the existing traversal-pass target case resolves identically whether or not the traversal pass is enabled. Multi-file, multi-hop case: the defect is only visible by reading a caller the diff does not touch and tracing one further call through a helper. Second adjudication is available by oracle." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/audit-log-flush-keyword-probe.json b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/audit-log-flush-keyword-probe.json new file mode 100644 index 0000000..1ae20b6 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/audit-log-flush-keyword-probe.json @@ -0,0 +1,13 @@ +{ + "provenance_version": "1.0", + "case_id": "audit-log-flush-keyword-probe", + "origin": "minimized_reproduction", + "retention_authority": "Minimized from an adjudicated human review finding in the public repository shaug/atelier, pull request 755, review comment 3031027866, authored by the repository owner and accepted in the reply (comment 3031075465, describing the tightened regression) naming the concrete fix. Public source, owner-authored review, no third-party or customer material.", + "sanitization": "Rewritten from scratch against a fictional audit-log flush module. Retains only the failure shape: a retry path resends a stale precondition, and the accompanying fake test session grants success on a superficial keyword match in the request text rather than modelling whether the request actually persisted - the exact shape of `stale-claim-release-guard`'s test-insufficiency finding but disguised behind a mocked session boundary rather than an initial-state choice, so it is less visually obvious on a first read. No source identifier, path, symbol, prose, or diff was copied. No business logic, domain identifier, customer context, credential, or hidden reasoning.", + "adjudication": { + "first": "shaug/atelier PR 755 review comment 3031027866, accepted in reply 3031075465 (regression tightened around the session boundary; a malformed-helper-evidence failure case added).", + "second": "oracle" + }, + "recorded_at": "2026-07-29", + "notes": "Sourced for #89 as a harder discriminating sibling to `stale-claim-release-guard`, per #57's ablation matrix finding that the existing verification-sufficiency target case resolves identically whether or not the verification-sufficiency pass is enabled. The insufficiency is carried by a fake session's keyword-only success check rather than by the test's initial state, which is the harder-to-notice shape #89 asks for. Second adjudication is available by oracle." +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json new file mode 100644 index 0000000..f4fd5bf --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json @@ -0,0 +1,90 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/releases", + "base_branch": "main" + }, + "candidate": { + "head_sha": "4c960743db89826d0a87dc5b3558bbae8566593c", + "comparison_base_sha": "59d8eb504311bc773d02d2d1975b707672441318", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/promotion/gate.py b/promotion/gate.py\nindex 2cb8a07..01d515d 100644\n--- a/promotion/gate.py\n+++ b/promotion/gate.py\n@@ -1,8 +1,15 @@\n from promotion.model import Artifact\n \n \n-def artifact_promotable(artifact):\n- \"\"\"Return whether this artifact may be treated as promoted.\"\"\"\n+def artifact_promotable(artifact, *, external_proof=None):\n+ \"\"\"Return whether this artifact may be treated as promoted.\n+\n+ With `external_proof`, only a ledger-confirmed record counts. The\n+ promotion marker alone is written by the release tool and can be present\n+ on an artifact whose release announcement was never confirmed.\n+ \"\"\"\n+ if external_proof is not None:\n+ return bool(external_proof.verified)\n return bool(artifact.promotion_marker or artifact.external_ref)\n \n \n@@ -12,5 +19,10 @@ def hold_release(record):\n \n def decide_promotion(artifact, ledger):\n for record in [artifact]:\n- if not artifact_promotable(record):\n+ proof = (\n+ ledger.lookup(record.external_ref)\n+ if record.verification_required\n+ else None\n+ )\n+ if not artifact_promotable(record, external_proof=proof):\n hold_release(record)\ndiff --git a/tests/test_promotion_gate.py b/tests/test_promotion_gate.py\nindex 16ed52a..985aa13 100644\n--- a/tests/test_promotion_gate.py\n+++ b/tests/test_promotion_gate.py\n@@ -8,3 +8,16 @@ def test_artifact_promotable_accepts_a_promotion_marker():\n \n def test_artifact_promotable_accepts_an_external_ref():\n assert artifact_promotable(Artifact(external_ref=\"rel-9\"))\n+\n+\n+def test_artifact_promotable_rejects_a_marker_only_artifact_under_ledger_proof():\n+ artifact = Artifact(promotion_marker=True, external_ref=None)\n+ assert not artifact_promotable(artifact, external_proof=UnverifiedProof())\n+\n+\n+def test_decide_promotion_holds_a_verification_required_artifact_without_ledger_proof():\n+ artifact = Artifact(\n+ verification_required=True, promotion_marker=True, external_ref=None\n+ )\n+ decide_promotion(artifact, ledger=EmptyLedger())\n+ assert artifact.held\n" + } + }, + "change_contract": { + "goal": "Stop treating an artifact as promoted on the strength of its promotion marker alone once that artifact requires mandatory verification, because the marker is written by the release tool and can be present before the ledger confirms the release actually landed.", + "acceptance_criteria": [ + "An artifact that requires mandatory verification is only treated as promoted when the ledger confirms proof.", + "An artifact that does not require mandatory verification keeps today's marker/external-ref check." + ], + "non_goals": [ + "Change how the ledger records proof.", + "Change staging/rollout default selection." + ], + "preserved_behaviors": [ + "`artifact_promotable` keeps its permissive result when verification is not required.", + "`decide_promotion` keeps its existing signature." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Verification rules", + "location": "AGENTS.md", + "summary": "Once an artifact requires mandatory verification, only ledger-confirmed proof may satisfy a promotion decision; the promotion marker alone is not sufficient once that requirement applies." + } + ], + "named_documents": [ + { + "label": "Promotion proof contract", + "location": "docs/promotion.md" + }, + { + "label": "Artifact model", + "location": "promotion/model.py" + } + ], + "nearby_patterns": [ + { + "label": "Ledger client", + "location": "promotion/ledger.py" + }, + { + "label": "Rollout defaults", + "location": "rollout/targeting.py" + }, + { + "label": "Promotion gate tests", + "location": "tests/test_promotion_gate.py" + } + ] + }, + "validation": [ + { + "name": "promotion gate tests", + "command": "pytest tests/test_promotion_gate.py", + "scope": "focused", + "status": "passed", + "result": "7 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "146 passed" + } + ], + "context": { + "data": [ + "`promotion/gate.py` and `rollout/targeting.py` are the only two modules that call `artifact_promotable`. This diff does not touch `rollout/targeting.py`, whose relevant part is:\n\n def _pick_rollout_defaults(artifact):\n \"\"\"Choose default staging parameters for an artifact entering the environment.\"\"\"\n if not artifact_promotable(artifact):\n return STAGED_DEFAULTS\n return PROMOTED_DEFAULTS\n\n def resolve_rollout_environment(artifact):\n \"\"\"Return the environment config used when a worker enters an artifact into staging.\"\"\"\n defaults = _pick_rollout_defaults(artifact)\n return build_environment(defaults, artifact)", + "`resolve_rollout_environment` is called by the environment-selection worker whenever an artifact enters staging.", + "An artifact with `verification_required=True` can still carry a `promotion_marker` from the release tool whenever the tool ran before the ledger recorded confirmation; this happens whenever a release is announced before its ledger entry settles." + ], + "operational": [ + "The environment-selection worker runs once per staging entry and does not re-evaluate later; whatever defaults it assigns at entry persist for the artifact's whole staging window." + ] + } +} diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/audit-log-flush-keyword-probe/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/audit-log-flush-keyword-probe/packet.json new file mode 100644 index 0000000..6aa8743 --- /dev/null +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/audit-log-flush-keyword-probe/packet.json @@ -0,0 +1,86 @@ +{ + "schema_version": "1.0", + "repository": { + "identity": "example/ledger", + "base_branch": "main" + }, + "candidate": { + "head_sha": "5c1e4a2f7b09d836ac41f6205e7d918cb4a3e670", + "comparison_base_sha": "a870f3d6c4e19b52d087a56123fce8901ba7d4e2", + "diff": { + "format": "unified_diff", + "complete": true, + "content": "diff --git a/audit/flush.py b/audit/flush.py\nindex 62eeba1..8d98aa0 100644\n--- a/audit/flush.py\n+++ b/audit/flush.py\n@@ -8,8 +8,13 @@ def _render_script(entry, version):\n \n \n def flush_entry(entry, session):\n- \"\"\"Persist one audit entry.\"\"\"\n- script = _render_script(entry, entry.expected_version)\n+ \"\"\"Persist one audit entry, retrying once on a transient session error.\"\"\"\n+ version = entry.expected_version\n+ script = _render_script(entry, version)\n outcome = session.run_script(script)\n+ if outcome.status == \"transient_error\":\n+ # Retry with the same rendered script; the entry itself has not\n+ # changed, so resending it is safe.\n+ outcome = session.run_script(script)\n entry.flushed = outcome.status == \"ok\"\n return entry.flushed\ndiff --git a/tests/test_audit_flush.py b/tests/test_audit_flush.py\nindex 7cc6c94..882c430 100644\n--- a/tests/test_audit_flush.py\n+++ b/tests/test_audit_flush.py\n@@ -7,3 +7,25 @@ def test_flush_entry_persists_a_new_entry():\n session = _AlwaysOkSession()\n assert flush_entry(entry, session)\n assert entry.flushed\n+\n+\n+class _FakeSession:\n+ \"\"\"Simulates one transient error, then reports the script's own shape.\"\"\"\n+\n+ def __init__(self):\n+ self.calls = 0\n+\n+ def run_script(self, script):\n+ self.calls += 1\n+ if self.calls == 1:\n+ return _Outcome(status=\"transient_error\")\n+ if \"COMMIT\" in script:\n+ return _Outcome(status=\"ok\")\n+ return _Outcome(status=\"error\")\n+\n+\n+def test_flush_entry_retries_after_a_transient_error():\n+ entry = Entry(entry_id=\"e-1\", payload=\"new\", expected_version=3)\n+ session = _FakeSession()\n+ assert flush_entry(entry, session) is True\n+ assert entry.flushed is True\n" + } + }, + "change_contract": { + "goal": "Stop a transient session error from permanently losing an audit entry by retrying the flush once, while keeping the retry safe under the same optimistic-concurrency precondition the store already enforces.", + "acceptance_criteria": [ + "A flush that hits one transient session error retries and, once persisted, is recorded as flushed.", + "A retried flush re-derives its script from a fresh read of the entry's current version before resending, since a concurrent flush may have advanced the version between attempts." + ], + "non_goals": [ + "Change the number of retries beyond one.", + "Change how a permanent (non-transient) session error is handled." + ], + "preserved_behaviors": [ + "`flush_entry` keeps its existing signature.", + "A flush with no transient error still persists on the first attempt." + ] + }, + "sources": { + "repository_instructions": [ + { + "label": "Concurrency rules", + "location": "AGENTS.md", + "summary": "A retried write must re-verify every precondition it captured before the retry, because another writer may have acted in between; resending an already-built request is only safe when nothing it depends on could have changed." + } + ], + "named_documents": [ + { + "label": "Audit entry contract", + "location": "docs/audit-entries.md" + }, + { + "label": "Session boundary", + "location": "audit/session.py" + } + ], + "nearby_patterns": [ + { + "label": "Script rendering", + "location": "audit/flush.py" + }, + { + "label": "Flush tests", + "location": "tests/test_audit_flush.py" + } + ] + }, + "validation": [ + { + "name": "flush tests", + "command": "pytest tests/test_audit_flush.py", + "scope": "focused", + "status": "passed", + "result": "6 passed" + }, + { + "name": "full tests", + "command": "pytest", + "scope": "full", + "status": "passed", + "result": "132 passed" + } + ], + "context": { + "data": [ + "`_render_script(entry, version)` builds a single script that both updates the row and commits, guarded by `WHERE version = {version}` for optimistic concurrency; its full body is unchanged by this diff:\n\n def _render_script(entry, version):\n return SCRIPT_TEMPLATE.format(\n entry_id=entry.entry_id, payload=entry.payload, version=version\n )", + "The session boundary (`audit/session.py`) reports a script's outcome as `ok` whenever the script runs without a SQL error - including when its `WHERE version = ...` clause matches zero rows. It does not surface a rows-affected count to the caller; a caller cannot tell a real write from a no-op write from `outcome.status` alone.", + "A concurrent flush of the same entry (from a retried worker elsewhere, or a background compaction pass) can advance the row's stored `version` at any time between two calls to `session.run_script`." + ], + "operational": [ + "Audit entries are flushed individually by many worker processes concurrently; a transient session error is common enough under load that the retry path runs routinely, not only in rare failure modes." + ] + } +} diff --git a/review-suite/evals/v2/discriminating-case-validation-frozen-configuration.md b/review-suite/evals/v2/discriminating-case-validation-frozen-configuration.md new file mode 100644 index 0000000..fcf11d7 --- /dev/null +++ b/review-suite/evals/v2/discriminating-case-validation-frozen-configuration.md @@ -0,0 +1,131 @@ +# Frozen configuration for the #89 discriminating-case validation + +This freezes the configuration for #89's own validation run, before any of its +scored output is examined, matching every prior measurement task in this epic. +It does not modify `S1-ABLATION-MATRIX.md`, `FROZEN-V2-CONFIGURATION.md`, +`CLOSEOUT-REPORT.md`, `gate-manifest.json`, `DECISION-RECORD.md`, +`FAILURE-TAXONOMY.md`, `audits/`, or `baseline/v1/` - all remain exactly as +prior tickets delivered them. It does not touch `skills/review-correctness/`, +the traversal pass, or the verification-sufficiency pass. + +## What this validates + +#57's ablation matrix (`S1-ABLATION-MATRIX.md`) found that +`dependency-strictness-propagation` and `stale-claim-release-guard` - the sole +cases justifying #52's schema and #53's two passes - resolve at +`mean_combined_recall: 1.0` in every tested configuration, including the one +where the pass meant to fix each case is disabled. Neither case demonstrates +unique causal contribution. #89 sources one new, harder, disguised case per pass +and validates whether either shows a real, reproducible discriminating gap +before treating it as corpus evidence. + +Two new cases, committed under +`review-suite/evals/strata/s1-correctness-orchestrator/` in the same commit as +this record: + +- `artifact-promotion-environment-shortcut` - a harder sibling to + `dependency-strictness-propagation`. The untouched sibling decision path is + two calls deep (`resolve_rollout_environment` -> `_pick_rollout_defaults` -> + `artifact_promotable`) rather than one direct call, and lives in a module + named for staging/rollout defaults, not dependency or promotion checking. +- `audit-log-flush-keyword-probe` - a harder sibling to + `stale-claim-release-guard`. The test insufficiency is carried by a mocked + session that grants success on a keyword match in the request text, rather + than by the test's initial state (the original case's shape), so it is less + visually obvious on a first read. + +Each case ships an executable oracle +(`review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py`, +`review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py`) that +independently confirms the stated root cause: the requirement fails against the +candidate reproduction and holds once corrected. Both pass under +`python3 -m unittest review-suite.scripts.tests.test_eval_oracles`. + +## Validation design: one variable changed per comparison + +Per #89's own instruction, each case is scored once with its owning pass enabled +and once disabled, 5 runs each, and only kept as corpus evidence if a real, +reproducible gap appears. Reusing #57's own three configurations keeps exactly +one pass different between the two runs compared for each case, rather than +comparing configurations that differ in two passes at once: + +| Case | Owning pass | "Enabled" configuration | "Disabled" configuration | What differs | +| ----------------------------------------- | --------------------------- | ----------------------------------------------------- | ------------------------------------------------------- | ---------------------------------- | +| `artifact-promotion-environment-shortcut` | traversal (consumer/impact) | both passes together (real, unablated `skills/` tree) | verification-sufficiency-pass-only (traversal disabled) | traversal pass only | +| `audit-log-flush-keyword-probe` | verification-sufficiency | both passes together (real, unablated `skills/` tree) | traversal-pass-only (verification-sufficiency disabled) | verification-sufficiency pass only | + +Both "disabled" configurations reuse the exact overlay mechanism +`FROZEN-V2-CONFIGURATION.md` established: a fresh local mirror of this +worktree's real `skills/` tree, with `review-correctness/SKILL.md` overwritten +by the byte-identical committed content of +`review-suite/evals/v2/ablation-skill-roots/{traversal-only,verification-only}/review-correctness/SKILL.md`. +Verified byte-identical by direct `diff` immediately before each run. The "both +passes together" configuration is the real, unablated `skills/` tree with no +overlay. + +## Runtime/model stratum (verified immediately before scoring) + +- `runtime`: `claude` (Claude Code CLI) +- `runtime_cli_version`: `2.1.92`, verified via `claude --version` immediately + before scoring - matches `FROZEN-V2-CONFIGURATION.md`'s own pin exactly, so + this run's figures are runtime-comparable to every prior measurement in this + epic. +- `model`: no `--model` flag is passed to `claude_executor.py`, the same + invocation shape every prior ablation run in this epic used; the resolved + model is recorded verbatim in each report's own + `configuration.executor_models` field. + +## Suite commit, corpus, and grader versions + +- `suite_commit`: the commit that adds this record and the two new cases to this + worktree's branch (recorded verbatim in each report's own + `configuration.suite_commit` field; this file is written and committed before + the first scored attempt, so it cannot be edited after seeing a result). +- `corpus_version`: `0.2-s1-populated` (the two new cases added to the existing + `0.1-s1-populated` stratum; the corpus index's own `stratum.purpose` field + records the addition). +- `grader_version`: `1.1` (unchanged; the same fixed grader every scored run in + this epic since `GRADER-1.1-COMPARABILITY.md` uses). +- `protocol_version`: `1.0` (unchanged). + +## Executor and run configuration + +- `executor_command`: `python3 review-suite/scripts/evals/claude_executor.py` +- `runs_per_case`: `5`, matching every prior `s1` measurement in this epic for + comparability. +- `timeout_seconds`: `300`, matching `FROZEN-V2-CONFIGURATION.md`'s pin for its + own two new ablation runs. +- `max_output_bytes`: default (`4,000,000`), unchanged. +- Each run scores its own case in isolation via a scoped ephemeral corpus + (mirroring the confirming-rerun's own method): a fresh local directory + carrying only the one case's `reviewer/`, `private/expectations/`, and + `private/provenance/` files, with `corpus.json`'s other fields carried + verbatim, unedited, from the real stratum corpus committed alongside this + record. This is a targeted validation of one new case per run, not a re-score + of the full (now nine-case) stratum. +- `retry_policy`: none; a failed attempt is an evaluation failure and is never + retried, unchanged from every prior measurement in this epic. + +## Cost ceiling (preregistered before any scored output is examined) + +- **This ticket's own hard ceiling: $5.00.** +- Sizing, per #89's own issue body: two candidate cases x two configurations x 5 + runs is 20 attempts; this stratum's real historical per-attempt cost is + roughly $0.05-0.10 (`S1-ABLATION-MATRIX.md`'s two 35-attempt runs cost $3.560 + and $3.867, or about $0.10-0.11/attempt; the confirming rerun's 5-attempt + single-case run cost $0.569, about $0.114/attempt). Twenty attempts at that + rate is roughly $1.00-$2.30, so $5.00 has generous headroom without being + sized to force a particular outcome. +- No threshold, gate value, or ceiling in this record was adjusted after seeing + any scored output; this record is committed before the first scored attempt + runs. + +## Invalid runs, missing data, and threshold-change discipline + +Unchanged from every prior measurement task in this epic: an attempt classified +`spawn_failure`/`timeout`/`runtime_failure`/`output_too_large`/ +`malformed_output`/`protocol_mismatch` is never graded and never silently +retried. The discriminating-gap determination (a real, reproducible +false-positive or false-negative rate difference between the "enabled" and +"disabled" configuration for the same case) is fixed here, before this ticket +examines a single scored attempt. diff --git a/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py b/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py new file mode 100644 index 0000000..448b978 --- /dev/null +++ b/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py @@ -0,0 +1,137 @@ +"""Oracle: the rollout-environment path still decides promotion permissively. + +Requirement, from the packet's acceptance criteria: every path that decides +whether an artifact counts as promoted applies the same mandatory-verification +rule once verification is required. The check drives the exact artifact shape +the change exists to catch - verification required, carrying a promotion +marker, no ledger proof - through both the hardened gate path and the two-hop +rollout path (`resolve_rollout_environment` -> `_pick_rollout_defaults` -> +`artifact_promotable`), and requires them to agree. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from . import CaseOracle + + +@dataclass +class Artifact: + verification_required: bool = False + promotion_marker: bool = False + external_ref: str | None = None + held: bool = False + + +class _Proof: + def __init__(self, verified: bool): + self.verified = verified + + +class _Ledger: + """Confirms only the artifact whose external ref names a settled release.""" + + def lookup(self, external_ref): + if external_ref is None: + return None + return _Proof(verified=external_ref == "rel-confirmed") + + +def _artifact_promotable(artifact: Artifact, *, external_proof=None) -> bool: + if external_proof is not None: + return bool(external_proof.verified) + return bool(artifact.promotion_marker or artifact.external_ref) + + +class _Candidate: + """`decide_promotion` hardened; the rollout path left exactly as it was.""" + + @staticmethod + def gate_decides_promoted(artifact: Artifact, ledger: _Ledger) -> bool: + proof = ( + ledger.lookup(artifact.external_ref) + if artifact.verification_required + else None + ) + return _artifact_promotable(artifact, external_proof=proof) + + @staticmethod + def rollout_decides_promoted(artifact: Artifact) -> bool: + # `_pick_rollout_defaults`, unmodified: no `external_proof` is ever passed. + return _artifact_promotable(artifact) + + +class _Corrected: + """Both paths adopt the ledger-proof requirement.""" + + @staticmethod + def gate_decides_promoted(artifact: Artifact, ledger: _Ledger) -> bool: + proof = ( + ledger.lookup(artifact.external_ref) + if artifact.verification_required + else None + ) + return _artifact_promotable(artifact, external_proof=proof) + + @staticmethod + def rollout_decides_promoted(artifact: Artifact) -> bool: + ledger = _Ledger() + proof = ( + ledger.lookup(artifact.external_ref) + if artifact.verification_required + else None + ) + return _artifact_promotable(artifact, external_proof=proof) + + +def _decide_promotion(subject, artifact: Artifact, ledger: _Ledger) -> None: + if not subject.gate_decides_promoted(artifact, ledger): + artifact.held = True + + +def _check(subject) -> bool: + ledger = _Ledger() + unverified = Artifact( + verification_required=True, promotion_marker=True, external_ref="rel-9" + ) + verified = Artifact( + verification_required=True, + promotion_marker=True, + external_ref="rel-confirmed", + ) + not_required = Artifact( + verification_required=False, promotion_marker=True, external_ref=None + ) + + _decide_promotion(subject, unverified, ledger) + _decide_promotion(subject, verified, ledger) + + return ( + # The two decision paths must agree on the unverified artifact. + subject.rollout_decides_promoted(unverified) + == subject.gate_decides_promoted(unverified, ledger) + # ...and must agree the answer is "not promoted". + and subject.gate_decides_promoted(unverified, ledger) is False + # A genuinely ledger-confirmed artifact stays promoted on both paths. + and subject.rollout_decides_promoted(verified) is True + # An artifact that does not require verification keeps the permissive + # behaviour the contract preserves. + and subject.rollout_decides_promoted(not_required) is True + # The unverified artifact is the one the gate must hold. + and unverified.held is True + and verified.held is False + ) + + +ORACLE = CaseOracle( + case_id="artifact-promotion-environment-shortcut", + requirement=( + "Every path that decides whether an artifact counts as promoted " + "applies the same mandatory-verification rule once verification is " + "required." + ), + candidate=_Candidate, + corrected=_Corrected, + check=_check, +) diff --git a/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py b/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py new file mode 100644 index 0000000..5ca8402 --- /dev/null +++ b/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py @@ -0,0 +1,133 @@ +"""Oracle: a retried flush resends its already-rendered, now-stale script. + +Requirement, from the packet's acceptance criteria: a retried flush re-derives +its script from a fresh read of the entry's current version before resending. +The check drives the exact interleaving the change exists to prevent - a +transient session error on the first attempt, then a concurrent flush +advancing the row's version before the retry runs - and asks whether the entry +is recorded as flushed only when it was actually persisted. + +The session boundary modelled here never tells its caller how many rows a +script's `WHERE version = ...` clause matched (`status` is `ok` for any +script that runs without a SQL error, matched row or not), matching the +packet's own stated fact about `audit/session.py`. Only this oracle - not the +production code under review - is allowed to peek at the underlying row to +settle whether persistence genuinely happened. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from . import CaseOracle + + +@dataclass +class Entry: + entry_id: str + payload: str + expected_version: int + flushed: bool = False + + +class _Row: + def __init__(self, version: int, payload: str): + self.version = version + self.payload = payload + + +class _Store: + """An `UPDATE ... WHERE version = ...` that matches zero rows is a no-op, + but the session still reports the script as having run successfully.""" + + def __init__(self, version: int, payload: str): + self.row = _Row(version, payload) + + def apply(self, payload: str, expected_version: int) -> None: + if self.row.version == expected_version: + self.row.version += 1 + self.row.payload = payload + + +class _Outcome: + def __init__(self, status: str): + self.status = status + + +class _Session: + """Reports one transient error on the first call, then always `ok` - + the session boundary never surfaces whether a row actually matched.""" + + def __init__(self, store: _Store, on_first_call=None): + self._store = store + self._calls = 0 + self._on_first_call = on_first_call + + def run_script(self, payload: str, expected_version: int) -> _Outcome: + self._calls += 1 + if self._calls == 1: + if self._on_first_call: + self._on_first_call() + return _Outcome(status="transient_error") + self._store.apply(payload, expected_version) + return _Outcome(status="ok") + + +def _flush_candidate(entry: Entry, session: _Session, fresh_version_reader) -> bool: + """Candidate: retries with the same, already-captured version.""" + version = entry.expected_version + outcome = session.run_script(entry.payload, version) + if outcome.status == "transient_error": + outcome = session.run_script(entry.payload, version) + entry.flushed = outcome.status == "ok" + return entry.flushed + + +def _flush_corrected(entry: Entry, session: _Session, fresh_version_reader) -> bool: + """Corrected: re-reads the current version before resending.""" + version = entry.expected_version + outcome = session.run_script(entry.payload, version) + if outcome.status == "transient_error": + version = fresh_version_reader() + outcome = session.run_script(entry.payload, version) + entry.flushed = outcome.status == "ok" + return entry.flushed + + +@dataclass +class _Result: + entry: Entry + store: _Store + + +def _run(flush) -> _Result: + store = _Store(version=3, payload="old") + + def concurrent_write() -> None: + # Another flush lands while the first attempt is erroring out. + store.row.version += 1 + store.row.payload = "concurrent-write" + + session = _Session(store, on_first_call=concurrent_write) + entry = Entry(entry_id="e-1", payload="new", expected_version=3) + flush(entry, session, lambda: store.row.version) + return _Result(entry=entry, store=store) + + +def _check(result: _Result) -> bool: + if not result.entry.flushed: + return False + # Flushed must mean actually persisted, not merely "the script ran". + return result.store.row.payload == result.entry.payload + + +ORACLE = CaseOracle( + case_id="audit-log-flush-keyword-probe", + requirement=( + "A retried flush re-derives its script from a fresh read of the " + "entry's current version before resending." + ), + candidate=lambda: _run(_flush_candidate), + corrected=lambda: _run(_flush_corrected), + check=_check, +) From 5e9b3de63335e23d80781a85de49c43c231d9d07 Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Wed, 29 Jul 2026 23:35:51 -0700 Subject: [PATCH 2/3] docs(review-suite): validate the two new discriminating cases and report an asymmetric result (#89) - Fix a construction defect found in `artifact-promotion-environment-shortcut` during its first scored run: the packet never stated `ledger.lookup`'s contract for a missing external ref, so 4/5 verification-only attempts raised an unrelated `None`-fallthrough concern instead of the intended rollout-path finding. State the contract explicitly in the packet and correct the oracle's `_Ledger` to match; re-mint the case's `head_sha`. - Re-run both traversal-case configurations against the fixed packet: both passes together `mean_combined_recall` 1.0 (5/5); verification-only (traversal disabled) 0.4 (2/5, 3/5 false-clean, zero false positives). A real, reproducible, uncontaminated discriminating gap. - Score `audit-log-flush-keyword-probe` both ways: both together 1.0 (5/5); traversal-only (verification-sufficiency disabled) also 1.0 (5/5). No gap, same pattern as the existing `stale-claim-release-guard` target case. - Record both results, the sourcing/sanitization table, and the cost ledger in `review-suite/evals/v2/discriminating-case-validation.md`, with the four trustworthy raw reports committed alongside it. - #89 requires validating discriminating power before treating either new case as corpus evidence, and reporting honestly even when the answer is "no gap" for one of the two. The traversal pass now has one real demonstrated case; the verification-sufficiency pass still does not, even after a genuine, harder, disguised construction attempt. - $3.6657 total against the preregistered $5.00 ceiling (30 attempts across six scored runs, including 10 discarded pre-fix attempts whose real cost still counts against this ticket's spend even though their output was not used as evidence). --- CHANGELOG.md | 5 + .../packet.json | 5 +- ...tifact-promotion-both-together.report.json | 176 ++++++++++++++ ...ct-promotion-verification-only.report.json | 149 ++++++++++++ ...-audit-log-flush-both-together.report.json | 212 +++++++++++++++++ ...audit-log-flush-traversal-only.report.json | 221 ++++++++++++++++++ .../v2/discriminating-case-validation.md | 199 ++++++++++++++++ ...artifact_promotion_environment_shortcut.py | 9 +- 8 files changed, 971 insertions(+), 5 deletions(-) create mode 100644 review-suite/evals/v2/discriminating-case-artifact-promotion-both-together.report.json create mode 100644 review-suite/evals/v2/discriminating-case-artifact-promotion-verification-only.report.json create mode 100644 review-suite/evals/v2/discriminating-case-audit-log-flush-both-together.report.json create mode 100644 review-suite/evals/v2/discriminating-case-audit-log-flush-traversal-only.report.json create mode 100644 review-suite/evals/v2/discriminating-case-validation.md diff --git a/CHANGELOG.md b/CHANGELOG.md index e2e5159..bf4980e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,14 @@ summary: Chronological history of repository and skill changes. ## 2026-07-29 — Sourced two harder discriminating cases for the traversal and verification-sufficiency passes, designed the review-fix-loop skill, migrated implement-ticket and babysit-pr to consume the final review-result contract, rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect, added connector-outcome curation and promotion tooling, added a skill-root override for mechanism ablation runs, ran the preregistered v2 ablation and integration closeout, and confirmed the session-continuation-summary verification-only regression with an independent rerun +- docs(review-suite): validate the two new discriminating cases with-pass and + without-pass, fixing a construction defect found in the traversal case along + the way, and report the traversal pass discriminates while the + verification-sufficiency pass still does not (#89) - feat(review-suite): source two harder discriminating `s1-correctness-orchestrator` cases for the traversal and verification-sufficiency passes and preregister their validation ceiling (#89) + (`bfec2910a81422df365ddc3ba4c70672a9ebe269`) - docs: design the review-fix-loop skill (`06538e5c097ff8e6ef15b12d5fbf61b3d959abf7`) - docs(review-suite): add a confirming rerun of the session-continuation-summary diff --git a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json index f4fd5bf..a8bab47 100644 --- a/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json +++ b/review-suite/evals/strata/s1-correctness-orchestrator/reviewer/artifact-promotion-environment-shortcut/packet.json @@ -5,7 +5,7 @@ "base_branch": "main" }, "candidate": { - "head_sha": "4c960743db89826d0a87dc5b3558bbae8566593c", + "head_sha": "57a56b3cd4d327512251eb72c62d72bdb8d606f9", "comparison_base_sha": "59d8eb504311bc773d02d2d1975b707672441318", "diff": { "format": "unified_diff", @@ -81,7 +81,8 @@ "data": [ "`promotion/gate.py` and `rollout/targeting.py` are the only two modules that call `artifact_promotable`. This diff does not touch `rollout/targeting.py`, whose relevant part is:\n\n def _pick_rollout_defaults(artifact):\n \"\"\"Choose default staging parameters for an artifact entering the environment.\"\"\"\n if not artifact_promotable(artifact):\n return STAGED_DEFAULTS\n return PROMOTED_DEFAULTS\n\n def resolve_rollout_environment(artifact):\n \"\"\"Return the environment config used when a worker enters an artifact into staging.\"\"\"\n defaults = _pick_rollout_defaults(artifact)\n return build_environment(defaults, artifact)", "`resolve_rollout_environment` is called by the environment-selection worker whenever an artifact enters staging.", - "An artifact with `verification_required=True` can still carry a `promotion_marker` from the release tool whenever the tool ran before the ledger recorded confirmation; this happens whenever a release is announced before its ledger entry settles." + "An artifact with `verification_required=True` can still carry a `promotion_marker` from the release tool whenever the tool ran before the ledger recorded confirmation; this happens whenever a release is announced before its ledger entry settles.", + "`ledger.lookup(external_ref)` always returns a proof object, never `None`: `proof.verified` is `False` whenever there is no external ref or the ledger holds no confirming record for one. A promotion decision never treats a `None` lookup result as \"verification not required\" - the only way `external_proof` is `None` in `artifact_promotable` is when the caller does not pass it at all (verification not required for that artifact)." ], "operational": [ "The environment-selection worker runs once per staging entry and does not re-evaluate later; whatever defaults it assigns at entry persist for the artifact's whole staging window." diff --git a/review-suite/evals/v2/discriminating-case-artifact-promotion-both-together.report.json b/review-suite/evals/v2/discriminating-case-artifact-promotion-both-together.report.json new file mode 100644 index 0000000..d717180 --- /dev/null +++ b/review-suite/evals/v2/discriminating-case-artifact-promotion-both-together.report.json @@ -0,0 +1,176 @@ +{ + "adjudication_required": [ + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "rollout-targeting-bypasses-verification", + "reason": "partial", + "run_number": 1 + }, + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "rollout-targeting-skips-verification", + "reason": "partial", + "run_number": 2 + }, + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "corr.rollout-bypasses-verification-gate", + "reason": "partial", + "run_number": 3 + }, + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "rollout-targeting-skips-verification", + "reason": "partial", + "run_number": 4 + }, + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "rollout-targeting-bypass", + "reason": "partial", + "run_number": 5 + } + ], + "attempts": 5, + "baseline_eligible": true, + "configuration": { + "artifacts_retained": false, + "cases": 1, + "corpus_version": "0.2-s1-populated", + "executor": "python3 review-suite/scripts/evals/claude_executor.py", + "executor_models": [ + "claude-opus-4-6[1m]" + ], + "grader_version": "1.1", + "max_output_bytes": 4000000, + "runs_per_case": 5, + "skill_root": "skills", + "stratum": { + "grading_is_signal": true, + "ground_truth": "human-review", + "id": "s1-correctness-orchestrator", + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss. `0.2` adds two cases (#89): a harder, disguised sibling to `dependency-strictness-propagation` and to `stale-claim-release-guard`, sourced after #57's ablation matrix found neither original target case demonstrates unique causal contribution to the pass meant to fix it.", + "scored": true + }, + "suite_commit": "bfec2910a81422df365ddc3ba4c70672a9ebe269", + "target_skill": "review-code-change", + "target_skill_dependencies": [ + "review-solution-simplicity", + "review-correctness", + "review-code-simplicity" + ], + "target_skill_digest": "f8578e3fcd87a7be", + "target_skill_documents": [ + "review-code-change/SKILL.md", + "review-code-change/references/orchestration-protocol.md", + "review-code-simplicity/SKILL.md", + "review-code-simplicity/references/code-simplicity-rubric.md", + "review-correctness/SKILL.md", + "review-correctness/references/correctness-rubric.md", + "review-solution-simplicity/SKILL.md", + "review-solution-simplicity/references/solution-simplicity-rubric.md" + ], + "timeout_seconds": 300.0 + }, + "failures": { + "blocked_rate": 0.0, + "malformed_output_rate": 0.0, + "output_too_large_rate": 0.0, + "protocol_mismatch_rate": 0.0, + "runtime_failure_rate": 0.0, + "spawn_failure_rate": 0.0, + "timeout_rate": 0.0 + }, + "graded_attempts": 5, + "grader_version": "1.1", + "latency": { + "count": 5, + "max_seconds": 122.69005558360368, + "mean_seconds": 59.48467412488535, + "min_seconds": 38.91936695901677, + "p50_seconds": 47.07643108302727 + }, + "per_case": [ + { + "attempts": 5, + "case_id": "artifact-promotion-environment-shortcut", + "ever_referred_relevant_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "ever_referred_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "expected_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "false_alarm_attempts": 0, + "false_clean_attempts": 0, + "false_positive_attempts": 0, + "finding_stability": 1.0, + "graded_attempts": 5, + "intersection_root_cause_ids": [], + "mean_combined_recall": 1.0, + "mean_recall": 0.0, + "stability_denominator": 5, + "statuses": { + "review_result": 5 + }, + "union_root_cause_ids": [], + "verdict_stability": 1.0 + } + ], + "protocol_version": "1.0", + "quality": { + "false_alarm_denominator": 0, + "false_alarm_rate": null, + "false_clean_denominator": 5, + "false_clean_rate": 0.0, + "false_positive_denominator": 5, + "false_positive_rate": 0.0, + "material_finding_recall": 0.0, + "recall_attempts": 5, + "referred_denominator": 5, + "referred_rate": 1.0, + "unique_finding_contribution": [] + }, + "report_version": "1.0", + "simulation": false, + "stability": { + "mean_finding_stability": 1.0, + "mean_verdict_stability": 1.0, + "per_case_finding_stability": { + "artifact-promotion-environment-shortcut": 1.0 + }, + "per_case_stability_denominator": { + "artifact-promotion-environment-shortcut": 5 + }, + "per_case_verdict_stability": { + "artifact-promotion-environment-shortcut": 1.0 + }, + "stability_denominator": 5 + }, + "usage": { + "available": true, + "reporting_attempts_cost_usd": 5, + "reporting_attempts_input_tokens": 5, + "reporting_attempts_output_tokens": 5, + "total_cost_usd": 0.59849175, + "total_input_tokens": 185045.0, + "total_output_tokens": 14208.0 + } +} diff --git a/review-suite/evals/v2/discriminating-case-artifact-promotion-verification-only.report.json b/review-suite/evals/v2/discriminating-case-artifact-promotion-verification-only.report.json new file mode 100644 index 0000000..dd93733 --- /dev/null +++ b/review-suite/evals/v2/discriminating-case-artifact-promotion-verification-only.report.json @@ -0,0 +1,149 @@ +{ + "adjudication_required": [ + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "rollout-bypass-verification-gate", + "reason": "partial", + "run_number": 1 + }, + { + "candidate_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "case_id": "artifact-promotion-environment-shortcut", + "finding_id": "rollout-targeting-bypasses-verification", + "reason": "partial", + "run_number": 3 + } + ], + "attempts": 5, + "baseline_eligible": true, + "configuration": { + "artifacts_retained": false, + "cases": 1, + "corpus_version": "0.2-s1-populated", + "executor": "python3 review-suite/scripts/evals/claude_executor.py", + "executor_models": [ + "claude-opus-4-6[1m]" + ], + "grader_version": "1.1", + "max_output_bytes": 4000000, + "runs_per_case": 5, + "skill_root": "/private/tmp/claude-501/-Users-scott--claude-worktrees-agent-scripts-epic-49-implementation-eab73d/d2c1ed44-f510-4ae0-8cdd-ed14e9491168/scratchpad/skillroot-verification-only", + "stratum": { + "grading_is_signal": true, + "ground_truth": "human-review", + "id": "s1-correctness-orchestrator", + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss. `0.2` adds two cases (#89): a harder, disguised sibling to `dependency-strictness-propagation` and to `stale-claim-release-guard`, sourced after #57's ablation matrix found neither original target case demonstrates unique causal contribution to the pass meant to fix it.", + "scored": true + }, + "suite_commit": "bfec2910a81422df365ddc3ba4c70672a9ebe269", + "target_skill": "review-code-change", + "target_skill_dependencies": [ + "review-solution-simplicity", + "review-correctness", + "review-code-simplicity" + ], + "target_skill_digest": "6e05fb283aa9ed69", + "target_skill_documents": [ + "review-code-change/SKILL.md", + "review-code-change/references/orchestration-protocol.md", + "review-code-simplicity/SKILL.md", + "review-code-simplicity/references/code-simplicity-rubric.md", + "review-correctness/SKILL.md", + "review-correctness/references/correctness-rubric.md", + "review-solution-simplicity/SKILL.md", + "review-solution-simplicity/references/solution-simplicity-rubric.md" + ], + "timeout_seconds": 300.0 + }, + "failures": { + "blocked_rate": 0.0, + "malformed_output_rate": 0.0, + "output_too_large_rate": 0.0, + "protocol_mismatch_rate": 0.0, + "runtime_failure_rate": 0.0, + "spawn_failure_rate": 0.0, + "timeout_rate": 0.0 + }, + "graded_attempts": 5, + "grader_version": "1.1", + "latency": { + "count": 5, + "max_seconds": 114.89980866620317, + "mean_seconds": 71.93627889184282, + "min_seconds": 37.035000999923795, + "p50_seconds": 62.087140374816954 + }, + "per_case": [ + { + "attempts": 5, + "case_id": "artifact-promotion-environment-shortcut", + "ever_referred_relevant_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "ever_referred_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "expected_root_cause_ids": [ + "rc.rollout-path-keeps-marker-only-promotion-check" + ], + "false_alarm_attempts": 0, + "false_clean_attempts": 3, + "false_positive_attempts": 0, + "finding_stability": 1.0, + "graded_attempts": 5, + "intersection_root_cause_ids": [], + "mean_combined_recall": 0.4, + "mean_recall": 0.0, + "stability_denominator": 5, + "statuses": { + "review_result": 5 + }, + "union_root_cause_ids": [], + "verdict_stability": 0.6 + } + ], + "protocol_version": "1.0", + "quality": { + "false_alarm_denominator": 0, + "false_alarm_rate": null, + "false_clean_denominator": 5, + "false_clean_rate": 0.6, + "false_positive_denominator": 5, + "false_positive_rate": 0.0, + "material_finding_recall": 0.0, + "recall_attempts": 5, + "referred_denominator": 5, + "referred_rate": 0.4, + "unique_finding_contribution": [] + }, + "report_version": "1.0", + "simulation": false, + "stability": { + "mean_finding_stability": 1.0, + "mean_verdict_stability": 0.6, + "per_case_finding_stability": { + "artifact-promotion-environment-shortcut": 1.0 + }, + "per_case_stability_denominator": { + "artifact-promotion-environment-shortcut": 5 + }, + "per_case_verdict_stability": { + "artifact-promotion-environment-shortcut": 0.6 + }, + "stability_denominator": 5 + }, + "usage": { + "available": true, + "reporting_attempts_cost_usd": 5, + "reporting_attempts_input_tokens": 5, + "reporting_attempts_output_tokens": 5, + "total_cost_usd": 0.6463319999999999, + "total_input_tokens": 185130.0, + "total_output_tokens": 16116.0 + } +} diff --git a/review-suite/evals/v2/discriminating-case-audit-log-flush-both-together.report.json b/review-suite/evals/v2/discriminating-case-audit-log-flush-both-together.report.json new file mode 100644 index 0000000..e387d17 --- /dev/null +++ b/review-suite/evals/v2/discriminating-case-audit-log-flush-both-together.report.json @@ -0,0 +1,212 @@ +{ + "adjudication_required": [ + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "correctness.stale-version-retry", + "reason": "partial", + "run_number": 1 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "correctness.test-does-not-exercise-stale-version", + "reason": "partial", + "run_number": 1 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "stale-version-on-retry", + "reason": "partial", + "run_number": 2 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "test-does-not-exercise-stale-version-risk", + "reason": "partial", + "run_number": 2 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "stale-version-retry", + "reason": "partial", + "run_number": 3 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.stale-script-retry", + "reason": "partial", + "run_number": 4 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.test-does-not-exercise-concurrent-version", + "reason": "partial", + "run_number": 4 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "stale-script-retry", + "reason": "partial", + "run_number": 5 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "test-does-not-exercise-stale-version-risk", + "reason": "partial", + "run_number": 5 + } + ], + "attempts": 5, + "baseline_eligible": true, + "configuration": { + "artifacts_retained": false, + "cases": 1, + "corpus_version": "0.2-s1-populated", + "executor": "python3 review-suite/scripts/evals/claude_executor.py", + "executor_models": [ + "claude-opus-4-6[1m]" + ], + "grader_version": "1.1", + "max_output_bytes": 4000000, + "runs_per_case": 5, + "skill_root": "skills", + "stratum": { + "grading_is_signal": true, + "ground_truth": "human-review", + "id": "s1-correctness-orchestrator", + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss. `0.2` adds two cases (#89): a harder, disguised sibling to `dependency-strictness-propagation` and to `stale-claim-release-guard`, sourced after #57's ablation matrix found neither original target case demonstrates unique causal contribution to the pass meant to fix it.", + "scored": true + }, + "suite_commit": "bfec2910a81422df365ddc3ba4c70672a9ebe269", + "target_skill": "review-code-change", + "target_skill_dependencies": [ + "review-solution-simplicity", + "review-correctness", + "review-code-simplicity" + ], + "target_skill_digest": "f8578e3fcd87a7be", + "target_skill_documents": [ + "review-code-change/SKILL.md", + "review-code-change/references/orchestration-protocol.md", + "review-code-simplicity/SKILL.md", + "review-code-simplicity/references/code-simplicity-rubric.md", + "review-correctness/SKILL.md", + "review-correctness/references/correctness-rubric.md", + "review-solution-simplicity/SKILL.md", + "review-solution-simplicity/references/solution-simplicity-rubric.md" + ], + "timeout_seconds": 300.0 + }, + "failures": { + "blocked_rate": 0.0, + "malformed_output_rate": 0.0, + "output_too_large_rate": 0.0, + "protocol_mismatch_rate": 0.0, + "runtime_failure_rate": 0.0, + "spawn_failure_rate": 0.0, + "timeout_rate": 0.0 + }, + "graded_attempts": 5, + "grader_version": "1.1", + "latency": { + "count": 5, + "max_seconds": 87.47376416716725, + "mean_seconds": 72.95093940002843, + "min_seconds": 52.83206895785406, + "p50_seconds": 75.30209245812148 + }, + "per_case": [ + { + "attempts": 5, + "case_id": "audit-log-flush-keyword-probe", + "ever_referred_relevant_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "ever_referred_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "expected_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "false_alarm_attempts": 0, + "false_clean_attempts": 0, + "false_positive_attempts": 0, + "finding_stability": 1.0, + "graded_attempts": 5, + "intersection_root_cause_ids": [], + "mean_combined_recall": 1.0, + "mean_recall": 0.0, + "stability_denominator": 5, + "statuses": { + "review_result": 5 + }, + "union_root_cause_ids": [], + "verdict_stability": 1.0 + } + ], + "protocol_version": "1.0", + "quality": { + "false_alarm_denominator": 0, + "false_alarm_rate": null, + "false_clean_denominator": 5, + "false_clean_rate": 0.0, + "false_positive_denominator": 5, + "false_positive_rate": 0.0, + "material_finding_recall": 0.0, + "recall_attempts": 5, + "referred_denominator": 5, + "referred_rate": 1.0, + "unique_finding_contribution": [] + }, + "report_version": "1.0", + "simulation": false, + "stability": { + "mean_finding_stability": 1.0, + "mean_verdict_stability": 1.0, + "per_case_finding_stability": { + "audit-log-flush-keyword-probe": 1.0 + }, + "per_case_stability_denominator": { + "audit-log-flush-keyword-probe": 5 + }, + "per_case_verdict_stability": { + "audit-log-flush-keyword-probe": 1.0 + }, + "stability_denominator": 5 + }, + "usage": { + "available": true, + "reporting_attempts_cost_usd": 5, + "reporting_attempts_input_tokens": 5, + "reporting_attempts_output_tokens": 5, + "total_cost_usd": 0.627549, + "total_input_tokens": 184110.0, + "total_output_tokens": 15432.0 + } +} diff --git a/review-suite/evals/v2/discriminating-case-audit-log-flush-traversal-only.report.json b/review-suite/evals/v2/discriminating-case-audit-log-flush-traversal-only.report.json new file mode 100644 index 0000000..4a8b59a --- /dev/null +++ b/review-suite/evals/v2/discriminating-case-audit-log-flush-traversal-only.report.json @@ -0,0 +1,221 @@ +{ + "adjudication_required": [ + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.stale-script-on-retry", + "reason": "partial", + "run_number": 1 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.test-does-not-exercise-version-rederivation", + "reason": "partial", + "run_number": 1 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.stale-version-retry", + "reason": "partial", + "run_number": 2 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.test-does-not-exercise-stale-version", + "reason": "partial", + "run_number": 2 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.stale-script-retry", + "reason": "partial", + "run_number": 3 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.test-missing-concurrent-version-advance", + "reason": "partial", + "run_number": 3 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.stale-version-retry", + "reason": "partial", + "run_number": 4 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.test-does-not-exercise-version-rederivation", + "reason": "partial", + "run_number": 4 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.stale-retry-script", + "reason": "partial", + "run_number": 5 + }, + { + "candidate_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "case_id": "audit-log-flush-keyword-probe", + "finding_id": "corr.test-does-not-exercise-stale-version", + "reason": "partial", + "run_number": 5 + } + ], + "attempts": 5, + "baseline_eligible": true, + "configuration": { + "artifacts_retained": false, + "cases": 1, + "corpus_version": "0.2-s1-populated", + "executor": "python3 review-suite/scripts/evals/claude_executor.py", + "executor_models": [ + "claude-opus-4-6[1m]" + ], + "grader_version": "1.1", + "max_output_bytes": 4000000, + "runs_per_case": 5, + "skill_root": "/private/tmp/claude-501/-Users-scott--claude-worktrees-agent-scripts-epic-49-implementation-eab73d/d2c1ed44-f510-4ae0-8cdd-ed14e9491168/scratchpad/skillroot-traversal-only", + "stratum": { + "grading_is_signal": true, + "ground_truth": "human-review", + "id": "s1-correctness-orchestrator", + "purpose": "Correctness and verification cases for the orchestrator target, whose payload therefore carries its three required lens skills. Every case carries the recorded source disposition as its first adjudication and an executable oracle - a runnable module independent of the reviewer's model - as its second. SCORED: both owner-gated inputs are satisfied (the $15 total cost ceiling and complete independent adjudication across all three strata), and every expectation stays `calibrated: false` permanently, per the owner-settled three-way grading method - a scored case is never calibrated on its own prose, and a grader miss surfaces as `referred_root_cause_ids` rather than a silent reviewer-miss. `0.2` adds two cases (#89): a harder, disguised sibling to `dependency-strictness-propagation` and to `stale-claim-release-guard`, sourced after #57's ablation matrix found neither original target case demonstrates unique causal contribution to the pass meant to fix it.", + "scored": true + }, + "suite_commit": "bfec2910a81422df365ddc3ba4c70672a9ebe269", + "target_skill": "review-code-change", + "target_skill_dependencies": [ + "review-solution-simplicity", + "review-correctness", + "review-code-simplicity" + ], + "target_skill_digest": "89046a3b35667858", + "target_skill_documents": [ + "review-code-change/SKILL.md", + "review-code-change/references/orchestration-protocol.md", + "review-code-simplicity/SKILL.md", + "review-code-simplicity/references/code-simplicity-rubric.md", + "review-correctness/SKILL.md", + "review-correctness/references/correctness-rubric.md", + "review-solution-simplicity/SKILL.md", + "review-solution-simplicity/references/solution-simplicity-rubric.md" + ], + "timeout_seconds": 300.0 + }, + "failures": { + "blocked_rate": 0.0, + "malformed_output_rate": 0.0, + "output_too_large_rate": 0.0, + "protocol_mismatch_rate": 0.0, + "runtime_failure_rate": 0.0, + "spawn_failure_rate": 0.0, + "timeout_rate": 0.0 + }, + "graded_attempts": 5, + "grader_version": "1.1", + "latency": { + "count": 5, + "max_seconds": 119.59103174973279, + "mean_seconds": 82.75553960828111, + "min_seconds": 63.294554916676134, + "p50_seconds": 73.73995999991894 + }, + "per_case": [ + { + "attempts": 5, + "case_id": "audit-log-flush-keyword-probe", + "ever_referred_relevant_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "ever_referred_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "expected_root_cause_ids": [ + "rc.retry-reuses-stale-optimistic-concurrency-script" + ], + "false_alarm_attempts": 0, + "false_clean_attempts": 0, + "false_positive_attempts": 0, + "finding_stability": 1.0, + "graded_attempts": 5, + "intersection_root_cause_ids": [], + "mean_combined_recall": 1.0, + "mean_recall": 0.0, + "stability_denominator": 5, + "statuses": { + "review_result": 5 + }, + "union_root_cause_ids": [], + "verdict_stability": 1.0 + } + ], + "protocol_version": "1.0", + "quality": { + "false_alarm_denominator": 0, + "false_alarm_rate": null, + "false_clean_denominator": 5, + "false_clean_rate": 0.0, + "false_positive_denominator": 5, + "false_positive_rate": 0.0, + "material_finding_recall": 0.0, + "recall_attempts": 5, + "referred_denominator": 5, + "referred_rate": 1.0, + "unique_finding_contribution": [] + }, + "report_version": "1.0", + "simulation": false, + "stability": { + "mean_finding_stability": 1.0, + "mean_verdict_stability": 1.0, + "per_case_finding_stability": { + "audit-log-flush-keyword-probe": 1.0 + }, + "per_case_stability_denominator": { + "audit-log-flush-keyword-probe": 5 + }, + "per_case_verdict_stability": { + "audit-log-flush-keyword-probe": 1.0 + }, + "stability_denominator": 5 + }, + "usage": { + "available": true, + "reporting_attempts_cost_usd": 5, + "reporting_attempts_input_tokens": 5, + "reporting_attempts_output_tokens": 5, + "total_cost_usd": 0.67492525, + "total_input_tokens": 184035.0, + "total_output_tokens": 17332.0 + } +} diff --git a/review-suite/evals/v2/discriminating-case-validation.md b/review-suite/evals/v2/discriminating-case-validation.md new file mode 100644 index 0000000..2ebae8a --- /dev/null +++ b/review-suite/evals/v2/discriminating-case-validation.md @@ -0,0 +1,199 @@ +# Discriminating-case validation (#89) + +This validates whether the two new, harder `s1-correctness-orchestrator` cases +sourced for #89 actually discriminate between the relevant pass enabled and +disabled - the question #57's ablation matrix left open for +`dependency-strictness-propagation` and `stale-claim-release-guard`. It does not +modify `S1-ABLATION-MATRIX.md`, `FROZEN-V2-CONFIGURATION.md`, +`CLOSEOUT-REPORT.md`, `gate-manifest.json`, `DECISION-RECORD.md`, +`FAILURE-TAXONOMY.md`, `audits/`, or `baseline/v1/`. It does not modify +`skills/review-correctness/`, the traversal pass, or the verification- +sufficiency pass, and it does not decide #52/#53's disposition - see +`discriminating-case-validation-frozen-configuration.md` for the preregistered +configuration and cost ceiling this run held to. + +## Result, stated plainly up front + +**One candidate discriminates; the other does not.** + +- `artifact-promotion-environment-shortcut` (traversal-pass sibling): a real, + reproducible gap. `mean_combined_recall` 1.0 with the traversal pass enabled + vs. 0.4 with it disabled, 5 runs each, zero evaluation failures either way. +- `audit-log-flush-keyword-probe` (verification-sufficiency-pass sibling): no + gap. `mean_combined_recall` 1.0 in both configurations, 5 runs each, zero + false positives or evaluation failures either way - the same pattern + `S1-ABLATION-MATRIX.md` already reported for `stale-claim-release-guard`. + +This is reported exactly as it came out: an asymmetric result, not forced into a +uniform conclusion in either direction. + +## A defect found and fixed before trusting the traversal-case result + +The first scored run of `artifact-promotion-environment-shortcut` under the +verification-only (traversal-disabled) configuration produced a result that +looked like a discriminating gap by the numbers (`mean_combined_recall` 1.0 -> +0.2) but was not trustworthy evidence: 4 of 5 attempts raised a `blocking` +finding unrelated to the intended root cause, claiming that `decide_promotion`'s +own `ledger.lookup(record.external_ref)` call could return `None` for an +artifact with no `external_ref`, causing `artifact_promotable`'s +`external_proof is not None` check to fall through to the permissive marker-only +path. Reading the raw attempts directly (not just the aggregate) surfaced this: +none of the four false positives said anything about the rollout module at all - +they were entirely about a different, unintended ambiguity in the packet's own +gate function, and none of the five attempts under the real, unablated +configuration raised it even once. + +The packet never stated what `ledger.lookup` returns for a record with no +`external_ref`, so "returns `None`" was a reasonable inference, and the diff's +own added test +(`test_decide_promotion_holds_a_verification_required_artifact_without_ledger_proof`) +only proves the outcome (`artifact.held`), not the mechanism - a reviewer +reading it can't rule out the `None`-fallthrough reading from the diff alone. +This was an integrity gap in the case's construction, not evidence about either +pass, and reporting the contaminated `0.2` figure as "the traversal pass +discriminates" would have been exactly the kind of false-looking-real result +this ticket exists to avoid. + +**Fix applied** (both committed in the same revision, before any further +scoring): the packet's `context.data` now states the `ledger.lookup` contract +explicitly - it always returns a proof object, `verified: False` for a missing +or unconfirmed ref, never bare `None` - and the executable oracle's `_Ledger` +was corrected to match (it previously modelled `lookup(None)` as returning +`None`, inconsistent with the packet's own added test). Both changes are visible +in this same commit's diff, under +`reviewer/artifact-promotion-environment-shortcut/` and +`scripts/evals/oracles/`, and the case's `head_sha` was re-minted to mark the +revision. `python3 -m unittest review-suite.scripts.tests.test_eval_oracles` and +`python3 review-suite/scripts/evals/audit_corpus.py` both pass against the +corrected packet. + +The original (contaminated) run's raw reports were not committed; only the +corrected rerun's reports are retained as evidence, per the table below. + +## Sourcing and sanitization (per #58's discipline, reused exactly) + +Both cases are minimized from real, adjudicated, public review findings in +`shaug/atelier` - the same public source #58's corpus draws from - rewritten +from scratch against fictional subjects, carrying no business logic, domain +identifier, customer context, credential, or hidden reasoning. Neither case was +sourced from, derived from, or informed by the private connector-review source +referenced in #56; that source was not consulted, named, or considered at any +point in sourcing either case. + +| Case | Source | Disposition | Sanitization | +| ----------------------------------------- | -------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `artifact-promotion-environment-shortcut` | `shaug/atelier` PR 315, review comment 2866874599 (owner-authored, public) | Accepted in reply 2866898151 ("Good catch; this is now aligned"), naming the two tests added | Rewritten against a fictional artifact-release system. Untouched sibling path is two calls deep (`resolve_rollout_environment` -> `_pick_rollout_defaults` -> `artifact_promotable`) in a module named for staging/rollout defaults, not dependency/promotion checking - deliberately less salient than `dependency-strictness-propagation`'s one-hop `pipeline/dependencies.py`. | +| `audit-log-flush-keyword-probe` | `shaug/atelier` PR 755, review comment 3031027866 (owner-authored, public) | Accepted in reply 3031075465, describing the tightened regression | Rewritten against a fictional audit-log flush module. Test insufficiency is carried by a mocked session that grants success on a keyword match in the request text, rather than by the test's initial state (the original case's shape) - deliberately less visually obvious than `stale-claim-release-guard`'s owned-entry-only test. | + +Full provenance records (retention authority, adjudication trail, sanitization +statement) are committed at +`review-suite/evals/strata/s1-correctness-orchestrator/private/provenance/artifact-promotion-environment-shortcut.json` +and `.../audit-log-flush-keyword-probe.json`. Each case's materiality is also +independently adjudicated by an executable oracle +(`review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py`, +`.../audit_log_flush_keyword_probe.py`), confirming the stated requirement fails +against the candidate reproduction and holds once corrected - verified by +`python3 -m unittest review-suite.scripts.tests.test_eval_oracles`. + +## Validation runs and figures + +Configuration, runtime, and cost ceiling frozen in +`discriminating-case-validation-frozen-configuration.md` before any scored +output was examined. All four runs below: `grader_version 1.1`, +`corpus_version 0.2-s1-populated`, `runs_per_case 5`, `runtime claude 2.1.92`, +`model claude-opus-4-6[1m]`, zero evaluation failures of any kind in any run. + +### `artifact-promotion-environment-shortcut` (traversal pass) + +| Configuration | `mean_recall` | `mean_combined_recall` | False positives | False-clean | `verdict_stability` | +| -------------------------------------------------------- | ------------- | ---------------------- | --------------- | ----------- | ------------------- | +| Both passes together (traversal enabled, real `skills/`) | 0.0 | **1.0** | 0/5 | 0/5 | 1.0 | +| Verification-only (traversal **disabled**) | 0.0 | **0.4** | 0/5 | 3/5 | 0.6 | + +Raw attempts confirm the shape directly: with the traversal pass, all 5 attempts +named the untouched `_pick_rollout_defaults`/`resolve_rollout_environment` call +chain (referred, surface-relevant). Without it, only 2 of 5 attempts found it; +the other 3 returned `clean`, missing the defect entirely rather than raising it +incorrectly or elsewhere - a clean, uncontaminated miss, unlike the pre-fix run. +Committed raw reports: +[`discriminating-case-artifact-promotion-both-together.report.json`](discriminating-case-artifact-promotion-both-together.report.json), +[`discriminating-case-artifact-promotion-verification-only.report.json`](discriminating-case-artifact-promotion-verification-only.report.json). + +**This is a real, reproducible discriminating gap.** It is the first evidence +anywhere in this epic that the traversal (consumer/impact) pass changes reviewer +behavior on a case built to need it - the two prior target cases +(`dependency-strictness-propagation`, and this same case's own owning +mechanism's other target) never showed this. Sample size is 5 attempts per +configuration; a gap this size (1.0 vs 0.4, 5/5 vs 2/5) is unlikely to be pure +noise but a larger run would sharpen the estimate. + +### `audit-log-flush-keyword-probe` (verification-sufficiency pass) + +| Configuration | `mean_recall` | `mean_combined_recall` | False positives | False-clean | `verdict_stability` | +| ----------------------------------------------------------------------- | ------------- | ---------------------- | --------------- | ----------- | ------------------- | +| Both passes together (verification-sufficiency enabled, real `skills/`) | 0.0 | **1.0** | 0/5 | 0/5 | 1.0 | +| Traversal-only (verification-sufficiency **disabled**) | 0.0 | **1.0** | 0/5 | 0/5 | 1.0 | + +All 10 attempts (5 per configuration) named the retry-reuses-a-stale-script root +cause, referred and surface-relevant, with zero false positives and zero misses +in either configuration. Committed raw reports: +[`discriminating-case-audit-log-flush-both-together.report.json`](discriminating-case-audit-log-flush-both-together.report.json), +[`discriminating-case-audit-log-flush-traversal-only.report.json`](discriminating-case-audit-log-flush-traversal-only.report.json). + +**This candidate does not discriminate**, even after a genuine, disguised-mock +construction and a full validation pass. Reported as a plain null result: +disguising the test-insufficiency shape behind a mocked session rather than the +original case's initial-state shape was not, on this evidence, enough to make +the verification-sufficiency pass's own explicit instruction the deciding factor +\- a capable reviewer already reasons about retry safety and mock fidelity +without being told to run a named "verification-sufficiency" pass, on this case, +at this sample size. + +## What this means for #52/#53 (report only, no disposition decided here) + +- **Traversal pass (consumer/impact-traversal, #52/#53):** now has one real, + demonstrated case behind it - `artifact-promotion-environment-shortcut` shows + a reproducible recall drop (1.0 -> 0.4) when the pass is disabled, unlike + either of its two prior target cases. This does not retroactively validate + `dependency-strictness-propagation` or the schema's original justification in + `DECISION-RECORD.md`; it is new, independent evidence found by deliberately + sourcing a harder case. +- **Verification-sufficiency pass (#53):** still has no case in this corpus + demonstrating unique causal contribution, even after a genuine, disguised + construction specifically designed to be harder than + `stale-claim-release-guard`. Combined with `S1-ABLATION-MATRIX.md`'s existing + finding of a real false-positive regression (`session-continuation-summary`, + 3/5, independently reproduced in the confirming rerun) when this pass runs in + isolation, the evidentiary case for this specific pass, as currently + instructed, is weaker after this validation than before it - a candidate built + to be more likely to need it did not need it. +- Whether to keep, simplify, or remove either pass, and whether to revise + `DECISION-RECORD.md`'s wording, is the repository owner's decision. This + record only reports what a genuine attempt to find harder, discriminating + evidence actually found. + +## Cost ceiling and actual spend + +Preregistered ceiling: **$5.00** (see +`discriminating-case-validation-frozen-configuration.md`). Six scored +real-runtime runs were executed against this ceiling - four trustworthy final +runs plus the two-run pre-fix pass whose result was discarded as contaminated +(its cost still counts against this ticket's own spend, since real money was +spent, even though its output was not used as evidence): + +| Run | Attempts | Cost (USD) | Kept as evidence? | +| ------------------------------------------------------ | -------- | ------------- | ------------------------ | +| Traversal case, both-together (pre-fix, discarded) | 5 | 0.3689225 | No - contaminated packet | +| Traversal case, verification-only (pre-fix, discarded) | 5 | 0.74949 | No - contaminated packet | +| Verification case, both-together | 5 | 0.627549 | Yes | +| Verification case, traversal-only | 5 | 0.67492525 | Yes | +| Traversal case, both-together (post-fix) | 5 | 0.59849175 | Yes | +| Traversal case, verification-only (post-fix) | 5 | 0.6463319999 | Yes | +| **Total** | **30** | **3.6657105** | - | + +**$3.67 spent against a $5.00 ceiling, $1.33 headroom remaining, ceiling never +approached.** The extra 10 discarded-pre-fix attempts (about $1.12) were a real, +disclosed deviation from the original 20-attempt estimate, caused by finding and +fixing a genuine construction defect in the traversal case rather than by any +change in scope; the ceiling still held. No threshold, gate value, or the +ceiling itself was adjusted after seeing any scored output. diff --git a/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py b/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py index 448b978..6ed45d2 100644 --- a/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py +++ b/review-suite/scripts/evals/oracles/artifact_promotion_environment_shortcut.py @@ -30,11 +30,14 @@ def __init__(self, verified: bool): class _Ledger: - """Confirms only the artifact whose external ref names a settled release.""" + """Confirms only the artifact whose external ref names a settled release. + + Always returns a proof object, never `None` - `verified` is `False` for a + missing ref or one the ledger has no confirming record for, matching the + packet's own stated `ledger.lookup` contract. + """ def lookup(self, external_ref): - if external_ref is None: - return None return _Proof(verified=external_ref == "rel-confirmed") From 25cd4c83e3aa08d02b1a9c695b65569a757341ae Mon Sep 17 00:00:00 2001 From: Scott Haug Date: Wed, 29 Jul 2026 23:54:15 -0700 Subject: [PATCH 3/3] fix(review-suite): unify the audit-log-flush oracle's candidate/corrected retry policy (#89) ## Summary - Replace `_flush_candidate`/`_flush_corrected` with one `_flush(..., *, refresh_on_retry: bool)`, mirroring the sibling oracle's own established `guard_unconditionally` flag idiom instead of duplicating the entire retry-policy body across two near-identical functions. ## Why - review-code-change (code-simplicity lens, strong_recommendation): the new oracle repeated its whole policy body across two functions when the directory it lives in already established a single-function-plus-flag shape for this exact kind of candidate/corrected difference (`stale_claim_release_guard.py`). Oracle polarity is unchanged - re-verified by `python3 -m unittest review-suite.scripts.tests.test_eval_oracles`. --- .../oracles/audit_log_flush_keyword_probe.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py b/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py index 5ca8402..e6ef4b0 100644 --- a/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py +++ b/review-suite/scripts/evals/oracles/audit_log_flush_keyword_probe.py @@ -73,22 +73,17 @@ def run_script(self, payload: str, expected_version: int) -> _Outcome: return _Outcome(status="ok") -def _flush_candidate(entry: Entry, session: _Session, fresh_version_reader) -> bool: - """Candidate: retries with the same, already-captured version.""" +def _flush( + entry: Entry, session: _Session, fresh_version_reader, *, refresh_on_retry: bool +) -> bool: + """Retry a transient error once; `refresh_on_retry` selects candidate vs. + corrected, mirroring `stale_claim_release_guard.py`'s `guard_unconditionally` + flag rather than duplicating the policy body across two functions.""" version = entry.expected_version outcome = session.run_script(entry.payload, version) if outcome.status == "transient_error": - outcome = session.run_script(entry.payload, version) - entry.flushed = outcome.status == "ok" - return entry.flushed - - -def _flush_corrected(entry: Entry, session: _Session, fresh_version_reader) -> bool: - """Corrected: re-reads the current version before resending.""" - version = entry.expected_version - outcome = session.run_script(entry.payload, version) - if outcome.status == "transient_error": - version = fresh_version_reader() + if refresh_on_retry: + version = fresh_version_reader() outcome = session.run_script(entry.payload, version) entry.flushed = outcome.status == "ok" return entry.flushed @@ -127,7 +122,15 @@ def _check(result: _Result) -> bool: "A retried flush re-derives its script from a fresh read of the " "entry's current version before resending." ), - candidate=lambda: _run(_flush_candidate), - corrected=lambda: _run(_flush_corrected), + candidate=lambda: _run( + lambda entry, session, reader: _flush( + entry, session, reader, refresh_on_retry=False + ) + ), + corrected=lambda: _run( + lambda entry, session, reader: _flush( + entry, session, reader, refresh_on_retry=True + ) + ), check=_check, )