Skip to content

Commit 303e2ca

Browse files
authored
Merge pull request #37 from PyAutoLabs/feature/review-faculty
feat: review faculty — the ship gate's automatic-review leg
2 parents 76b6703 + 81f0f31 commit 303e2ca

6 files changed

Lines changed: 245 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ humans invoke identically, so behaviour isn't re-derived from prose each time.
115115
PyAutoHeart readiness verdict and explains it, mapping each reason to its
116116
capability. The single component that talks to Heart; every conductor
117117
consults it rather than querying Heart directly. Never dispatches or mutates.
118+
- **`agents/faculties/review/`***reviews the change*: prepares a feature
119+
branch's ReviewSurface (diff, commits, risk flags) and defines the procedure
120+
by which the reviewing agent maps it to a **CLEAN / FINDINGS / BLOCKED**
121+
verdict — the automatic-review leg of the autonomous-ship gate
122+
([`AUTONOMY.md`](AUTONOMY.md)). Dev-workflow ship only: it never opines on
123+
release readiness (Heart's, via vitals) and never fixes what it finds.
118124

119125
> **Build Agent vs. the release conductor — resolved.** The Build Agent's
120126
> 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)
145151
bin/pyauto-brain release # reason about readiness, then release on green
146152
bin/pyauto-brain health # (conductor) run the health loop with a human, toward green
147153
bin/pyauto-brain vitals # (faculty) one tick + the unified dashboard card (raw read)
154+
bin/pyauto-brain review --task <name> # (faculty) a branch's ReviewSurface for the ship gate's review leg
148155
```
149156

150157
Like the other PyAuto repos, PyAutoBrain runs from its checkout (no pip install);

agents/faculties/review/AGENTS.md

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# Review faculty
2+
3+
> **Tier: faculty** — a read-only reasoning capability the conductors
4+
> *consult*, not a front door you drive. It *reviews the change*: given a task
5+
> worktree / feature branch, it returns a verdict — **CLEAN**, **FINDINGS**, or
6+
> **BLOCKED** — and stops. It never dispatches, never mutates, never fixes what
7+
> it finds. It is the automatic-review leg of the autonomous-ship gate
8+
> ([`../../../AUTONOMY.md`](../../../AUTONOMY.md)).
9+
10+
## Boundary (settled — do not reopen)
11+
12+
A diff review is a **side-effect-free opinion**, which is the definition of a
13+
faculty — it does **not** belong in Heart. Heart is the organism-state observer
14+
(repo state, CI, PRs, deep install checks on `main`); it never looks at feature
15+
branches and stays the sole authority on *release* readiness. This faculty
16+
gates the **dev workflow's ship step** only:
17+
18+
- It must never grow release opinions — release readiness is Heart's, adopted
19+
via the vitals faculty.
20+
- It must never edit code, post comments, or open anything — findings are
21+
returned to the consulting conductor, which decides what to do.
22+
- Its sensors are the branch diff and the harness review tooling, the way the
23+
vitals faculty's sensor is Heart.
24+
25+
## The verdict
26+
27+
| Verdict | Meaning | Autonomous-run consequence |
28+
|---------|---------|---------------------------|
29+
| **CLEAN** | review + verify pass found nothing that must change | ship leg satisfied |
30+
| **FINDINGS** | ranked defects/cleanups that must be resolved or judged | resolve, or downgrade to a human checkpoint — never ship past it |
31+
| **BLOCKED** | could not review (no diff, unresolvable base, tooling failure) | treat as `human-required` |
32+
33+
Verdict semantics are consumed by the ship gate exactly as `AUTONOMY.md`
34+
defines — a verdict is an input to the gate, never a bypass of it.
35+
36+
## How the faculty works (surface script + reviewing agent)
37+
38+
The deterministic entrypoint prepares the **review surface**; the *verdict* is
39+
produced by the reviewing agent (the session or subagent consulting this
40+
faculty) following the procedure below — mirroring how vitals' script reads
41+
Heart and the agent reasons over the verdict.
42+
43+
1. `review.sh` (→ `_review.py`, stdlib-only) resolves the task worktree or
44+
repo paths and emits, per repo: merge-base against `origin/main`, commits
45+
ahead, diff stat, changed files, and risk flags (public-API-shaped paths,
46+
config/schema files, tests changed or not, generated files).
47+
2. The reviewing agent runs, over that surface: a **code review at high
48+
effort** (correctness first, then reuse/simplification) and a **verify
49+
pass** — drive the affected flow end-to-end, not just tests (for
50+
doc/doctrine diffs: link resolution + single-source check instead).
51+
3. Map the outcome to the verdict: any unresolved must-fix → **FINDINGS**
52+
(ranked list, file:line, failure scenario); nothing → **CLEAN**; could not
53+
complete steps 1–2 → **BLOCKED** (say why).
54+
55+
## Run
56+
57+
```bash
58+
bin/pyauto-brain review --task <task-name> # resolve ~/Code/PyAutoLabs-wt/<task>/ claimed repos
59+
bin/pyauto-brain review --repo <path> [--repo ...] # explicit repo checkouts
60+
bin/pyauto-brain review --task <task-name> --json # machine-readable ReviewSurface
61+
```
62+
63+
Exit codes: `0` surface produced · `4` no reviewable diff / could not resolve ·
64+
`5` bad usage. The script never exits non-zero for *findings* — findings are
65+
the agent's judgment, not the script's.
66+
67+
## What this faculty must never do
68+
69+
- Dispatch, mutate, fix, comment, or open PRs/issues — it only opines.
70+
- Query Heart or emit release-readiness opinions (vitals' job).
71+
- Substitute its verdict for tests, smoke tests, or the Heart gate — the
72+
autonomous-ship gate requires **all four** legs (`AUTONOMY.md`).
73+
- Auto-resolve its own FINDINGS — resolution belongs to the consulting
74+
conductor and re-review.

agents/faculties/review/_review.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
#!/usr/bin/env python3
2+
"""agents/faculties/review/_review.py — the review-surface substrate.
3+
4+
The **review faculty** is a read-only opinion sink: given a task worktree or
5+
explicit repo checkouts, it prepares the **ReviewSurface** — everything the
6+
reviewing agent needs to run the review procedure in this faculty's AGENTS.md
7+
(code review at high effort + a verify pass) and map the outcome to a
8+
CLEAN / FINDINGS / BLOCKED verdict.
9+
10+
This script produces the *surface*, never the *verdict*: findings are the
11+
reviewing agent's judgment. It is stdlib-only, never writes anything, and
12+
never exits non-zero because of what the diff contains.
13+
14+
Exit codes: 0 surface produced · 4 no reviewable diff / could not resolve ·
15+
5 bad usage.
16+
"""
17+
from __future__ import annotations
18+
19+
import argparse
20+
import json
21+
import subprocess
22+
import sys
23+
from pathlib import Path
24+
25+
WT_BASE = Path.home() / "Code" / "PyAutoLabs-wt"
26+
27+
# Paths that smell like public API / behaviour when changed — flags only,
28+
# the reviewing agent judges.
29+
RISK_MARKERS = {
30+
"config-or-schema": (".yaml", ".yml", ".json", ".toml", ".ini", ".cfg"),
31+
"packaging": ("setup.py", "setup.cfg", "pyproject.toml", "requirements"),
32+
}
33+
34+
35+
def _git(repo: Path, *args: str) -> str:
36+
out = subprocess.run(
37+
["git", "-C", str(repo), *args],
38+
capture_output=True, text=True, check=False,
39+
)
40+
return out.stdout.strip() if out.returncode == 0 else ""
41+
42+
43+
def repo_surface(repo: Path) -> dict | None:
44+
"""One repo's slice of the ReviewSurface, or None if there is no diff."""
45+
if not (repo / ".git").exists():
46+
return None
47+
branch = _git(repo, "rev-parse", "--abbrev-ref", "HEAD")
48+
base = _git(repo, "merge-base", "HEAD", "origin/main")
49+
if not base:
50+
return None
51+
commits = _git(repo, "log", "--oneline", f"{base}..HEAD")
52+
if not commits:
53+
return None
54+
files = _git(repo, "diff", "--name-status", f"{base}..HEAD").splitlines()
55+
stat = _git(repo, "diff", "--shortstat", f"{base}..HEAD")
56+
changed = [line.split("\t")[-1] for line in files if line.strip()]
57+
flags = []
58+
if not any("test" in f.lower() for f in changed):
59+
flags.append("no-test-changes")
60+
for name, exts in RISK_MARKERS.items():
61+
if any(f.endswith(exts) or any(m in f for m in exts) for f in changed):
62+
flags.append(name)
63+
if any(f.endswith(".py") and "test" not in f.lower() for f in changed):
64+
flags.append("python-source")
65+
return {
66+
"repo": repo.name,
67+
"path": str(repo),
68+
"branch": branch,
69+
"base": base[:12],
70+
"commits_ahead": len(commits.splitlines()),
71+
"commits": commits.splitlines(),
72+
"shortstat": stat,
73+
"files": files,
74+
"risk_flags": flags,
75+
}
76+
77+
78+
def resolve_repos(task: str | None, repos: list[str]) -> list[Path]:
79+
if task:
80+
root = WT_BASE / task
81+
if not root.is_dir():
82+
return []
83+
# Claimed repos are real directories (not symlinks) holding a .git
84+
# file/dir — worktree_create symlinks everything unclaimed.
85+
return sorted(
86+
p for p in root.iterdir()
87+
if p.is_dir() and not p.is_symlink() and (p / ".git").exists()
88+
)
89+
return [Path(r).resolve() for r in repos]
90+
91+
92+
def emit_human(surfaces: list[dict]) -> None:
93+
print("== ReviewSurface (review faculty — surface only; the verdict is the")
94+
print(" reviewing agent's, per agents/faculties/review/AGENTS.md) ==")
95+
for s in surfaces:
96+
print(f"\n-- {s['repo']} [{s['branch']}] base {s['base']}")
97+
print(f" {s['commits_ahead']} commit(s) ahead — {s['shortstat']}")
98+
for c in s["commits"][:10]:
99+
print(f" * {c}")
100+
print(f" risk flags: {', '.join(s['risk_flags']) or 'none'}")
101+
for f in s["files"][:40]:
102+
print(f" {f}")
103+
if len(s["files"]) > 40:
104+
print(f" ... and {len(s['files']) - 40} more")
105+
print("\nVerdict rubric: CLEAN (nothing must change) | FINDINGS (ranked,")
106+
print("file:line, failure scenario) | BLOCKED (could not review — say why).")
107+
108+
109+
def main(argv=None) -> int:
110+
ap = argparse.ArgumentParser(prog="review")
111+
ap.add_argument("--task", default="", help="task worktree name under ~/Code/PyAutoLabs-wt/")
112+
ap.add_argument("--repo", action="append", default=[], help="explicit repo checkout path")
113+
ap.add_argument("--json", action="store_true", dest="as_json")
114+
a = ap.parse_args(argv)
115+
if not a.task and not a.repo:
116+
print("review: pass --task <name> or --repo <path>", file=sys.stderr)
117+
return 5
118+
repos = resolve_repos(a.task or None, a.repo)
119+
if not repos:
120+
print("review: could not resolve any repo checkout", file=sys.stderr)
121+
return 4
122+
surfaces = [s for s in (repo_surface(r) for r in repos) if s]
123+
if not surfaces:
124+
print("review: no reviewable diff against origin/main", file=sys.stderr)
125+
return 4
126+
if a.as_json:
127+
print(json.dumps({"review_surface": surfaces}, indent=2))
128+
else:
129+
emit_human(surfaces)
130+
return 0
131+
132+
133+
if __name__ == "__main__":
134+
sys.exit(main())

agents/faculties/review/review.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/usr/bin/env bash
2+
# agents/faculties/review/review.sh — the review faculty (a PyAutoBrain
3+
# read-only reasoning capability). It reviews the change.
4+
#
5+
# Prepares the ReviewSurface for a task worktree / feature branch: per-repo
6+
# merge-base against origin/main, commits ahead, diff stat, changed files and
7+
# risk flags. The VERDICT (CLEAN / FINDINGS / BLOCKED) is produced by the
8+
# reviewing agent following AGENTS.md in this directory — this script only
9+
# prepares the surface, mirroring how vitals.sh reads Heart and the agent
10+
# reasons over the result. Read-only: it never dispatches or mutates, never
11+
# fixes a finding, and never opines on release readiness (Heart's, via the
12+
# vitals faculty).
13+
#
14+
# Usage:
15+
# review.sh --task <task-name> # resolve ~/Code/PyAutoLabs-wt/<task>/
16+
# review.sh --repo <path> [--repo ...] # explicit repo checkouts
17+
# review.sh ... --json # machine-readable ReviewSurface
18+
19+
set -uo pipefail
20+
21+
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
22+
23+
exec python3 "$HERE/_review.py" "$@"

bin/pyauto-brain

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
# pyauto-brain health [args] (conductor) the organism's clinician: run the health
2121
# loop with a human, dispatch by dispatch, toward green
2222
# pyauto-brain vitals [args] (faculty) read-only: read the Heart's pulse (the verdict)
23+
# pyauto-brain review [args] (faculty) read-only: prepare a branch's ReviewSurface for the
24+
# ship gate's review leg (verdict is the reviewing agent's)
2325
# pyauto-brain help [name] list agents or show one agent's docs
2426
#
2527
# Renamed from `pyauto-agent`; the former back-compat shim has been removed now
@@ -42,6 +44,7 @@ declare -A AGENT_SCRIPT=(
4244
[release]="$CONDUCTORS_DIR/release/release.sh"
4345
[health]="$CONDUCTORS_DIR/health/health.sh"
4446
[vitals]="$FACULTIES_DIR/vitals/vitals.sh"
47+
[review]="$FACULTIES_DIR/review/review.sh"
4548
)
4649
declare -A AGENT_DESC=(
4750
[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=(
5154
[release]="Release door → the Build Agent release mode (single gate); 'release rehearse'/'release validate' drive release validation"
5255
[health]="The organism's clinician: run the health loop with a human, dispatch by dispatch, toward green"
5356
[vitals]="Read-only: read the Heart's pulse — the PyAutoHeart readiness verdict (consulted by the conductors)"
57+
[review]="Read-only: prepare the branch ReviewSurface — the reviewing agent maps it to CLEAN/FINDINGS/BLOCKED (the ship gate's review leg)"
5458
)
5559
# Conductors are the front doors a human drives; faculties are consulted (and
5660
# runnable read-only). Both are dispatchable, but the menu groups them by tier.
5761
CONDUCTOR_ORDER=(intake feature bug build release health)
58-
FACULTY_ORDER=(vitals)
62+
FACULTY_ORDER=(vitals review)
5963
AGENT_ORDER=("${CONDUCTOR_ORDER[@]}" "${FACULTY_ORDER[@]}")
6064

6165
cmd_help() {

skills/WORKFLOW.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ per-work-type caps, and the hard invariants (merge is always human; autonomous
5151
runs end at PR-open). Levels bind **only** under an explicit `--auto` launch;
5252
default runs present-and-wait at every checkpoint, exactly as the steps below
5353
describe. Do not restate checkpoint rules in a skill body — link the contract.
54+
The gate's automatic-review leg is the **review faculty**
55+
(`bin/pyauto-brain review --task <name>`; `agents/faculties/review/AGENTS.md`).
5456

5557
## Brain agent entry points
5658

0 commit comments

Comments
 (0)