pragent/docs/plans/2026-08-21-pragent-update-design.md

# pragent — Update Design

**Date:** 2026-08-21
**Status:** Approved (brainstorm, 2026-08-21)
**Replaces:** none — additive + behavioral. Existing `docs/plans/2026-08-04-pragent-design.md` stays authoritative on architecture.

## Problem

The pilot has been live long enough to surface pain that the original design didn't
cover:

1. **Two labels to remember.** `AI-REVIEW` (gate) + `AI-USAGE` (opt-in for the
   cost block) are per-PR. Every new contributor reads the README wrong at least
   once. Reviews that the team *wanted* are skipped because nobody labeled; reviews
   we *don't* want still run because the label is sticky.
2. **Token numbers are unreadable.** `Total Tokens: 2071025 in / 17303 out`
   requires a mental carry. The pilot already measures the tokens; the rendering
   just doesn't help.
3. **Cost is anchored on one provider.** The pilot runs free (headroom/glm-5.2)
   but the only equivalent-cost line is Claude Sonnet. We can't answer "what would
   this have cost on GPT / Gemini / Grok?" without running the CLI on a different
   model.
4. **The PR summary is operational, not useful.** A lens-fanout run posts
   `Multi-lens review of repo#index (sha X). Lenses: security,perf. Findings:
   critical=0 high=1 medium=2 low=1.` That tells a reviewer *how the bot worked*,
   not *what they should look at*. Real products post a risk verdict, a
   file-by-file walkthrough, and a test-coverage note.
5. **Triage noise is the dominant failure mode** in every competitor (CodeRabbit,
   Qodo, Greptile, DoorDash). We already address most of it (severity_floor,
   per-file cap, cross-lens agreement, tone-strip), but two cheap wins are left on
   the table: a per-PR *merge confidence* badge, and a richer severity scale that
   includes `trivial` / `info` (CodeRabbit's pattern).

This update also distills lessons from a 30-article survey of AI code review
products (CodeRabbit, Qodo/Merge + PR-Agent, Greptile, GitHub Copilot code
review, Gemini Code Assist, qodo-ai/pr-agent, anc95/ChatGPT-CodeReview, Sourcery,
Danger, plus the security literature around the April 2026 prompt-injection
disclosures). Where we already match the state of the art, this update notes it
and moves on; where a competitor's pattern is genuinely better, it lands here.

## Decisions

| Question | Decision | Why |
|---|---|---|
| Trigger | `.pr-review.json:enabled` on the PR's base branch | Repo opt-in replaces labels. No per-PR manual step. Trust stays on base. |
| Default when `.pr-review.json` is absent | Disabled | Explicit opt-in. Mirrors "labels fully removed." |
| Cost model | Always render when usage data is present | Drop the `report_usage` parameter + `AI-USAGE` label + `PRAGENT_USAGE_ALWAYS` env. |
| Token rendering | `1,234,567 (1.2M)` | Python `f"{n:,}"` + short suffix only when `n ≥ 1000`. |
| Multi-provider cost | Markdown table in the collapsible usage block | Replaces the single Sonnet line. Default compare set: Sonnet, GPT-5, Gemini 2.5 Pro, Grok 4.5. |
| Summary depth | Add `walkthrough` / `risk_verdict` / `test_coverage` to the agent JSON; Python fallback for lens synthesis | Agent produces the rich text; Python derives the same three when the lens fan-out is engaged. |
| Severity scale | Extend from 4 → 6 levels: add `trivial` + `info` | Matches CodeRabbit. Backward compat (unknown → medium). |
| Merge confidence | 1–5 integer in the review header. Python-computed. | Stole the badge idea from Greptile. |
| Reachability demotion | Defer | Needs the security graph. Note in §7. |
| Rules mining from feedback | Defer | `feedback_harvest` / `feedback_analyze` exist; distillation is a separate effort. |
| Sequence diagrams / T-rex / cross-repo | Skip | Too heavy for the pilot. |

## 1. Label removal + repo opt-in

### `.pr-review.json` schema delta

```diff
 {
+  "enabled": true,
   "focus": [...],
   "exclude_paths": [...],
   ...
 }
```

`enabled` is a top-level boolean, default `false`, read from the **base branch**
(unchanged trust rule — `fetch_repo_config(ref=base_ref)` already handles this).

### Webhook behavior (`pilot/webhook_server.py`)

- Remove constants `AI_REVIEW_LABEL`, `AI_USAGE_LABEL`. Remove
  `_labels_have_ai_review`. Remove the `report_usage` plumbing from
  `_handle_pull_request` and `_run_review`.
- New helper `is_repo_enabled(api, repo, ref, token) -> bool` in `webhook_server.py`
  (or reused via `fetch_repo_config` — see below). `False` on any failure
  (404, parse error, missing key, malformed value). Logs the reason to stderr.
- `_handle_pull_request` order of operations:
  1. `action in SKIP_ACTIONS` → `200 ignore`
  2. base_ref present + fetch config
  3. `if not config.get("enabled")` → `200 "skip (repo not opted in)"`
  4. claim in-flight slot
  5. thread off `_run_review`
- Pre-claim gate keeps opted-out repos from consuming concurrency slots on
  bursts. One extra `GET contents/.pr-review.json` per PR event (404 for
  unconfigured repos) — negligible.

### `pilot/ai_review.py` cleanup

- Delete `AI_REVIEW_LABEL`, `AI_USAGE_LABEL` constants.
- Delete `pr_has_label()` helper (its only call sites were the AI-USAGE
  re-reads at render time).
- Drop the `report_usage: bool` parameter from `review_pr()`. Always render
  the collapsible usage block when `usage` is not None.
- Remove the two `PRAGENT_USAGE_ALWAYS` references (env reads).
- Extend `parse_repo_config()` to extract `enabled` (validate is bool,
  default False).
- Extend `effective_config()` to preserve `enabled` through the style-defaults
  merge.

### Docs

- `README.md`: rewrite "Label a PR `AI-REVIEW`" + "add the AI-REVIEW label" to
  "commit `.pr-review.json: {"enabled": true}` to the default branch." Drop the
  AI-USAGE paragraph. Update the flow diagram.
- `pilot/README-webhook.md`: replace onboarding steps. Drop the per-PR label
  ceremony.
- `pilot/README.md` (CI-step path): if it still references labels, remove.

## 2. Token humanization

New helper in `pilot/ai_review.py`:

```
def fmt_tokens(n: int | None) -> str:
    """1234567 -> '1,234,567 (1.2M)'; 0 -> '0'; <1000 -> comma-form; None -> '?'."""
```

Rules:
- `None` → `"?"`.
- `n < 1000` → `f"{n:,}"` (no short suffix — most findings have ~tens of tokens).
- `1000 ≤ n < 1_000_000` → `f"{n:,} ({n/1000:.1f}K)"`, drop trailing `.0`.
- `1_000_000 ≤ n < 1_000_000_000` → `f"{n:,} ({n/1_000_000:.1f}M)"`.
- else `...B`.
- Negative inputs → `"?"` (defensive — never expected from usage dicts).

Apply in:
- `pilot/ai_review._render_collapsible_usage` — input, output, reasoning,
  cache_read, cache_write, total.
- `pilot/ai_review.inline_comment_body` — the `🪙 ~N tok (...)` per-finding
  line.

Tests: `test_fmt_tokens` golden vectors — `0`, `42`, `999`, `1000`, `1234`,
`1_234_567`, `1_234_567_890`, `None`, `-1`.

## 3. Multi-provider cost in usage section

### `pilot/cost_model.PRICES` — extend with real published rates

Source: Anthropic platform docs, OpenAI pricing, Gemini API pricing, xAI docs.
Fetched 2026-08-21. Numbers in USD per million tokens.

| key | input | output | cache_write | cache_read |
|---|---:|---:|---:|---:|
| `claude-opus-5` | 5.00 | 25.00 | 6.25 | 0.50 |
| `claude-sonnet-5` | 2.00 | 10.00 | 2.50 | 0.20 |
| `claude-haiku-4-5` | 1.00 | 5.00 | 1.25 | 0.10 |
| `gpt-5` | 1.25 | 10.00 | 1.25 | 0.125 |
| `gpt-5-mini` | 0.25 | 2.00 | 0.25 | 0.025 |
| `gemini-2.5-pro` | 1.875 | 12.50 | 1.875 | 0.1875 |
| `gemini-2.5-flash` | 0.30 | 2.50 | 0.30 | 0.03 |
| `grok-4.5` | 2.00 | 6.00 | 2.00 | 0.30 |
| `grok-4.3` | 1.25 | 2.50 | 1.25 | 0.20 |

Notes on derivation:
- Gemini 2.5 Pro publishes a tiered range (`$1.25–$2.50` in, `$10–$15` out,
  `$0.125–$0.25` cached). Midpoints are taken for a single line; the
  `compare_against` field lets a repo override per-key if precision matters.
- Providers without a separate cache_write charge (OpenAI, Gemini, Grok) set
  `cache_write = input` so the existing `cost()` formula continues to work
  without a branch on provider.
- `cost_target` (the highlighted single line) and `compare_against` (the table)
  are independent fields — see §3.2.

### 3.1 Render

Replace the single `**Est. cost on {provider}**: $X.XX` line in
`_render_collapsible_usage` with a compact markdown table:

```
**Equivalent cost on paid providers** (this run's measured tokens):

| Provider | Cost |
|---|---:|
| Claude Sonnet 5 | $4.32 |
| GPT-5 | $2.71 |
| Gemini 2.5 Pro | $4.04 |
| Grok 4.5 | $4.32 |
```

Sort cheapest-first. Skip rows whose cost is `$0.00`. Bold the row matching
`cost_target` (the user-selected highlight).

### 3.2 Config

`.pr-review.json`:
```json
{
  "enabled": true,
  "cost_target": "claude-sonnet-5",
  "compare_against": ["claude-sonnet-5", "gpt-5", "gemini-2.5-pro", "grok-4.5"]
}
```

`parse_repo_config()`:
- Validate each key exists in `PRICES`. Drop unknowns to stderr (keeps
  `cost_model._resolve_price_target`'s typo-reporting consistent).
- Cap the list at `CONFIG_MAX_LIST_ITEMS` (12).
- Default when absent: `["claude-sonnet-5", "gpt-5", "gemini-2.5-pro",
  "grok-4.5"]`.

### 3.3 Tests

`tests/pilot/test_cost_model.py`:
- Add equivalent-cost golden vectors against the new price keys.
- Update `test_observed_report_prices_every_model` and
  `test_report_renders_every_requested_model` to cover the new keys.
- Add `test_compare_against_parsing` (valid / unknown / over-cap / missing).

## 4. Richer review summary

### 4.1 Schema additions (agent prompts + `SYSTEM_PROMPT`)

```json
{
  "walkthrough": [
    "file X: does Y",
    "file Z: refactors W"
  ],
  "risk_verdict": "Medium risk: changes auth middleware without adding tests.",
  "test_coverage": "No tests for behavioral change in pilot/foo.py."
}
```

Rules (added to `.opencode/agents/pragent.md`, each lens agent `.md`, and the
ollama `SYSTEM_PROMPT`):
- `walkthrough`: 2–6 bullets, file- or change-grouped, plain prose (no
  severity emoji). Skip if the diff is one obvious line.
- `risk_verdict`: exactly one line. Lead with `Low|Medium|High|Critical risk:`
  followed by a concrete reason grounded in the diff.
- `test_coverage`: short string. One of
  `Tests added` / `Tests changed` / `No tests for behavioral change` /
  `No test files in repo` / a repo-specific free-text override from
  `instructions`.

### 4.2 Parsing

Extend `parse_review_output(text)` and the lens fan-out's synthetic-text
builder (`pilot/opencode_review.run_lenses_review`) to emit these three
fields in the final JSON block. Empty defaults preserve backward compat with
agents that haven't been re-deployed yet.

### 4.3 Python fallback (when fields are empty)

The multi-lens fan-out already synthesizes the findings JSON in Python today;
add a `_synthesize_summary_fields(findings, diff) -> dict` helper that
computes:
- `walkthrough`: group `merged` findings by `path`, one bullet per path
  containing the peak severity emoji and the first-problem truncated to ~80
  chars. If `merged` is empty, list `changed_files(diff)` with the size of
  the diff as the body ("`pilot/foo.py` — +12 lines").
- `risk_verdict`: from `sev_counts` and `_multi_lens` flags:
  - any critical → `Critical risk: <N> critical finding(s).`
  - any high → `High risk: <N> high finding(s).`
  - any medium → `Medium risk: <N> medium finding(s) (<lens> lens).`
  - else `Low risk: clean or minor nits only.`
- `test_coverage`: scan `changed_files(diff)` with `is_test_path()`. Three
  buckets:
  - any test path changed alongside non-test paths → `Tests added` (or
    `Tests changed`).
  - non-test paths present, no test path → `No tests for behavioral change in
    <first non-test path>.`
  - no test paths at all and non-test paths present → `No tests for
    behavioral change in <first non-test path>.` (same as above; the
    distinction "no test files in repo" needs a tree scan — keep it simple
    for v1).

### 4.4 Render

Extend `format_review_body()` to render three new sections between
`### Summary of Changes` and `### Key Risks & Concerns`:

```
### Risk Verdict
🟡 Medium risk: changes auth middleware without adding tests.

### Walkthrough
- `pilot/foo.py` — adds retry logic for transient Gitea API errors
- `pilot/bar.py` — extracts shared header parser

### Test Coverage
No tests for behavioral change in pilot/foo.py.
```

Each section renders an `_No <section> provided._` placeholder when empty
(matches the existing `Summary of Changes` / `Key Risks & Concerns` collapse
behavior).

### 4.5 Tests

`tests/pilot/test_ai_review.py`:
- Golden vectors for each new section (provided + Python-fallback paths).
- Combined body test: summary + walkthrough + risk + tests + table +
  collapsible usage all render in the right order with no orphan markers.

## 6. Stolen ideas

### 6.1 Merge confidence 1–5 (Greptile)

New function `merge_confidence(findings: list[dict]) -> int` in
`pilot/ai_review.py`:

```
start at 5
-1 if any critical finding
-1 if any high finding
-1 if any medium finding
-1 if any _multi_lens: True finding (cross-lens agreement = harder to dismiss)
clamp to [1, 5]
```

Render in `REVIEW_HEADER`:

```
🤖 **AI Review** · pragent pilot · glm-5.2:cloud · `abc12345` · Merge confidence: 3/5 🟡
```

Badge map: 5/4 = 🟢, 3 = 🟡, 2 = 🟠, 1 = 🔴.

Tests: golden vectors for all 5 score branches.

### 6.2 Add `trivial` + `info` severity levels (CodeRabbit)

Extend `SEVERITIES` and `SEVERITY_RANK`:

```
SEVERITIES = ("critical", "high", "medium", "low", "trivial", "info")
SEVERITY_RANK = {"info": -1, "trivial": 0, "low": 1, "medium": 2, "high": 3, "critical": 4}
```

Update `_severity_badge` emoji map (`trivial`/`info` = ⚪). Update
`apply_repo_config` threshold semantics so `medium+` still means what it
meant (only `low` ranks below `medium` is unchanged). Update agent prompts
to permit emitting `trivial` / `info`. Backward compat: `_normalize_finding`
already coerces unknown severities to `medium`.

Tests: existing `test_apply_repo_config` cases keep passing; add
`test_severity_threshold_respects_new_levels` and
`test_unknown_severity_normalizes_to_medium`.

### 6.3 Reachability-aware severity demotion — DEFER

CodeRabbit Security demotes severity by one level when a vulnerability is
unreachable / only theoretically exploitable. We can't compute reachability
without the security graph. Document in §7 and revisit when a Code-Rabbit-
style graph index lands.

### 6.4 Rules mining from feedback — DEFER

`pilot/feedback_harvest.py` + `pilot/feedback_analyze.py` exist. A future
`pilot/learn_rules.py` cron job will distill FP-vote signals into
`.pr-review.learned.json` and merge into `instructions`. Document in §7.

### 6.5 Sequence diagrams / T-rex / cross-repo — SKIP

Too heavy for the pilot's footprint. Document in §7.

## 7. Deferred (not in this update)

- **Reachability-aware severity demotion.** Requires a Code-Rabbit-style
  reachability graph over the repo.
- **Rules mining from feedback.** A `learn_rules.py` job that consumes the
  feedback DB and writes `.pr-review.learned.json`. `feedback_harvest` /
  `feedback_analyze` are the substrate.
- **Sequence diagrams / T-rex sandbox / cross-repo review.** Three features
  Greptile / Qodo highlight. All require either a code graph index (heavy
  precompute) or sandbox runtime execution (separate infra). Skip.
- **Per-finding confidence scores.** Greptile publishes a 0–5 score on every
  comment. We deliberately stay on severity — confidence on findings
  requires the agent to self-estimate, which is unreliable without a
  cross-lens consensus check. The merge-confidence badge (§6.1) is the
  higher-signal version of the same idea.
- **`Fix with Cursor` handoff.** Greptile ships a one-click "send all findings
  to Cursor/Codex/Claude Code." Our users *are* the bot's host, not an
  external coding IDE. Skip.
- **Cost-model batch column.** The cost model already prices batch at 50%;
  the PR-review path will never use it (stateful agent loops aren't
  batchable). Keep the column for completeness, no new work.

## 8. Risk register

| Risk | Mitigation |
|---|---|
| Webhook floods the API with `.pr-review.json` fetches on a large owner | One `GET` per PR event, mostly 404. Documented acceptable. The dedicated `/health` already reports inflight count. |
| `.pr-review.json:enabled` set on a high-traffic repo creates surprise review load | The README will document the opt-in explicitly. The webhook's `PRAGENT_MAX_CONCURRENT_REVIEWS` already bounds the spawn rate. |
| New severity levels (`trivial` / `info`) break repos that filter on `medium+` | `apply_repo_config` threshold semantics preserve the rank of `low` and `medium`. `trivial` ranks below `low`, `info` below `trivial`. New filters naturally include them. |
| Token humanization loses precision a maintainer relies on | `fmt_tokens` always keeps the full comma-separated number; the short suffix is a parenthetical. |
| Multi-provider cost table is misleading when a provider has tiered pricing | `compare_against` is a per-repo override. The README documents the midpoints for Gemini 2.5 Pro. |
| Agent prompt change for `walkthrough` / `risk_verdict` / `test_coverage` causes regressions on deployed agents | Python fallback (§4.3) synthesizes the same fields when the agent omits them. Backward compat preserved by empty defaults. |
| Removing `report_usage` breaks the `review_pr` tests that pass it | Test updates are part of this update. |
| Removing labels breaks users who still apply them | No Gitea API change is needed; the labels just stop being read. A one-paragraph README note acknowledges the change. |

## 9. Prioritized implementation list

| # | Item | Section | Effort |
|---|---|---|---|
| P0 | Label removal + repo opt-in (`enabled` in `.pr-review.json`) | §1 | M |
| P1 | `fmt_tokens()` helper + apply in usage + inline | §2 | S |
| P1 | Multi-provider cost table (extend `PRICES`, render table, `compare_against`) | §3 | M |
| P2 | Richer summary (`walkthrough` / `risk_verdict` / `test_coverage`) schema + Python fallback | §4 | L |
| P2 | `trivial` + `info` severity levels | §6.2 | S |
| P3 | Merge confidence 1–5 in review header | §6.1 | S |
| P3 | README + `pilot/README-webhook.md` rewrite | §1, §10 | S |
| P3 | Test updates across all sections | (each) | M |

P0 first because it changes webhook behavior (must land with the repo-opt-in
docs so onboarding isn't broken mid-rollout). P1 items are independent and
small — ship together. P2 ships the user-visible summary improvement.