diff --git a/doc/reviewer/agentic-system-design.md b/doc/reviewer/agentic-system-design.md new file mode 100644 index 0000000..701939d --- /dev/null +++ b/doc/reviewer/agentic-system-design.md @@ -0,0 +1,215 @@ +# Agentic system design β€” multi-model audit-to-code pipeline + +**Purpose:** turn the static plan in [`finding-orchestration.md`](finding-orchestration.md) +into a running **multi-agent system** that works the planned sessions, interleaves +**reviews and fresh audits**, and lands findings in the code **cleanly** (atomic, +attributed, tested, parity-safe). + +This design is grounded in what Session 1+2 actually did: Haiku renamed/constants, +Sonnet did validation/error-handling, **Opus** did the numerics + the **review gate** +that validated the cheaper models' work, then diagnosed a CI OOM and sequenced the +#36β†’#39 rebase. The system below generalises exactly that flow. + +--- + +## 1. Design principles (the lessons that shape the architecture) + +1. **Assign by cognitive load, not severity.** A πŸ”΄ one-line switch is Haiku/Sonnet + work; a 🟑 numerical reformulation is Opus. The dispatcher routes on *"how much + must be understood to get it right"*. +2. **Reviewer β‰  implementer, and the reviewer is the strong model.** Every change + passes an **independent Opus review gate**. Self-review misses parity/numeric + regressions; a fresh high-capability pass catches them (it caught nothing wrong + this session β€” which is the point: it *certified* the cheap work). +3. **The plan doc is the blackboard.** Agents are stateless sessions that + read β†’ act β†’ write `finding-orchestration.md`. No message bus; the shared state + is human-inspectable markdown. +4. **One finding β†’ one commit β†’ one test β†’ one attribution.** Traceability and clean + revertability. Renames ship with `[[deprecated]]` aliases; refactors ship with + value-identity assertions. +5. **Gate everything irreversible to a human.** Legal/provenance (G0), public-API + stabilization (A4/A5), and the final merge are human decisions the system + *surfaces*, never makes. +6. **Separate code failures from infra failures.** A CI red is not automatically a + code bug β€” a dedicated diagnostic step distinguishes `cc1plus Killed` (OOM) from a + real defect before anyone touches the code. +7. **Exploit warm context.** Batch findings that touch the same code while a session + is warm (S2's I1/H1/N7 were done by Opus *immediately* after the `newton_core` + refactor instead of as a cold pickup). + +--- + +## 2. Roles + +| Role | Model | Owns | Triggered by | Output | +|---|---|---|---|---| +| **Orchestrator / Planner** | Sonnet | the backlog + dispatch | new findings, finished session | next session spec + launch prompt; updated status board | +| **Mechanic** | **Haiku** | renames, named constants, doc/citation/format fixes | dispatched mechanical findings | atomic commits + `[[deprecated]]` aliases | +| **Engineer** | **Sonnet** | tests, error handling, CI, small/safe API changes, robustness | dispatched standard findings | commits + per-finding tests | +| **Specialist** | **Opus** | numerics, math correctness, architecture refactors, public-API/irreversible calls | dispatched judgment findings | commits + tests + design notes | +| **Reviewer (Gatekeeper)** | **Opus** | the review gate after every implementation session | session marked "implemented" | APPROVE / CHANGES-REQUESTED + fixes | +| **Auditor (External reviewer)** | **Opus** | generating *new* audits with `file:line` + fix + acceptance | new module, schedule, or "re-audit" | new audit doc β†’ new backlog findings | +| **Integrator / Release** | Sonnet | git mechanics: branch, PR, rebase/conflict, CI watch, merge sequencing | approved session | PR; rebases; merge (after human gate) | +| **Infra Diagnostician** | Sonnetβ†’Opus | CI-failure triage (code vs runner/OOM), infra fixes | any CI red | "code vs infra" verdict + fix (e.g. `-j1`) | +| **Human (Owner)** | β€” | legal/provenance, API stabilization, final merge | surfaced decision | decision recorded in the plan | + +> Tiering rule of thumb: **Haiku β‰ˆ "transcribe the fix"**, **Sonnet β‰ˆ "engineer the +> fix from a clear spec"**, **Opus β‰ˆ "decide what the fix should be" + "judge someone +> else's fix"**. + +--- + +## 3. Workflow (state machine, per session) + +``` + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ BACKLOG (audit findings, status ⬜/⏸/β›” in the plan doc) β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + β”‚ Orchestrator picks next ⬜, routes by load + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ PLAN ──────────┐ emits a session-prompts.md block + β”‚ model + findings + branch │────────────────────────────────────────┐ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ + β–Ό β”‚ + IMPLEMENT (Mechanic | Engineer | Specialist) β”‚ + Β· atomic commit per finding Β· test per finding β”‚ + Β· deprecated aliases / value-identity asserts β”‚ + β–Ό β”‚ + SELF-VERIFY (build + full test suite green locally) β”‚ + β–Ό β”‚ + β”Œβ”€β”€β”€β”€ REVIEW GATE (Opus, independent) ────┐ β”‚ + β”‚ build Β· parity Β· value-identity Β· │── CHANGES-REQUESTED β”€β”€β”€β”€β”€β”˜ + β”‚ public-surface Β· attribution Β· plan βœ… β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ APPROVE + β–Ό + INTEGRATE (Integrator) β†’ open PR + β–Ό + CI ──red──► INFRA DIAGNOSTICIAN ──code?──► back to IMPLEMENT + β”‚ └─infra?─► fix infra (e.g. -j1), re-run + green + β–Ό + HUMAN MERGE GATE ──► MERGE ──► Orchestrator marks βœ…, updates plan + β–Ό + (every N sessions / per module) AUDITOR re-audit ──► new BACKLOG findings +``` + +Key edges proven this session: the **CHANGES-REQUESTED loop** (none needed β€” Sonnet/Haiku +work was clean), the **INFRA branch** (the `cc1plus` OOM β†’ `-j1`, *not* a code change), +and the **multi-PR sequencing** at INTEGRATE (#36 merged first, #39 rebased to drop the +duplicate A1–A3). + +--- + +## 4. Shared state & artifacts (the blackboard) + +- **`finding-orchestration.md`** β€” backlog + status board + per-finding model/session. + The single source of truth every agent reads/writes. +- **Audit docs** (`*-audit-*.md`) β€” the finding *specs*: `file:line`, fix, acceptance + criteria. The Auditor writes them; implementers consume them. +- **`session-prompts.md`** β€” launch templates the Orchestrator fills in per session. +- **PRs + commits** β€” the audit trail; commit trailer `Co-Authored-By: Claude ` + records *which* model did *which* finding. +- **Decision log** (a section in the plan) β€” human gates: G0 status ("authors emailed, + awaiting reply"), API-stabilization decisions, merge approvals. + +Why blackboard over a chat-bus: sessions are short-lived and stateless; the durable +state is the markdown; humans can read and override it at any point. + +--- + +## 5. Gates & guardrails + +**Automated gates (must pass to advance):** +- Build clean; **full test suite green, no count regression**. +- **No parity/golden-vector test perturbed** (HardJava-style defaults intact). +- New public surface (result types, enums, file formats) is intentional + documented. +- Each finding has a commit + a test; renames carry `[[deprecated]]` aliases. + +**Human gates (system surfaces, human decides):** +- **Legal/provenance (G0)** β€” porting/relicensing rights. Blocks G1–G12, D1/D2, A4/A5. +- **Public-API stabilization** β€” once in a released/CGAL API, renames need a deprecation + cycle; lock names only when the owner says so. +- **Final merge** β€” the agent prepares a green, mergeable PR; the human clicks merge + (or explicitly delegates it). + +**Blocked-dependency tracking:** findings gated by an open human decision are marked β›” +in the plan and never dispatched (S6 stays blocked until G0 resolves). + +--- + +## 6. How phases, reviews and audits interleave (cadence) + +- **Per session:** implement β†’ **review gate** (always). The review is *inline* with + the cadence, not a separate phase you forget. +- **Per PR:** CI + infra-triage + human merge gate. +- **Per module / every N sessions / on request:** an **Auditor** pass produces a fresh + audit, refilling the backlog. This is the loop that keeps the system honest as the + code grows β€” new code gets audited, new findings get planned, the cycle repeats. +- **Continuous:** the Orchestrator keeps the plan's status board current so any human + glance shows "done / in-flight / blocked". + +So the rhythm is: **audit β†’ plan β†’ (implement β†’ review)\* β†’ integrate β†’ merge β†’ re-audit.** + +--- + +## 7. How to actually run it with Claude + +Three implementation levels, cheapest first: + +1. **Manual sessions (today).** The Orchestrator (you, or a Sonnet session) hands a + `session-prompts.md` block to a fresh session set to the named model. The review + gate is a second session set to Opus. This already works β€” it is exactly Session 1+2. +2. **Claude Code subagents.** Define each role as a subagent with a pinned model and a + tight tool allow-list (Mechanic: edit+bash; Reviewer: read+bash, no write-to-main; + Integrator: bash/git/PR API). A top-level "Orchestrator" session dispatches via the + Task tool and reconciles the plan doc. +3. **Claude Agent SDK (autonomous).** A supervisor process loops the state machine: + reads the plan, spawns role-agents (model per role), runs the gates as code + (build/test/CI checks), and stops at human gates. The blackboard (the markdown + + git) is the durable state across runs. + +Pin models per role explicitly; do **not** let a cheap role silently escalate β€” that is +what the review gate and the dispatcher's load-routing are for. + +--- + +## 8. Cost & efficiency strategy + +- **Route down aggressively, gate up always.** Most findings go to Haiku/Sonnet; only + the Opus review gate and genuine-judgment findings pay for Opus. +- **Batch by warm context.** Group findings touching the same files into one session + (S2's three findings shared `newton_core`); avoid cold re-derivation. +- **Make the reviewer cheap to satisfy.** Atomic commits + per-finding tests + a fixed + checklist make the Opus review fast (it reads a small, well-scoped diff). +- **Fail fast on infra.** The diagnostician prevents wasted Opus cycles "fixing" a + phantom code bug that was really an OOM. + +--- + +## 9. Worked example (this session, mapped to the roles) + +| What happened | Role(s) | +|---|---| +| A1–A3 renames, N4/N6 constants, M-citations | **Mechanic (Haiku)** | +| V1–V4 error handling, C2/C3 coverage gate, I-tests | **Engineer (Sonnet)** | +| B1 inv-dist block-FD port, N3/N5, `newton_core` (H2+B2/B4/B5), S2 (I1/H1/N7) | **Specialist (Opus)** | +| Validated the Haiku/Sonnet commits (value-identity, alias forwarding, coverage logic) | **Reviewer (Opus)** | +| Built the plan, model assignment, session prompts | **Orchestrator** | +| `#36` merge β†’ rebase `#39` (drop dup A1–A3) β†’ PRs | **Integrator** | +| `test-fast` red β†’ proved `cc1plus` OOM via a pure-main probe β†’ `-j1` fix | **Infra Diagnostician** | +| G0 porting rights: "authors emailed, awaiting reply"; merge approval | **Human gate** | + +The audits themselves (the 11 docs) were a prior **Auditor (Opus)** pass β€” the input the +whole pipeline consumes. + +--- + +## 10. Failure modes the design absorbs + +- **Cheap model gets it subtly wrong** β†’ caught by the Opus review gate. +- **Refactor silently changes numerics** β†’ caught by parity tests + value-identity asserts. +- **CI red panic** β†’ diagnostician separates infra (OOM) from code before any "fix". +- **Two PRs overlap** (#36/#39) β†’ Integrator sequences merge + rebase deterministically. +- **A finding needs a human/legal call** β†’ marked β›”, never auto-dispatched. +- **Backlog goes stale as code grows** β†’ periodic Auditor re-audit refills it. +```