Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,17 @@ summary: Chronological history of repository and skill changes.

# Changelog

## 2026-07-29 — Rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect
## 2026-07-29 — Migrated implement-ticket and babysit-pr to consume the final review-result contract, and rechecked the s2/s3 strata under grader 1.1 for the same surface-in-prose defect

- docs: fix stale CHANGELOG SHAs left by the main rebase
- docs: backfill the CHANGELOG entry for the review_gate.py canonicalization fix
(`e2310bff8cc9c3a38b690a57844436d5357fa471`)
- fix: canonicalize review_gate.py through the existing sync-contracts mechanism
(`161424571551676c5e8009c2de2c2a102ab7c305`)
- feat: migrate implement-ticket and babysit-pr to the schema 1.3 review-result
contract (`016ffaa826dddf72a822e555796827a396a4041f`)
- docs(review-suite): recheck s2/s3 strata under grader 1.1 for the same
surface-in-prose defect
surface-in-prose defect (`7cf4a3b3fe3dd38f3d1a9da2e6ab82058a77f064`)

## 2026-07-28 — Added correctness traversal and verification-sufficiency passes, consumer/impact-traversal evidence, and required passing validation and current-head lens evidence for a clean review verdict

Expand Down
15 changes: 12 additions & 3 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ md_targets := "."
list-skills:
@find {{skills_dir}} -mindepth 1 -maxdepth 1 -type d -print

# Refresh the review-suite contract copies bundled into each review skill so
# the skills stay self-contained when installed outside this repository.
# Refresh the review-suite contract copies bundled into each review skill and
# each caller that consumes a review-code-change result, so every skill stays
# self-contained when installed outside this repository.
sync-contracts:
@for skill in review-code-change review-correctness review-code-simplicity review-solution-simplicity; do \
@for skill in review-code-change review-correctness review-code-simplicity review-solution-simplicity implement-ticket babysit-pr; do \
dest="{{skills_dir}}/$skill/references/review-suite"; \
mkdir -p "$dest"; \
cp review-suite/CONTRACT.md "$dest/CONTRACT.md"; \
Expand All @@ -20,6 +21,14 @@ sync-contracts:
cp review-suite/scripts/validate.py "$dest/validate.py"; \
echo "Synced $dest"; \
done
@for skill in implement-ticket babysit-pr; do \
scripts_dest="{{skills_dir}}/$skill/scripts"; \
tests_dest="$scripts_dest/tests"; \
mkdir -p "$tests_dest"; \
cp review-suite/scripts/review_gate.py "$scripts_dest/review_gate.py"; \
cp review-suite/scripts/tests/test_review_gate.py "$tests_dest/test_review_gate.py"; \
echo "Synced $scripts_dest/review_gate.py and $tests_dest/test_review_gate.py"; \
done

test: test-plugins
@found=0; \
Expand Down
135 changes: 135 additions & 0 deletions review-suite/scripts/review_gate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
#!/usr/bin/env python3
"""Reject an untrustworthy `review-code-change` aggregate result.

The bundled `references/review-suite/validate.py` enforces the shared review
result schema and cross-field semantics (stale/unsupported schema versions,
malformed shape, verdict/evidence consistency, and aggregate-clean lens
execution completeness). This module adds the one check that schema alone
cannot make: binding the result to *this* run's exact current candidate.

This is deliberately a thin consumption check, not a reviewer. It never
sequences lenses, explores evidence, or decides what a clean review means —
those stay owned by repository-owned `review-code-change`. It only refuses to
let this caller treat a stale, malformed, non-aggregate, non-clean, or
wrongly-bound result as publishable evidence.
"""

from __future__ import annotations

import argparse
import importlib.util
import json
import sys
from pathlib import Path
from typing import Any


def _validate_module_path() -> Path:
"""Locate the bundled `validate.py` in either supported layout.

Installed layout (each consuming skill): `scripts/review_gate.py` beside
`references/review-suite/validate.py`. Canonical layout (this monorepo):
`review-suite/scripts/review_gate.py` beside `review-suite/scripts/
validate.py`, in the same directory as this file.
"""
here = Path(__file__).resolve().parent
for candidate in (
here.parent / "references" / "review-suite" / "validate.py",
here / "validate.py",
):
if candidate.is_file():
return candidate
raise FileNotFoundError(f"Cannot locate validate.py near {here}")


VALIDATE_SPEC = importlib.util.spec_from_file_location(
"caller_review_suite_validate", _validate_module_path()
)
assert VALIDATE_SPEC and VALIDATE_SPEC.loader
VALIDATE = importlib.util.module_from_spec(VALIDATE_SPEC)
VALIDATE_SPEC.loader.exec_module(VALIDATE)


def evaluate_aggregate(
result: dict[str, Any], expected_head: str, expected_base: str
) -> list[str]:
"""Return rejection reasons for `result`; empty means accept as evidence.

`result` must be a schema-valid aggregate `clean` result whose candidate
and every fresh lens execution are bound to `expected_head`/
`expected_base` — the exact head and comparison base this caller captured
for its current candidate.
"""
errors = [f"schema: {error}" for error in VALIDATE.validate_result(result)]
if errors:
# A schema-level rejection (stale version, malformed shape, verdict
# contradictions, incomplete lens executions) already explains why
# the result is untrustworthy; do not layer confusing candidate-binding
# errors on top of a document that isn't even shape-valid.
return errors

if result.get("lens") != "aggregate":
errors.append(f"lens: expected an aggregate result, got {result.get('lens')!r}")
if result.get("verdict") != "clean":
errors.append(
f"verdict: expected clean, got {result.get('verdict')!r}; "
"changes_required and blocked results cannot be consumed as "
"publishable evidence"
)

candidate = result.get("candidate") or {}
if (
candidate.get("head_sha") != expected_head
or candidate.get("comparison_base_sha") != expected_base
):
errors.append(
"candidate: result is not bound to the current candidate "
f"(expected head {expected_head} / base {expected_base}, got "
f"head {candidate.get('head_sha')!r} / "
f"base {candidate.get('comparison_base_sha')!r})"
)

for execution in result.get("lens_executions") or []:
if not isinstance(execution, dict):
continue
if (
execution.get("head_sha") != expected_head
or execution.get("comparison_base_sha") != expected_base
):
errors.append(
f"lens_executions: {execution.get('lens')!r} execution is not "
"bound to the current candidate"
)

return errors


def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("result", type=Path, help="review-code-change result JSON")
parser.add_argument("--head", required=True, help="current captured head SHA")
parser.add_argument(
"--base", required=True, help="current captured comparison-base SHA"
)
args = parser.parse_args()

try:
result = json.loads(args.result.read_text())
except (OSError, json.JSONDecodeError) as error:
print(f"{args.result}: {error}", file=sys.stderr)
return 2
if not isinstance(result, dict):
print(f"{args.result}: top-level JSON value must be an object", file=sys.stderr)
return 2

errors = evaluate_aggregate(result, args.head, args.base)
if errors:
for error in errors:
print(error, file=sys.stderr)
return 1
print(f"clean current-candidate aggregate: {args.result}")
return 0


if __name__ == "__main__":
raise SystemExit(main())
44 changes: 40 additions & 4 deletions review-suite/scripts/tests/test_bundled_contracts.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
"""Verify skill-bundled review-suite contract copies match the canonical source.

Each review skill bundles the canonical contract and schemas under
`references/review-suite/` so the skill remains self-contained when installed
outside this repository. `just sync-contracts` refreshes the copies; this test
fails when any copy drifts from the canonical file.
Every review lens skill and every caller that consumes a review-code-change
result bundles the canonical contract and schemas under
`references/review-suite/` so each skill remains self-contained when installed
outside this repository. Callers that also validate a review result additionally
bundle the canonical `scripts/review_gate.py` and its test under `scripts/`.
`just sync-contracts` refreshes every copy; this test fails when any copy
drifts from its canonical file.
"""

from __future__ import annotations
Expand All @@ -20,6 +23,8 @@
"review-correctness",
"review-code-simplicity",
"review-solution-simplicity",
"implement-ticket",
"babysit-pr",
)
CANONICAL_FILES = {
"CONTRACT.md": REVIEW_SUITE / "CONTRACT.md",
Expand All @@ -32,6 +37,20 @@
"validate.py": REVIEW_SUITE / "scripts" / "validate.py",
}

# `review_gate.py` is a caller-side consumption check, not part of the review
# packet/result contract itself, so only skills that consume a
# `review-code-change` result bundle it (under `scripts/`, not
# `references/review-suite/`) — unlike CANONICAL_FILES above, which every
# review lens skill also bundles.
GATE_BUNDLING_SKILLS = ("implement-ticket", "babysit-pr")
GATE_CANONICAL_FILES = {
"scripts/review_gate.py": REVIEW_SUITE / "scripts" / "review_gate.py",
"scripts/tests/test_review_gate.py": REVIEW_SUITE
/ "scripts"
/ "tests"
/ "test_review_gate.py",
}


class BundledContractTests(unittest.TestCase):
def test_canonical_contract_marks_packet_prose_untrusted(self):
Expand Down Expand Up @@ -64,6 +83,23 @@ def test_every_review_skill_bundles_identical_contract_copies(self):
"run `just sync-contracts`",
)

def test_every_consuming_skill_bundles_an_identical_review_gate(self):
for skill in GATE_BUNDLING_SKILLS:
skill_root = REPOSITORY_ROOT / "skills" / skill
for relative, canonical in GATE_CANONICAL_FILES.items():
bundled = skill_root / relative
with self.subTest(skill=skill, file=relative):
self.assertTrue(
bundled.exists(),
f"{bundled} is missing; run `just sync-contracts`",
)
self.assertEqual(
canonical.read_bytes(),
bundled.read_bytes(),
f"{bundled} drifted from {canonical}; "
"run `just sync-contracts`",
)

def test_every_bundled_validator_executes_in_its_installed_layout(self):
"""The bundled validate.py must run from references/review-suite/.

Expand Down
Loading
Loading