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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ summary: Chronological history of repository and skill changes.

## 2026-07-19 — Epic workflow and review contract cleanup

- feat: compose repository-owned code review
- feat: add local code simplicity review
(d6ed890f6924a2ae7ae4b04fa95072ee853c9b97)
- feat: add whole-solution simplicity review
(8459402e95888047587cf423454f9f8ac42f6881)
- feat: add goal-first correctness review
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ A personal monorepo for agent skills and supporting scripts.

Current skills:

- `skills/review-code-change` — orchestrate the repository-owned review lenses
into one evidence-bound, deduplicated verdict
- `skills/implement-epic-sequence` — execute GitHub or Linear epics through
dependency-aware implementation, review, merge, cleanup, and closeout
- `skills/prepare-changesets` — decompose a large, review-ready branch into a
Expand Down
18 changes: 18 additions & 0 deletions review-suite/CONTRACT.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ fields that the caller could not establish and may preserve already-demonstrated
findings, but those findings do not convert the blocked review into a merge
verdict.

## Simplification proposal dispositions

When an orchestrator asks correctness to assess a validated simplification
result, supply that result beside the unchanged review packet. Do not add review
conclusions to the packet itself. Correctness returns one
`proposal_dispositions` item for every supplied gating proposal:

- `compatible` means the proposal preserves demonstrated correctness and remains
actionable; and
- `unsafe` means concrete correctness or repository evidence invalidates the
proposal even though the current candidate may already be correct.

Each disposition identifies the source finding and lens and cites concrete
evidence. It does not describe a candidate defect and therefore does not change
the correctness verdict by itself. If correctness cannot assess a supplied
proposal trustworthily, return `blocked`. Only `correctness` and `aggregate`
results may contain proposal dispositions.

## Candidate identity and base drift

Bind every result to the packet's captured head and comparison base. Any edit,
Expand Down
38 changes: 38 additions & 0 deletions review-suite/contracts/review-result.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,44 @@
"type": "array",
"items": {"type": "string", "minLength": 1}
},
"proposal_dispositions": {
"type": "array",
"items": {
"type": "object",
"additionalProperties": false,
"required": [
"finding_id",
"source_lens",
"disposition",
"reason",
"evidence"
],
"properties": {
"finding_id": {
"type": "string",
"pattern": "^[a-z0-9][a-z0-9._-]*$"
},
"source_lens": {
"enum": ["solution_simplicity", "code_simplicity"]
},
"disposition": {"enum": ["compatible", "unsafe"]},
"reason": {"type": "string", "minLength": 1},
"evidence": {
"type": "array",
"minItems": 1,
"items": {
"type": "object",
"additionalProperties": false,
"required": ["location", "detail"],
"properties": {
"location": {"type": "string", "minLength": 1},
"detail": {"type": "string", "minLength": 1}
}
}
}
}
}
},
"next_action": {"type": "string", "minLength": 1}
}
}
38 changes: 38 additions & 0 deletions review-suite/scripts/tests/test_contracts.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,44 @@ def test_unknown_finding_enum_is_rejected(self):
result["findings"][0]["confidence"] = "certain"
self.assertTrue(VALIDATOR.validate_result(result))

def test_clean_correctness_result_can_reject_an_unsafe_proposal(self):
result = copy.deepcopy(self.clean)
result["lens"] = "correctness"
result["proposal_dispositions"] = [
{
"finding_id": "solution-simplicity.remove-claim-fence",
"source_lens": "solution_simplicity",
"disposition": "unsafe",
"reason": "The predicates implement the required claim fence.",
"evidence": [
{
"location": "worker.py:complete",
"detail": "The token predicate prevents stale completion.",
}
],
}
]
self.assertEqual([], VALIDATOR.validate_result(result))

def test_simplification_lens_cannot_disposition_its_own_proposal(self):
result = copy.deepcopy(self.clean)
result["lens"] = "solution_simplicity"
result["proposal_dispositions"] = [
{
"finding_id": "solution-simplicity.remove-claim-fence",
"source_lens": "solution_simplicity",
"disposition": "unsafe",
"reason": "The predicates implement the required claim fence.",
"evidence": [
{
"location": "worker.py:complete",
"detail": "The token predicate prevents stale completion.",
}
],
}
]
self.assertTrue(VALIDATOR.validate_result(result))

def test_result_must_match_packet_candidate(self):
packet = load(ROOT / "fixtures" / "clean-change" / "packet.json")
result = copy.deepcopy(self.clean)
Expand Down
16 changes: 16 additions & 0 deletions review-suite/scripts/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,22 @@ def validate_result(result: dict[str, Any]) -> list[str]:
]
if foreign:
errors.append("$.findings: lens mismatch for " + ", ".join(foreign))

dispositions = result.get("proposal_dispositions", [])
if dispositions and result.get("lens") not in {"correctness", "aggregate"}:
errors.append(
"$.proposal_dispositions: only correctness or aggregate results may "
"disposition simplification proposals"
)
disposition_ids = [item.get("finding_id") for item in dispositions]
duplicate_dispositions = sorted(
{item for item in disposition_ids if disposition_ids.count(item) > 1}
)
if duplicate_dispositions:
errors.append(
"$.proposal_dispositions: duplicate finding id(s): "
+ ", ".join(duplicate_dispositions)
)
return errors


Expand Down
101 changes: 101 additions & 0 deletions skills/review-code-change/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
---
name: review-code-change
description: Run the complete repository-owned review of a PR, branch, patch, or change set. Use when one entrypoint should build a trustworthy evidence packet, invoke solution simplicity, correctness, and code simplicity in order, reconcile their results, and return one bounded aggregate verdict. Fail closed when evidence or local lens skills are missing, remain read-only, and never depend on a third-party review skill.
---

# Review Code Change

Produce one trustworthy, bounded verdict for one captured candidate. Orchestrate
the repository-owned lenses; do not reproduce their rubrics.

## Load the contracts and dependencies

1. Read `../../review-suite/CONTRACT.md` and both shared schemas.
2. Read [the orchestration protocol](references/orchestration-protocol.md).
3. Verify that `review-solution-simplicity`, `review-correctness`, and
`review-code-simplicity` are available and readable.
4. Return a conforming aggregate `blocked` result naming every missing skill. Do
not fall back to another skill or generic self-review.

## Build one evidence packet

Build the shared packet once from raw ticket, PR, repository, candidate, and
validation artifacts. Capture:

- observable goal, acceptance criteria, non-goals, and preserved behavior;
- repository instructions, named specifications, and representative nearby code
and tests;
- exact head and relevant base or merge-base identity plus the complete diff;
- focused and full validation commands with exact results; and
- worktree state required to prove read-only integrity.

Exclude implementation transcripts, intended answers, prior conclusions,
suspected findings, and fixture expected outputs. Validate the packet before
invoking any lens. Return `blocked` when required evidence cannot be recovered
safely.

## Run the deliberate sequence

For a full review, invoke the skills sequentially from the same validated
packet:

1. `review-solution-simplicity`
2. `review-correctness`
3. `review-code-simplicity`

Validate each result before continuing. Stop on a `blocked` result. Stop after a
solution-simplicity result that requires replacing the implementation strategy;
the caller must redesign and start a full review on a new head. For a tractable
in-strategy proposal, invoke correctness with the unchanged packet plus the
validated solution result as separate proposal context. Require one proposal
disposition for each gating simplification finding so correctness can reject an
unsafe proposal without inventing a defect in already-correct candidate code.

Stop after an actionable correctness result; do not spend tokens on local
simplification before the correctness fix. A clean pass through all applicable
lenses ends the review.

## Handle fixes and cycles

The orchestrator is read-only. Return the required fix and next lens; the caller
applies changes and supplies a new packet bound to the new head.

- After solution redesign, restart the full sequence.
- After a correctness fix, rerun correctness and every downstream lens whose
assumptions changed.
- After a code-simplicity fix, rerun code simplicity and then targeted
correctness.
- Use at most three full fix/re-review cycles by default. On the final cycle,
return unresolved material findings without requesting another automatic
cycle.
- Ignore style, praise, speculative hardening, and deferred scope when counting
cycles.

## Aggregate one result

Follow the protocol to validate, deduplicate, and reconcile results. Correctness
and explicit repository constraints override unsafe simplification. Preserve
deferred findings as non-gating. Preserve proposal dispositions in the aggregate
so every accepted or rejected simplification remains auditable.

Return only JSON conforming to the shared result schema with lens `aggregate`.
Include candidate identity, material findings, blocking reasons, validation
limitations, and the next required action.

- `changes_required`: a blocking or strong-recommendation finding remains.
- `blocked`: evidence, a required dependency, or a lens verdict is
untrustworthy.
- `clean`: every required lens completed and no gating finding remains.

Never count an unavailable or skipped required lens as clean.

## Preserve candidate integrity

Bind every packet and result to the captured head. Build a new packet after any
code change, rebase, conflict resolution, or update. For base-only drift, apply
the shared risk-based merge-candidate rules.

Do not edit or format reviewed files, apply fixes, create candidate artifacts,
commit, push, post reviews, resolve threads, approve, merge, or update tickets.
Run only safe read-only inspection and validation commands. Verify that the
candidate state is unchanged before returning.
4 changes: 4 additions & 0 deletions skills/review-code-change/agents/openai.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
interface:
display_name: "Review Code Change"
short_description: "Run the repository-owned code review suite"
default_prompt: "Use $review-code-change to build one evidence packet and run the complete repository-owned review sequence for this change."
Loading
Loading