From 81f0f31e6af0db091788e1112b6e31018ee17551 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Wed, 8 Jul 2026 13:11:07 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20review=20faculty=20=E2=80=94=20the=20sh?= =?UTF-8?q?ip=20gate's=20automatic-review=20leg=20(#36)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds agents/faculties/review/: a read-only faculty that prepares a feature branch's ReviewSurface (merge-base, commits ahead, diff stat, changed files, risk flags) via a stdlib entrypoint; the reviewing agent maps it to CLEAN/FINDINGS/BLOCKED per the documented procedure. Boundary settled in AGENTS.md: a diff review is a side-effect-free opinion (faculty), not a Heart check — Heart stays the sole release authority. Registered in bin/pyauto-brain; ship-path gate composition lands with series tasks 3-4. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UptUDNTYiB7izaQJBAKgS --- AGENTS.md | 7 ++ agents/faculties/review/AGENTS.md | 74 ++++++++++++++++ agents/faculties/review/_review.py | 134 +++++++++++++++++++++++++++++ agents/faculties/review/review.sh | 23 +++++ bin/pyauto-brain | 6 +- skills/WORKFLOW.md | 2 + 6 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 agents/faculties/review/AGENTS.md create mode 100755 agents/faculties/review/_review.py create mode 100755 agents/faculties/review/review.sh diff --git a/AGENTS.md b/AGENTS.md index 1d9dd5d..4f59307 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,6 +115,12 @@ humans invoke identically, so behaviour isn't re-derived from prose each time. PyAutoHeart readiness verdict and explains it, mapping each reason to its capability. The single component that talks to Heart; every conductor consults it rather than querying Heart directly. Never dispatches or mutates. +- **`agents/faculties/review/`** — *reviews the change*: prepares a feature + branch's ReviewSurface (diff, commits, risk flags) and defines the procedure + by which the reviewing agent maps it to a **CLEAN / FINDINGS / BLOCKED** + verdict — the automatic-review leg of the autonomous-ship gate + ([`AUTONOMY.md`](AUTONOMY.md)). Dev-workflow ship only: it never opines on + release readiness (Heart's, via vitals) and never fixes what it finds. > **Build Agent vs. the release conductor — resolved.** The Build Agent's > release mode owns the **single** readiness-gate + execution path @@ -145,6 +151,7 @@ bin/pyauto-brain build --dry-run # reason + plan only (emit the BuildDecision) bin/pyauto-brain release # reason about readiness, then release on green bin/pyauto-brain health # (conductor) run the health loop with a human, toward green bin/pyauto-brain vitals # (faculty) one tick + the unified dashboard card (raw read) +bin/pyauto-brain review --task # (faculty) a branch's ReviewSurface for the ship gate's review leg ``` Like the other PyAuto repos, PyAutoBrain runs from its checkout (no pip install); diff --git a/agents/faculties/review/AGENTS.md b/agents/faculties/review/AGENTS.md new file mode 100644 index 0000000..d140337 --- /dev/null +++ b/agents/faculties/review/AGENTS.md @@ -0,0 +1,74 @@ +# Review faculty + +> **Tier: faculty** — a read-only reasoning capability the conductors +> *consult*, not a front door you drive. It *reviews the change*: given a task +> worktree / feature branch, it returns a verdict — **CLEAN**, **FINDINGS**, or +> **BLOCKED** — and stops. It never dispatches, never mutates, never fixes what +> it finds. It is the automatic-review leg of the autonomous-ship gate +> ([`../../../AUTONOMY.md`](../../../AUTONOMY.md)). + +## Boundary (settled — do not reopen) + +A diff review is a **side-effect-free opinion**, which is the definition of a +faculty — it does **not** belong in Heart. Heart is the organism-state observer +(repo state, CI, PRs, deep install checks on `main`); it never looks at feature +branches and stays the sole authority on *release* readiness. This faculty +gates the **dev workflow's ship step** only: + +- It must never grow release opinions — release readiness is Heart's, adopted + via the vitals faculty. +- It must never edit code, post comments, or open anything — findings are + returned to the consulting conductor, which decides what to do. +- Its sensors are the branch diff and the harness review tooling, the way the + vitals faculty's sensor is Heart. + +## The verdict + +| Verdict | Meaning | Autonomous-run consequence | +|---------|---------|---------------------------| +| **CLEAN** | review + verify pass found nothing that must change | ship leg satisfied | +| **FINDINGS** | ranked defects/cleanups that must be resolved or judged | resolve, or downgrade to a human checkpoint — never ship past it | +| **BLOCKED** | could not review (no diff, unresolvable base, tooling failure) | treat as `human-required` | + +Verdict semantics are consumed by the ship gate exactly as `AUTONOMY.md` +defines — a verdict is an input to the gate, never a bypass of it. + +## How the faculty works (surface script + reviewing agent) + +The deterministic entrypoint prepares the **review surface**; the *verdict* is +produced by the reviewing agent (the session or subagent consulting this +faculty) following the procedure below — mirroring how vitals' script reads +Heart and the agent reasons over the verdict. + +1. `review.sh` (→ `_review.py`, stdlib-only) resolves the task worktree or + repo paths and emits, per repo: merge-base against `origin/main`, commits + ahead, diff stat, changed files, and risk flags (public-API-shaped paths, + config/schema files, tests changed or not, generated files). +2. The reviewing agent runs, over that surface: a **code review at high + effort** (correctness first, then reuse/simplification) and a **verify + pass** — drive the affected flow end-to-end, not just tests (for + doc/doctrine diffs: link resolution + single-source check instead). +3. Map the outcome to the verdict: any unresolved must-fix → **FINDINGS** + (ranked list, file:line, failure scenario); nothing → **CLEAN**; could not + complete steps 1–2 → **BLOCKED** (say why). + +## Run + +```bash +bin/pyauto-brain review --task # resolve ~/Code/PyAutoLabs-wt// claimed repos +bin/pyauto-brain review --repo [--repo ...] # explicit repo checkouts +bin/pyauto-brain review --task --json # machine-readable ReviewSurface +``` + +Exit codes: `0` surface produced · `4` no reviewable diff / could not resolve · +`5` bad usage. The script never exits non-zero for *findings* — findings are +the agent's judgment, not the script's. + +## What this faculty must never do + +- Dispatch, mutate, fix, comment, or open PRs/issues — it only opines. +- Query Heart or emit release-readiness opinions (vitals' job). +- Substitute its verdict for tests, smoke tests, or the Heart gate — the + autonomous-ship gate requires **all four** legs (`AUTONOMY.md`). +- Auto-resolve its own FINDINGS — resolution belongs to the consulting + conductor and re-review. diff --git a/agents/faculties/review/_review.py b/agents/faculties/review/_review.py new file mode 100755 index 0000000..e4c84a4 --- /dev/null +++ b/agents/faculties/review/_review.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +"""agents/faculties/review/_review.py — the review-surface substrate. + +The **review faculty** is a read-only opinion sink: given a task worktree or +explicit repo checkouts, it prepares the **ReviewSurface** — everything the +reviewing agent needs to run the review procedure in this faculty's AGENTS.md +(code review at high effort + a verify pass) and map the outcome to a +CLEAN / FINDINGS / BLOCKED verdict. + +This script produces the *surface*, never the *verdict*: findings are the +reviewing agent's judgment. It is stdlib-only, never writes anything, and +never exits non-zero because of what the diff contains. + +Exit codes: 0 surface produced · 4 no reviewable diff / could not resolve · +5 bad usage. +""" +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from pathlib import Path + +WT_BASE = Path.home() / "Code" / "PyAutoLabs-wt" + +# Paths that smell like public API / behaviour when changed — flags only, +# the reviewing agent judges. +RISK_MARKERS = { + "config-or-schema": (".yaml", ".yml", ".json", ".toml", ".ini", ".cfg"), + "packaging": ("setup.py", "setup.cfg", "pyproject.toml", "requirements"), +} + + +def _git(repo: Path, *args: str) -> str: + out = subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, text=True, check=False, + ) + return out.stdout.strip() if out.returncode == 0 else "" + + +def repo_surface(repo: Path) -> dict | None: + """One repo's slice of the ReviewSurface, or None if there is no diff.""" + if not (repo / ".git").exists(): + return None + branch = _git(repo, "rev-parse", "--abbrev-ref", "HEAD") + base = _git(repo, "merge-base", "HEAD", "origin/main") + if not base: + return None + commits = _git(repo, "log", "--oneline", f"{base}..HEAD") + if not commits: + return None + files = _git(repo, "diff", "--name-status", f"{base}..HEAD").splitlines() + stat = _git(repo, "diff", "--shortstat", f"{base}..HEAD") + changed = [line.split("\t")[-1] for line in files if line.strip()] + flags = [] + if not any("test" in f.lower() for f in changed): + flags.append("no-test-changes") + for name, exts in RISK_MARKERS.items(): + if any(f.endswith(exts) or any(m in f for m in exts) for f in changed): + flags.append(name) + if any(f.endswith(".py") and "test" not in f.lower() for f in changed): + flags.append("python-source") + return { + "repo": repo.name, + "path": str(repo), + "branch": branch, + "base": base[:12], + "commits_ahead": len(commits.splitlines()), + "commits": commits.splitlines(), + "shortstat": stat, + "files": files, + "risk_flags": flags, + } + + +def resolve_repos(task: str | None, repos: list[str]) -> list[Path]: + if task: + root = WT_BASE / task + if not root.is_dir(): + return [] + # Claimed repos are real directories (not symlinks) holding a .git + # file/dir — worktree_create symlinks everything unclaimed. + return sorted( + p for p in root.iterdir() + if p.is_dir() and not p.is_symlink() and (p / ".git").exists() + ) + return [Path(r).resolve() for r in repos] + + +def emit_human(surfaces: list[dict]) -> None: + print("== ReviewSurface (review faculty — surface only; the verdict is the") + print(" reviewing agent's, per agents/faculties/review/AGENTS.md) ==") + for s in surfaces: + print(f"\n-- {s['repo']} [{s['branch']}] base {s['base']}") + print(f" {s['commits_ahead']} commit(s) ahead — {s['shortstat']}") + for c in s["commits"][:10]: + print(f" * {c}") + print(f" risk flags: {', '.join(s['risk_flags']) or 'none'}") + for f in s["files"][:40]: + print(f" {f}") + if len(s["files"]) > 40: + print(f" ... and {len(s['files']) - 40} more") + print("\nVerdict rubric: CLEAN (nothing must change) | FINDINGS (ranked,") + print("file:line, failure scenario) | BLOCKED (could not review — say why).") + + +def main(argv=None) -> int: + ap = argparse.ArgumentParser(prog="review") + ap.add_argument("--task", default="", help="task worktree name under ~/Code/PyAutoLabs-wt/") + ap.add_argument("--repo", action="append", default=[], help="explicit repo checkout path") + ap.add_argument("--json", action="store_true", dest="as_json") + a = ap.parse_args(argv) + if not a.task and not a.repo: + print("review: pass --task or --repo ", file=sys.stderr) + return 5 + repos = resolve_repos(a.task or None, a.repo) + if not repos: + print("review: could not resolve any repo checkout", file=sys.stderr) + return 4 + surfaces = [s for s in (repo_surface(r) for r in repos) if s] + if not surfaces: + print("review: no reviewable diff against origin/main", file=sys.stderr) + return 4 + if a.as_json: + print(json.dumps({"review_surface": surfaces}, indent=2)) + else: + emit_human(surfaces) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/agents/faculties/review/review.sh b/agents/faculties/review/review.sh new file mode 100755 index 0000000..18cc0ef --- /dev/null +++ b/agents/faculties/review/review.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# agents/faculties/review/review.sh — the review faculty (a PyAutoBrain +# read-only reasoning capability). It reviews the change. +# +# Prepares the ReviewSurface for a task worktree / feature branch: per-repo +# merge-base against origin/main, commits ahead, diff stat, changed files and +# risk flags. The VERDICT (CLEAN / FINDINGS / BLOCKED) is produced by the +# reviewing agent following AGENTS.md in this directory — this script only +# prepares the surface, mirroring how vitals.sh reads Heart and the agent +# reasons over the result. Read-only: it never dispatches or mutates, never +# fixes a finding, and never opines on release readiness (Heart's, via the +# vitals faculty). +# +# Usage: +# review.sh --task # resolve ~/Code/PyAutoLabs-wt// +# review.sh --repo [--repo ...] # explicit repo checkouts +# review.sh ... --json # machine-readable ReviewSurface + +set -uo pipefail + +HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" + +exec python3 "$HERE/_review.py" "$@" diff --git a/bin/pyauto-brain b/bin/pyauto-brain index f02b478..d19f4ef 100755 --- a/bin/pyauto-brain +++ b/bin/pyauto-brain @@ -20,6 +20,8 @@ # pyauto-brain health [args] (conductor) the organism's clinician: run the health # loop with a human, dispatch by dispatch, toward green # pyauto-brain vitals [args] (faculty) read-only: read the Heart's pulse (the verdict) +# pyauto-brain review [args] (faculty) read-only: prepare a branch's ReviewSurface for the +# ship gate's review leg (verdict is the reviewing agent's) # pyauto-brain help [name] list agents or show one agent's docs # # Renamed from `pyauto-agent`; the former back-compat shim has been removed now @@ -42,6 +44,7 @@ declare -A AGENT_SCRIPT=( [release]="$CONDUCTORS_DIR/release/release.sh" [health]="$CONDUCTORS_DIR/health/health.sh" [vitals]="$FACULTIES_DIR/vitals/vitals.sh" + [review]="$FACULTIES_DIR/review/review.sh" ) declare -A AGENT_DESC=( [intake]="Conceive a task: turn raw input into a formal, headed PyAutoMind prompt (files it; never starts dev)" @@ -51,11 +54,12 @@ declare -A AGENT_DESC=( [release]="Release door → the Build Agent release mode (single gate); 'release rehearse'/'release validate' drive release validation" [health]="The organism's clinician: run the health loop with a human, dispatch by dispatch, toward green" [vitals]="Read-only: read the Heart's pulse — the PyAutoHeart readiness verdict (consulted by the conductors)" + [review]="Read-only: prepare the branch ReviewSurface — the reviewing agent maps it to CLEAN/FINDINGS/BLOCKED (the ship gate's review leg)" ) # Conductors are the front doors a human drives; faculties are consulted (and # runnable read-only). Both are dispatchable, but the menu groups them by tier. CONDUCTOR_ORDER=(intake feature bug build release health) -FACULTY_ORDER=(vitals) +FACULTY_ORDER=(vitals review) AGENT_ORDER=("${CONDUCTOR_ORDER[@]}" "${FACULTY_ORDER[@]}") cmd_help() { diff --git a/skills/WORKFLOW.md b/skills/WORKFLOW.md index db5a713..aed16dd 100644 --- a/skills/WORKFLOW.md +++ b/skills/WORKFLOW.md @@ -51,6 +51,8 @@ per-work-type caps, and the hard invariants (merge is always human; autonomous runs end at PR-open). Levels bind **only** under an explicit `--auto` launch; default runs present-and-wait at every checkpoint, exactly as the steps below describe. Do not restate checkpoint rules in a skill body — link the contract. +The gate's automatic-review leg is the **review faculty** +(`bin/pyauto-brain review --task `; `agents/faculties/review/AGENTS.md`). ## Brain agent entry points