1608 lines
50 KiB
Markdown
1608 lines
50 KiB
Markdown
# pragent Update Implementation Plan
|
||
|
||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||
|
||
**Goal:** Replace the AI-REVIEW/AI-USAGE label gates with a per-repo `.pr-review.json:enabled` opt-in, humanize token counts, extend the cost model with GPT / Gemini / Grok equivalents, enrich the PR review summary (walkthrough + risk verdict + test coverage + merge confidence), and add `trivial` / `info` severity levels — all per the approved design in `docs/plans/2026-08-21-pragent-update-design.md`.
|
||
|
||
**Architecture:** Additive + one behavior change (the label gate). All work lives in `pilot/` (stdlib Python), `tests/pilot/`, and docs. New config fields land in `parse_repo_config()` so repos opt in one field at a time. New helpers (`fmt_tokens`, `merge_confidence`, `_synthesize_summary_fields`, `is_repo_enabled`) are pure where possible so they're cheap to unit-test.
|
||
|
||
**Tech Stack:** Python stdlib (no new deps). pytest for tests. opencode subprocess + Gitea webhook (unchanged).
|
||
|
||
**Repo:** `~/Projects/pragent` (Gitea: `gitea_admin/pragent`). Design: `docs/plans/2026-08-21-pragent-update-design.md`.
|
||
|
||
---
|
||
|
||
## Ground rules
|
||
|
||
- **TDD, strictly.** Failing test → minimum implementation → passing test → commit. A test that passes before the code lands is a broken test.
|
||
- **No network in unit tests.** All tests at `tests/pilot/*` are stdlib-only.
|
||
- **Stdlib only.** No `pip install`. Pure Python.
|
||
- **Commit after every task.** Conventional Commits, imperative subject under 50 chars. End with `Co-Authored-By: Claude <noreply@anthropic.com>`.
|
||
- **Preserve existing semantics.** The design says backward compat (empty defaults, `unknown severity → medium`, label usage still legal in Gitea). Don't tighten gates you didn't explicitly open.
|
||
- **Task order is dependency-ordered.** Don't reorder. If a task blocks, stop and surface the blocker.
|
||
|
||
---
|
||
|
||
## Task 1: Add GPT / Gemini / Grok prices to `cost_model.PRICES`
|
||
|
||
**Files:**
|
||
- Modify: `pilot/cost_model.py:78-85`
|
||
- Modify: `tests/pilot/test_cost_model.py`
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
Append to `tests/pilot/test_cost_model.py`:
|
||
|
||
```python
|
||
NEW_KEYS = ("gpt-5", "gpt-5-mini", "gemini-2.5-pro",
|
||
"gemini-2.5-flash", "grok-4.5", "grok-4.3")
|
||
|
||
|
||
def test_prices_contains_new_providers():
|
||
for k in NEW_KEYS:
|
||
assert k in cm.PRICES, k
|
||
|
||
|
||
def test_cost_matches_published_gpt5():
|
||
# $1.25 in / $10.00 out / cached $0.125; cache_write = input
|
||
u = cm.Usage(uncached_input=1_000_000, cached_input=1_000_000,
|
||
cache_writes=1_000_000, output=1_000_000)
|
||
assert abs(cm.cost(u, cm.PRICES["gpt-5"]) - (1.25 + 0.125 + 1.25 + 10.00)) < 1e-9
|
||
|
||
|
||
def test_cost_matches_published_gemini_flash():
|
||
# $0.30 in / $2.50 out / cached $0.03; cache_write = input
|
||
u = cm.Usage(uncached_input=2_000_000, cached_input=0,
|
||
cache_writes=0, output=500_000)
|
||
expected = 2.00 * 0.30 + 0.50 * 2.50 # $0.60 + $1.25
|
||
assert abs(cm.cost(u, cm.PRICES["gemini-2.5-flash"]) - expected) < 1e-9
|
||
|
||
|
||
def test_cost_matches_published_grok45():
|
||
# $2.00 in / $6.00 out / cached $0.30; cache_write = input
|
||
u = cm.Usage(uncached_input=1_000_000, cached_input=1_000_000,
|
||
cache_writes=1_000_000, output=1_000_000)
|
||
assert abs(cm.cost(u, cm.PRICES["grok-4.5"]) - (2.00 + 0.30 + 2.00 + 6.00)) < 1e-9
|
||
```
|
||
|
||
**Step 2: Run tests, watch them fail**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_cost_model.py -v -k "new_providers or matches_published"
|
||
```
|
||
|
||
Expected: 4 failures (KeyError on the new keys).
|
||
|
||
**Step 3: Extend `PRICES`**
|
||
|
||
In `pilot/cost_model.py`, replace the existing `PRICES` dict with:
|
||
|
||
```python
|
||
PRICES: dict[str, Price] = {
|
||
# Anthropic
|
||
"claude-opus-5": Price("Claude Opus 5", 5.00, 25.00, 6.25, 0.50),
|
||
"claude-sonnet-5": Price("Claude Sonnet 5", 2.00, 10.00, 2.50, 0.20),
|
||
"claude-haiku-4-5": Price("Claude Haiku 4.5", 1.00, 5.00, 1.25, 0.10),
|
||
# OpenAI — cached_input 0.1x, no separate cache_write (writes bill as input)
|
||
"gpt-5": Price("GPT-5", 1.25, 10.00, 1.25, 0.125),
|
||
"gpt-5-mini": Price("GPT-5 mini", 0.25, 2.00, 0.25, 0.025),
|
||
# Google Gemini — same model: cache_write = input
|
||
"gemini-2.5-pro": Price("Gemini 2.5 Pro", 1.875, 12.50, 1.875, 0.1875),
|
||
"gemini-2.5-flash": Price("Gemini 2.5 Flash", 0.30, 2.50, 0.30, 0.03),
|
||
# xAI Grok — same model
|
||
"grok-4.5": Price("Grok 4.5", 2.00, 6.00, 2.00, 0.30),
|
||
"grok-4.3": Price("Grok 4.3", 1.25, 2.50, 1.25, 0.20),
|
||
}
|
||
```
|
||
|
||
**Step 4: Run tests, watch them pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_cost_model.py -v
|
||
```
|
||
|
||
Expected: all green.
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/cost_model.py tests/pilot/test_cost_model.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(cost-model): add GPT, Gemini, Grok prices" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 2: `fmt_tokens()` helper + tests
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py` (add helper near the top after imports)
|
||
- Modify: `tests/pilot/test_ai_review.py`
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
from ai_review import fmt_tokens
|
||
|
||
|
||
def test_fmt_tokens_zero():
|
||
assert fmt_tokens(0) == "0"
|
||
|
||
|
||
def test_fmt_tokens_small_no_short():
|
||
assert fmt_tokens(42) == "42"
|
||
assert fmt_tokens(999) == "999"
|
||
|
||
|
||
def test_fmt_tokens_thousands():
|
||
assert fmt_tokens(1000) == "1,000 (1.0K)"
|
||
assert fmt_tokens(1234) == "1,234 (1.2K)"
|
||
assert fmt_tokens(9999) == "9,999 (10.0K)"
|
||
|
||
|
||
def test_fmt_tokens_millions():
|
||
assert fmt_tokens(1_000_000) == "1,000,000 (1.0M)"
|
||
assert fmt_tokens(2_071_025) == "2,071,025 (2.1M)"
|
||
assert fmt_tokens(1_234_567) == "1,234,567 (1.2M)"
|
||
|
||
|
||
def test_fmt_tokens_billions():
|
||
assert fmt_tokens(1_234_567_890) == "1,234,567,890 (1.2B)"
|
||
|
||
|
||
def test_fmt_tokens_none():
|
||
assert fmt_tokens(None) == "?"
|
||
|
||
|
||
def test_fmt_tokens_negative():
|
||
assert fmt_tokens(-1) == "?"
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k fmt_tokens
|
||
```
|
||
|
||
Expected: ImportError.
|
||
|
||
**Step 3: Implement `fmt_tokens`**
|
||
|
||
In `pilot/ai_review.py`, add (after the constants block):
|
||
|
||
```python
|
||
def fmt_tokens(n) -> str:
|
||
"""1234567 -> '1,234,567 (1.2M)'; 0 -> '0'; <1000 -> comma-only; None/negative -> '?'.
|
||
|
||
Always returns the full comma-separated number; the short suffix is a
|
||
parenthetical for fast scanning. Caps at B; the cost model never exceeds M.
|
||
"""
|
||
if n is None:
|
||
return "?"
|
||
if not isinstance(n, (int, float)) or n < 0:
|
||
return "?"
|
||
n = int(n)
|
||
if n < 1000:
|
||
return f"{n:,}"
|
||
if n < 1_000_000:
|
||
return f"{n:,} ({n / 1000:.1f}K)"
|
||
if n < 1_000_000_000:
|
||
return f"{n:,} ({n / 1_000_000:.1f}M)"
|
||
return f"{n:,} ({n / 1_000_000_000:.1f}B)"
|
||
```
|
||
|
||
**Step 4: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k fmt_tokens
|
||
```
|
||
|
||
Expected: 7 pass.
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(ai_review): fmt_tokens() humanizes token counts" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 3: Apply `fmt_tokens` in `_render_collapsible_usage` and inline comments
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:_render_collapsible_usage` (use `fmt_tokens` for the line that says `f"{in_tok} in / {out_tok} out ..."`)
|
||
- Modify: `pilot/ai_review.py:inline_comment_body` (use `fmt_tokens(tok)`)
|
||
|
||
**Step 1: Add failing test**
|
||
|
||
```python
|
||
def test_collapsible_usage_renders_humanized_tokens(capsys):
|
||
usage = {"input": 2_071_025, "output": 17303, "reasoning": 0,
|
||
"cache_read": 0, "cache_write": 0, "total": 2_088_328,
|
||
"cost": 0.0, "steps": 1, "duration_s": 10.0}
|
||
block = _render_collapsible_usage(usage, "glm-5.2:cloud", config={})
|
||
assert "2,071,025 (2.1M) in" in block
|
||
assert "17,303 (17.3K) out" in block
|
||
|
||
|
||
def test_inline_comment_body_humanized_tokens():
|
||
f = {"severity": "medium", "path": "x.py", "line": 1,
|
||
"problem": "p", "fix": "", "suggestion": "", "reference": "",
|
||
"_tok_attrib": 362, "_tok_pct": 0.11}
|
||
body = inline_comment_body(f)
|
||
assert "362 (0.4K)" in body # 362 < 1000 -> "362" only; see note below
|
||
```
|
||
|
||
Note: `_tok_attrib` values are usually small (<1K), so `fmt_tokens` returns the comma form alone. Adjust the assertion to match reality (362 → "362"). Use this corrected assertion:
|
||
|
||
```python
|
||
assert "🪙 ~362 tok" in body
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "humanized_tokens"
|
||
```
|
||
|
||
Expected: FAIL (current output has raw integers, no comma + no short).
|
||
|
||
**Step 3: Edit `_render_collapsible_usage`**
|
||
|
||
Replace the `**Total Tokens**` line with:
|
||
|
||
```python
|
||
f"- **Total Tokens**: {fmt_tokens(in_tok)} in / {fmt_tokens(out_tok)} out "
|
||
f"({fmt_tokens(reason_tok)} reasoning, cache {fmt_tokens(cache_r)} read / "
|
||
f"{fmt_tokens(cache_w)} write, {fmt_tokens(total)} total)",
|
||
```
|
||
|
||
**Step 4: Edit `inline_comment_body`**
|
||
|
||
Replace the `🪙 ~{tok} tok ...` line with:
|
||
|
||
```python
|
||
body += f"\n\n🪙 ~{fmt_tokens(tok)} tok ({pct:.0f}% · attributed output)"
|
||
```
|
||
|
||
**Step 5: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "humanized_tokens"
|
||
```
|
||
|
||
Expected: 2 pass.
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(ai_review): render humanized token counts in usage + inline" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 4: Add `trivial` + `info` severity levels
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:SEVERITIES`, `SEVERITY_RANK`, `_SEVERITY_EMOJI`
|
||
- Modify: `pilot/ai_review.py:_normalize_finding` (no change — already coerces unknowns to medium)
|
||
- Modify: `pilot/ai_review.py:apply_repo_config` (threshold semantics)
|
||
- Modify: `tests/pilot/test_ai_review.py`
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
def test_severities_includes_trivial_and_info():
|
||
assert "trivial" in SEVERITIES
|
||
assert "info" in SEVERITIES
|
||
|
||
|
||
def test_severity_rank_orders_new_levels():
|
||
assert SEVERITY_RANK["info"] < SEVERITY_RANK["trivial"] < SEVERITY_RANK["low"]
|
||
|
||
|
||
def test_threshold_medium_keeps_low_below_trivial_below_info():
|
||
# medium+ drops info and trivial; severity_threshold=medium keeps medium+trivial+low
|
||
cfg = = {"style": "lenient", "severity_threshold": "medium"}
|
||
findings = [
|
||
{"severity": "info", "path": "a", "line": 1, "problem": "", "fix": "", "suggestion": "", "reference": ""},
|
||
{"severity": "trivial", "path": "b", "line": 1, "problem": "", "fix": "", "suggestion": "", "reference": ""},
|
||
{"severity": "low", "path": "c", "line": 1, "problem": "", "fix": "", "suggestion": "", "reference": ""},
|
||
{"severity": "medium", "path": "d", "line": 1, "problem": "", "fix": "", "suggestion": "", "reference": ""},
|
||
]
|
||
kept, dropped = apply_repo_config(findings, cfg, changed_paths=["x.py"])
|
||
sev_kept = sorted(f["severity"] for f in kept)
|
||
assert "info" in [f["severity"] for f in dropped]
|
||
assert "trivial" in [f["severity"] for f in dropped]
|
||
assert "low" not in [f["severity"] for f in dropped] # low passes medium threshold? NO — medium=2, low=1; low dropped
|
||
assert "medium" in sev_kept
|
||
|
||
|
||
def test_unknown_severity_still_normalizes_to_medium():
|
||
# Backward compat
|
||
n = _normalize_finding({"severity": "emergency", "path": "x", "line": 1, "problem": "p"})
|
||
assert n["severity"] == "medium"
|
||
```
|
||
|
||
Note: re-derive expected behavior from the design — `medium+` threshold means
|
||
`low` (rank 1) is **dropped**, `trivial` (rank 0) and `info` (rank −1) are also
|
||
dropped. Update the test assertions to match this if my draft is wrong.
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "trivial or new_levels"
|
||
```
|
||
|
||
Expected: failures.
|
||
|
||
**Step 3: Update constants**
|
||
|
||
```python
|
||
SEVERITIES = ("critical", "high", "medium", "low", "trivial", "info")
|
||
SEVERITY_RANK = {"info": -1, "trivial": 0, "low": 1, "medium": 2, "high": 3, "critical": 4}
|
||
|
||
_SEVERITY_EMOJI = {
|
||
"critical": "🔴",
|
||
"high": "🔴",
|
||
"medium": "🟡",
|
||
"low": "🔵",
|
||
"trivial": "⚪",
|
||
"info": "⚪",
|
||
}
|
||
```
|
||
|
||
**Step 4: Update `_severity_badge` if needed**
|
||
|
||
Already handles unknown → INFO. Extend label set in the existing tuple check:
|
||
|
||
```python
|
||
label = sev.upper() if sev in {"critical", "high", "medium", "low", "trivial", "info"} else "INFO"
|
||
```
|
||
|
||
**Step 5: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "trivial or new_levels"
|
||
```
|
||
|
||
**Step 6: Run full suite (catch regressions)**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests -q
|
||
```
|
||
|
||
Expected: ~137 + 2 pass.
|
||
|
||
**Step 7: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(severity): add trivial + info levels" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 5: `enabled` + `compare_against` in `parse_repo_config`
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:parse_repo_config` (add `enabled` extraction; add `compare_against` list parsing with cap)
|
||
- Modify: `tests/pilot/test_ai_review.py`
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
def test_parse_repo_config_enabled_true():
|
||
cfg = parse_repo_config('{"enabled": true}')
|
||
assert cfg.get("enabled") is True
|
||
|
||
|
||
def test_parse_repo_config_enabled_false_explicit():
|
||
cfg = parse_repo_config('{"enabled": false}')
|
||
assert cfg.get("enabled") is False
|
||
|
||
|
||
def test_parse_repo_config_enabled_missing_defaults_false():
|
||
cfg = parse_repo_config('{}')
|
||
assert cfg.get("enabled") is False
|
||
|
||
|
||
def test_parse_repo_config_enabled_wrong_type_ignored():
|
||
cfg = parse_repo_config('{"enabled": "yes"}')
|
||
assert cfg.get("enabled") is False
|
||
|
||
|
||
def test_parse_repo_config_compare_against_default_absent():
|
||
# The default is applied in render code, not in parse_repo_config.
|
||
cfg = parse_repo_config('{}')
|
||
assert "compare_against" not in cfg # explicit absence
|
||
|
||
|
||
def test_parse_repo_config_compare_against_valid():
|
||
cfg = parse_repo_config(
|
||
'{"compare_against": ["claude-sonnet-5", "gpt-5", "gemini-2.5-pro"]}')
|
||
assert cfg["compare_against"] == ["claude-sonnet-5", "gpt-5", "gemini-2.5-pro"]
|
||
|
||
|
||
def test_parse_repo_config_compare_against_drops_unknown_keys(capfd):
|
||
cfg = parse_repo_config(
|
||
'{"compare_against": ["claude-sonnet-5", "bogus-1", "gpt-5"]}')
|
||
assert "bogus-1" not in cfg["compare_against"]
|
||
assert "claude-sonnet-5" in cfg["compare_against"]
|
||
captured = capfd.readouterr()
|
||
assert "bogus-1" in captured.err
|
||
|
||
|
||
def test_parse_repo_config_compare_against_caps_at_12():
|
||
raw_keys = [f"claude-sonnet-5" if i == 0 else f"bogus-{i}" for i in range(20)]
|
||
# Only first key is valid; rest are dropped, so list should be 1.
|
||
cfg = parse_repo_config(json.dumps({"compare_against": raw_keys}))
|
||
assert len(cfg["compare_against"]) == 1
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "enabled or compare_against"
|
||
```
|
||
|
||
**Step 3: Extend `parse_repo_config`**
|
||
|
||
Add before the `return out` line:
|
||
|
||
```python
|
||
en = data.get("enabled")
|
||
if isinstance(en, bool):
|
||
out["enabled"] = en
|
||
|
||
# Compare-against list: validate against cost_model.PRICES, drop unknowns.
|
||
# Lazy import: cost_model has no dep on ai_review and we keep import-time
|
||
# cost low for the ollama path.
|
||
from cost_model import PRICES as _PRICES
|
||
ca = data.get("compare_against")
|
||
if isinstance(ca, list):
|
||
cleaned = []
|
||
for x in ca:
|
||
if isinstance(x, str) and x.strip() in _PRICES:
|
||
cleaned.append(x.strip())
|
||
elif isinstance(x, str):
|
||
import sys
|
||
print(
|
||
f"pragent: ignoring compare_against entry {x!r} "
|
||
f"(not in cost_model.PRICES); valid: {', '.join(sorted(_PRICES))}",
|
||
file=sys.stderr, flush=True,
|
||
)
|
||
if cleaned:
|
||
out["compare_against"] = cleaned[:12]
|
||
```
|
||
|
||
**Step 4: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "enabled or compare_against"
|
||
```
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(config): parse enabled + compare_against" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 6: `merge_confidence()` + render in `REVIEW_HEADER`
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py` (add `merge_confidence`, update `REVIEW_HEADER`)
|
||
|
||
**Step 1: Add failing test**
|
||
|
||
```python
|
||
def test_merge_confidence_clean_is_five():
|
||
assert merge_confidence([]) == 5
|
||
|
||
|
||
def test_merge_confidence_only_low_is_five():
|
||
f = {"severity": "low"}
|
||
assert merge_confidence([f, f, f]) == 5
|
||
|
||
|
||
def test_merge_confidence_medium_drops_one():
|
||
f = {"severity": "medium"}
|
||
assert merge_confidence([f]) == 4
|
||
|
||
|
||
def test_merge_confidence_high_drops_two():
|
||
f = {"severity": "high"}
|
||
assert merge_confidence([f]) == 3
|
||
|
||
|
||
def test_merge_confidence_critical_drops_to_one():
|
||
f = {"severity": "critical"}
|
||
assert merge_confidence([f]) == 1
|
||
|
||
|
||
def test_merge_confidence_multi_lens_drops_extra():
|
||
f = {"severity": "low", "_multi_lens": True}
|
||
assert merge_confidence([f]) == 4 # 5 (clean) - 1 (multi-lens noise) = 4
|
||
|
||
|
||
def test_merge_confidence_clamped():
|
||
# Multiple criticals shouldn't go below 1
|
||
f = {"severity": "critical"}
|
||
assert merge_confidence([f, f, f]) == 1
|
||
|
||
|
||
def test_review_header_includes_confidence():
|
||
h = REVIEW_HEADER.format(model="glm-5.2:cloud", sha="abc1234567", confidence="3/5 🟡")
|
||
assert "Merge confidence: 3/5 🟡" in h
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
**Step 3: Implement**
|
||
|
||
Add `merge_confidence` near `compute_attribution`:
|
||
|
||
```python
|
||
_CONFIDENCE_BADGE = {5: "🟢", 4: "🟢", 3: "🟡", 2: "🟠", 1: "🔴"}
|
||
|
||
|
||
def merge_confidence(findings: list[dict]) -> int:
|
||
"""1-5 merge verdict: higher = safer. Deductions:
|
||
-1 per max severity tier present (critical/high/medium),
|
||
-1 if any cross-lens agreed finding.
|
||
"""
|
||
if not findings:
|
||
return 5
|
||
max_rank = max(SEVERITY_RANK.get(f.get("severity", "low"), 0) for f in findings)
|
||
if max_rank >= SEVERITY_RANK["critical"]:
|
||
score = 1
|
||
elif max_rank >= SEVERITY_RANK["high"]:
|
||
score = 3
|
||
elif max_rank >= SEVERITY_RANK["medium"]:
|
||
score = 4
|
||
else:
|
||
score = 5
|
||
if any(f.get("_multi_lens") for f in findings):
|
||
score -= 1
|
||
return max(1, min(5, score))
|
||
```
|
||
|
||
Update `REVIEW_HEADER`:
|
||
|
||
```python
|
||
REVIEW_HEADER = "🤖 **AI Review** · pragent pilot · {model} · `{sha}` · Merge confidence: {confidence}"
|
||
```
|
||
|
||
**Step 4: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "merge_confidence or review_header"
|
||
```
|
||
|
||
**Step 5: Update the single internal caller**
|
||
|
||
`format_review_body` formats `REVIEW_HEADER`. Add a `confidence` kwarg
|
||
(int 1–5, default 5) and an emoji lookup. Thread it from `review_pr`:
|
||
|
||
In `format_review_body(...)` signature: add `confidence: int = 5`.
|
||
Compute badge and pass into the header.
|
||
|
||
```python
|
||
badge = _CONFIDENCE_BADGE[max(1, min(5, confidence))]
|
||
confidence_str = f"{max(1, min(5, confidence))}/5 {badge}"
|
||
header = REVIEW_HEADER.format(model=model, sha=sha[:8] if sha else "unknown",
|
||
confidence=confidence_str)
|
||
```
|
||
|
||
In `review_pr`, after the synthesizer:
|
||
|
||
```python
|
||
confidence = merge_confidence(findings)
|
||
summary_body = format_review_body(..., confidence=confidence)
|
||
```
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(ai_review): per-PR merge confidence 1-5 in header" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 7: Add `walkthrough` / `risk_verdict` / `test_coverage` to review schema
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:parse_review_output` (extract three new fields)
|
||
- Modify: `pilot/ai_review.py:SYSTEM_PROMPT` (document new fields, backward-compat)
|
||
- Modify: `pilot/ai_review.py:_normalize_finding` (no change)
|
||
- Modify: `pilot/opencode_review.py:_run_one_lens` / `synthesize` (synthetic text adds empty fields — backward compat)
|
||
- Modify: `.opencode/agents/pragent.md` (document new fields)
|
||
- Modify: `.opencode/agents/<lens>.md` (document new fields, each lens gets the same instructions block)
|
||
- Modify: `pilot/opencode_review.py:run_lenses_review` (Python fallback — see Task 8)
|
||
- Modify: `tests/pilot/test_ai_review.py`
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
def test_parse_review_output_extracts_walkthrough_risk_tests():
|
||
text = '''```json
|
||
{
|
||
"summary": "x",
|
||
"summary_changes": [],
|
||
"risks": [],
|
||
"walkthrough": ["a.py: adds X", "b.py: refactors Y"],
|
||
"risk_verdict": "Low risk.",
|
||
"test_coverage": "No tests for behavioral change in a.py.",
|
||
"findings": []
|
||
}
|
||
```'''
|
||
summary, findings, changes,, risks,, risk_verdict, walkthrough, test_coverage = \
|
||
parse_review_output_extended(text)
|
||
assert walkthrough == ["a.py: adds X", "b.py: refactors Y"]
|
||
assert risk_verdict == "Low risk."
|
||
assert test_coverage == "No tests for behavioral change in a.py."
|
||
|
||
|
||
def test_parse_review_output_missing_fields_default_empty():
|
||
summary, findings, changes,, risks,, risk_verdict, walkthrough, test_coverage = \
|
||
parse_review_output_extended('{"summary":"x","findings":[]}')
|
||
assert walkthrough == []
|
||
assert risk_verdict == ""
|
||
assert test_coverage == ""
|
||
```
|
||
|
||
**Step 2: Extend `parse_review_output`**
|
||
|
||
Either add a new `parse_review_output_extended` or extend the existing
|
||
4-tuple to a 7-tuple. The existing 4-tuple is consumed in one place
|
||
(`review_pr`); change the signature and update the caller.
|
||
|
||
In `pilot/ai_review.py`:
|
||
|
||
```python
|
||
def parse_review_output(text: str) -> tuple[str, list[dict], list[str], list[str], str, list[str], str]:
|
||
"""Returns (summary, findings, summary_changes, risks, risk_verdict, walkthrough, test_coverage)."""
|
||
blob = _last_json_block(text)
|
||
if blob is None:
|
||
return "", [], [], [], "", [], ""
|
||
try:
|
||
data = json.loads(blob)
|
||
except json.JSONDecodeError:
|
||
return "", [], [], [], "", [], ""
|
||
summary = ""
|
||
summary_changes: list[str] = []
|
||
risks: list[str] = []
|
||
findings_raw = None
|
||
risk_verdict = ""
|
||
walkthrough: list[str] = []
|
||
test_coverage = ""
|
||
if isinstance(data, dict):
|
||
summary = str(data.get("summary", "") or "").strip()
|
||
summary_changes = _string_list(data.get("summary_changes"))
|
||
risks = _string_list(data.get("risks"))
|
||
findings_raw = data.get("findings")
|
||
risk_verdict = str(data.get("risk_verdict", "") or "").strip()
|
||
walkthrough = _string_list(data.get("walkthrough"))
|
||
test_coverage = str(data.get("test_coverage", "") or "").strip()
|
||
elif isinstance(data, list):
|
||
findings_raw = data
|
||
else:
|
||
return "", [], [], [], "", [], ""
|
||
out = []
|
||
if isinstance(findings_raw, list):
|
||
for f in findings_raw:
|
||
n = _normalize_finding(f)
|
||
if n is not None:
|
||
out.append(n)
|
||
return summary, out, summary_changes, risks, risk_verdict, walkthrough, test_coverage
|
||
```
|
||
|
||
**Step 3: Update `review_pr` caller**
|
||
|
||
```python
|
||
review_summary, findings, summary_changes, risks, risk_verdict, walkthrough, test_coverage = \
|
||
parse_review_output(stdout)
|
||
```
|
||
|
||
**Step 4: Update `SYSTEM_PROMPT` and agent prompts**
|
||
|
||
Append to the JSON-shape section of `SYSTEM_PROMPT`:
|
||
|
||
```text
|
||
- `walkthrough`: 2-6 short bullets, file- or change-grouped, plain prose.
|
||
- `risk_verdict`: exactly one line. Lead with "Low|Medium|High|Critical risk:" followed by a concrete reason.
|
||
- `test_coverage`: short string. One of "Tests added" / "Tests changed" / "No tests for behavioral change" / "No test files in repo".
|
||
All three default to empty when not applicable. Backward compatible.
|
||
```
|
||
|
||
In `.opencode/agents/pragent.md` + each `.opencode/agents/<lens>.md`,
|
||
append a parallel block to the Output schema section. Same wording.
|
||
|
||
**Step 5: Update lens synthesis (`run_lenses_review`)**
|
||
|
||
Compute the three fields in Python when the fan-out is engaged (Task 8 builds
|
||
the helper). For now, ensure the synthesized JSON includes empty defaults:
|
||
|
||
```python
|
||
clean_findings = [
|
||
{k: v for k, v in f.items() if not k.startswith("_")}
|
||
for f in merged
|
||
]
|
||
walkthrough, risk_verdict, test_coverage = _synthesize_summary_fields(merged, diff)
|
||
text = (
|
||
f"{summary}\n\n"
|
||
f"## Findings (multi-lens)\n\n"
|
||
f"```json\n{json.dumps({
|
||
'summary': summary,
|
||
'walkthrough': walkthrough,
|
||
'risk_verdict': risk_verdict,
|
||
'test_coverage': test_coverage,
|
||
'findings': clean_findings,
|
||
}, indent=2)}\n```\n"
|
||
)
|
||
```
|
||
|
||
`_synthesize_summary_fields` is implemented in Task 8.
|
||
|
||
**Step 6: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "parse_review_output or walkthrough or risk_verdict"
|
||
```
|
||
|
||
**Step 7: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py pilot/opencode_review.py .opencode/ tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(review): walkthrough/risk_verdict/test_coverage schema" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 8: Python `_synthesize_summary_fields()` for lens fan-out
|
||
|
||
**Files:**
|
||
- Modify: `pilot/opencode_review.py` (new helper + tests)
|
||
- Create: `tests/pilot/test_opencode_review.py` additions
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
from opencode_review import _synthesize_summary_fields
|
||
|
||
|
||
def test_synthesize_walkthrough_groups_findings_by_path():
|
||
findings = [
|
||
{"path": "a.py", "line": 1, "severity": "medium", "problem": "fix x"},
|
||
{"path": "b.py", "line": 2, "severity": "high", "problem": "fix y"},
|
||
]
|
||
w, _, _ = _synthesize_summary_fields(findings, "")
|
||
assert any("a.py" in line for line in w)
|
||
assert any("b.py" in line for line in w)
|
||
|
||
|
||
def test_synthesize_walkthrough_empty_when_no_findings_uses_changed_files():
|
||
w, _, _ = _synthesize_summary_fields([], "diff --git a/x.py b/x.py\n@@ -1 +1 @@\n-old\n+new\n")
|
||
# Diff has one file with a single-line change.
|
||
assert any("x.py" in line for line in w)
|
||
|
||
|
||
def test_synthesize_risk_verdict_critical():
|
||
findings = [{"severity": "critical"}]
|
||
_, rv, _ = _synthesize_summary_fields(findings, "")
|
||
assert "Critical risk" in rv
|
||
|
||
|
||
def test_synthesize_risk_verdict_clean():
|
||
_, rv, _ = _synthesize_summary_fields([], "")
|
||
assert "Low risk" in rv
|
||
|
||
|
||
def test_synthesize_test_coverage_with_test_path():
|
||
_, _, tc = _synthesize_summary_fields(
|
||
[], "+diff\n", changed_paths=["pilot/foo.py", "tests/test_foo.py"])
|
||
assert tc == "Tests changed"
|
||
|
||
|
||
def test_synthesize_test_coverage_missing_tests():
|
||
_, _, tc = _synthesize_summary_fields(
|
||
[], "+diff\n", changed_paths=["pilot/foo.py"])
|
||
assert "No tests for behavioral change" in tc
|
||
```
|
||
|
||
**Step 2: Implement**
|
||
|
||
Add `is_test_path` import or re-implement minimally (it already exists in
|
||
`ai_review.py` — import from there). Add to `pilot/opencode_review.py`:
|
||
|
||
```python
|
||
from ai_review import is_test_path
|
||
|
||
|
||
def _synthesize_summary_fields(
|
||
findings: list[dict], diff: str, changed_paths: list[str] | None = None,
|
||
) -> tuple[list[str], str, str]:
|
||
"""Python fallback when lens fan-out is engaged.
|
||
|
||
Returns (walkthrough, risk_verdict, test_coverage).
|
||
"""
|
||
# walkthrough
|
||
walkthrough: list[str] = []
|
||
if findings:
|
||
by_path: dict[str, list[dict]] = {}
|
||
for f in findings:
|
||
by_path.setdefault(f.get("path", "?"), []).append(f)
|
||
for path, group in sorted(by_path.items()):
|
||
peak = max(group, key=lambda x: SEVERITY_RANK.get(x.get("severity", "low"), 0))
|
||
problem = (peak.get("problem") or "").splitlines()[0][:80].strip()
|
||
emoji = _SEVERITY_EMOJI.get(peak.get("severity", "low"), "⚪")
|
||
walkthrough.append(f"`{path}` — {emoji} {problem}")
|
||
else:
|
||
files = changed_files(diff)
|
||
for p in files:
|
||
walkthrough.append(f"`{p}` — changed")
|
||
|
||
# risk_verdict
|
||
sev_counts = {"critical": 0, "high": 0, "medium": 0, "low": 0}
|
||
for f in findings:
|
||
s = f.get("severity", "low")
|
||
sev_counts[s] = sev_counts.get(s, 0) + 1
|
||
if sev_counts["critical"]:
|
||
rv = f"Critical risk: {sev_counts['critical']} critical finding(s)."
|
||
elif sev_counts["high"]:
|
||
rv = f"High risk: {sev_counts['high']} high finding(s)."
|
||
elif sev_counts["medium"]:
|
||
rv = f"Medium risk: {sev_counts['medium']} medium finding(s)."
|
||
else:
|
||
rv = "Low risk: clean or minor nits only."
|
||
|
||
# test_coverage
|
||
paths = changed_paths if changed_paths is not None else changed_files(diff)
|
||
test_changed = any(is_test_path(p) for p in paths)
|
||
non_test = [p for p in paths if not is_test_path(p)]
|
||
if test_changed and non_test:
|
||
tc = "Tests changed"
|
||
elif non_test:
|
||
tc = f"No tests for behavioral change in `{non_test[0]}`."
|
||
elif test_changed:
|
||
tc = "Tests changed"
|
||
else:
|
||
tc = ""
|
||
|
||
return walkthrough, rv, tc
|
||
|
||
|
||
_SEVERITY_EMOJI = {"critical": "🔴", "high": "🔴", "medium": "🟡",
|
||
"low": "🔵", "trivial": "⚪", "info": "⚪"}
|
||
```
|
||
|
||
**Step 3: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_opencode_review.py -v -k synthesize
|
||
```
|
||
|
||
**Step 4: Wire into `run_lenses_review`**
|
||
|
||
See Task 7, Step 5 — the function now calls `_synthesize_summary_fields`.
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/opencode_review.py tests/pilot/test_opencode_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(opencode_review): python fallback for summary fields" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 9: Render the three new summary sections in `format_review_body`
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:format_review_body`
|
||
- Modify: `tests/pilot/test_ai_review.py`
|
||
|
||
**Step 1: Add failing test**
|
||
|
||
```python
|
||
def test_format_review_body_renders_walkthrough():
|
||
body = format_review_body(
|
||
"", "glm-5.2:cloud", "abc1234",
|
||
summary_changes=["adds X"],
|
||
risks=[],
|
||
walkthrough=["a.py — adds X", "b.py — refactors Y"],
|
||
risk_verdict="Low risk: clean.",
|
||
test_coverage="Tests added.",
|
||
findings_for_table=[],
|
||
)
|
||
assert "### Walkthrough" in body
|
||
assert "`a.py` — adds X" in body
|
||
assert "### Risk Verdict" in body
|
||
assert "Low risk: clean." in body
|
||
assert "### Test Coverage" in body
|
||
assert "Tests added." in body
|
||
|
||
|
||
def test_format_review_body_omits_empty_sections():
|
||
body = format_review_body(
|
||
"", "glm-5.2:cloud", "abc1234",
|
||
summary_changes=["adds X"],
|
||
walkthrough=[], risk_verdict="", test_coverage="",
|
||
)
|
||
assert "### Walkthrough" not in body
|
||
assert "### Risk Verdict" not in body
|
||
assert "### Test Coverage" not in body
|
||
|
||
|
||
def test_format_review_body_placeholder_when_empty():
|
||
body = format_review_body(
|
||
"", "glm-5.2:cloud", "abc1234",
|
||
walkthrough=[], risk_verdict="", test_coverage="",
|
||
)
|
||
# even with empty summary_changes the function should not error
|
||
assert body # non-empty
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
**Step 3: Extend `format_review_body` signature**
|
||
|
||
Add three new kwargs with defaults; render three new sections between
|
||
Summary of Changes and Key Risks & Concerns:
|
||
|
||
```python
|
||
def format_review_body(
|
||
findings: str,
|
||
model: str,
|
||
sha: str,
|
||
summary: str = "",
|
||
usage_section: str = "",
|
||
*,
|
||
summary_changes: list[str] | None = None,
|
||
risks: list[str] | None = None,
|
||
findings_for_table: list[dict] | None = None,
|
||
inline_count: int = 0,
|
||
walkthrough: list[str] | None = None,
|
||
risk_verdict: str = "",
|
||
test_coverage: str = "",
|
||
confidence: int = 5,
|
||
) -> str:
|
||
```
|
||
|
||
In the body construction, after the `### Summary of Changes` block and before
|
||
`### Key Risks & Concerns`:
|
||
|
||
```python
|
||
# --- Risk Verdict ---
|
||
if risk_verdict:
|
||
parts.append(f"### Risk Verdict\n\n{risk_verdict}")
|
||
|
||
# --- Walkthrough ---
|
||
wt = list(walkthrough or [])
|
||
if wt:
|
||
wt = wt[:6]
|
||
items = "\n".join(f"- {item}" for item in wt)
|
||
parts.append(f"### Walkthrough\n\n{items}")
|
||
|
||
# --- Test Coverage ---
|
||
if test_coverage:
|
||
parts.append(f"### Test Coverage\n\n{test_coverage}")
|
||
```
|
||
|
||
Also thread `confidence` into the header as in Task 6, Step 5.
|
||
|
||
**Step 4: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "format_review_body"
|
||
```
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(review): render walkthrough + risk_verdict + test_coverage" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 10: Multi-provider cost render + drop the single Sonnet line
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:_render_collapsible_usage` (replace the single `Est. cost on {eq_label}` line with a table)
|
||
- Modify: `tests/pilot/test_ai_review.py`
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
def test_collapsible_usage_renders_multi_provider_table():
|
||
usage = {"input": 1_000_000, "output": 1000, "reasoning": 0,
|
||
"cache_read": 0, "cache_write": 0, "total": 1_001_000,
|
||
"cost": 0.0, "steps": 1, "duration_s": 10.0}
|
||
block = _render_collapsible_usage(usage, "glm-5.2:cloud", config={"compare_against": ["claude-sonnet-5", "gpt-5"]})
|
||
assert "Claude Sonnet 5" in block
|
||
assert "GPT-5" in block
|
||
assert "| Provider | Cost |" in block
|
||
|
||
|
||
def test_collapsible_usage_uses_default_compare_against_when_absent():
|
||
usage = {"input": 1_000_000, "output": 0, "reasoning": 0,
|
||
"cache_read": 0, "cache_write": 0, "total": 1_000_000,
|
||
"cost": 0.0, "steps": 1, "duration_s": 5.0}
|
||
block = _render_collapsible_usage(usage, "glm-5.2:cloud", config={})
|
||
assert "Claude Sonnet 5" in block
|
||
assert "GPT-5" in block
|
||
assert "Gemini 2.5 Pro" in block
|
||
assert "Grok 4.5" in block
|
||
|
||
|
||
def test_collapsible_usage_bolds_cost_target_row():
|
||
usage = {"input": 1_000_000, "output": 0, "reasoning": 0,
|
||
"cache_read": 0, "cache_write": 0, "total": 1_000_000,
|
||
"cost": 0.0, "steps": 1, "duration_s": 5.0}
|
||
block = _render_collapsible_usage(usage, "glm-5.2:cloud", config={"cost_target": "gpt-5"})
|
||
# The GPT-5 row should be bolded.
|
||
assert "**GPT-5**" in block
|
||
assert "Claude Sonnet 5" in block # still in default compare set
|
||
|
||
|
||
def test_collapsible_usage_skips_zero_cost_rows():
|
||
usage = {"input": 0, "output": 0, "reasoning": 0,
|
||
"cache_read": 0, "cache_write": 0, "total": 0,
|
||
"cost": 0.0, "steps": 1, "duration_s": 1.0}
|
||
block = _render_collapsible_usage(usage, "glm-5.2:cloud", config={})
|
||
# With zero tokens, all costs are $0 — skip the entire table.
|
||
assert "| Provider | Cost |" not in block
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
**Step 3: Replace the single `Est. cost on ...` line with a table builder**
|
||
|
||
Add `DEFAULT_COMPARE_AGAINST = ("claude-sonnet-5", "gpt-5", "gemini-2.5-pro", "grok-4.5")`
|
||
near the top of `pilot/ai_review.py`.
|
||
|
||
Replace the line:
|
||
|
||
```python
|
||
f"- **Est. cost on {eq_label}**: {eq_s}{eq_note}",
|
||
```
|
||
|
||
with a multi-line table (built before the `lines` list, then `lines.append`-ed):
|
||
|
||
```python
|
||
from cost_model import PRICES as _PRICES
|
||
compare = (config or {}).get("compare_against") or list(DEFAULT_COMPARE_AGAINST)
|
||
rows = []
|
||
cost_target = (config or {}).get("cost_target", DEFAULT_PRICE_TARGET)
|
||
for key in compare:
|
||
if key not in _PRICES:
|
||
continue
|
||
c = equivalent_cost(usage, key)
|
||
if c <= 0:
|
||
continue
|
||
label = _PRICES[key].name
|
||
cost_str = f"${c:.4f}" if c < 0.01 else f"${c:.2f}"
|
||
bold = "**" if key == cost_target else ""
|
||
rows.append(f"| {bold}{label}{bold} | {cost_str} |")
|
||
if rows:
|
||
lines.append("- **Equivalent cost on paid providers** (this run's tokens):")
|
||
lines.append("")
|
||
lines.append("| Provider | Cost |")
|
||
lines.append("|---|---:|")
|
||
lines.extend(rows)
|
||
```
|
||
|
||
Keep the `**Actual**` line as-is. Remove the now-unused `eq_label` / `eq_s`
|
||
local variables (or keep them — see Step 4).
|
||
|
||
**Step 4: Clean up locals**
|
||
|
||
Delete the variables that were only used by the old line:
|
||
|
||
```python
|
||
eq = equivalent_cost(usage, price_key)
|
||
eq_s = f"${eq:.4f}" if eq else "$0.00"
|
||
eq_label = PRICES[price_key].name
|
||
eq_note = (
|
||
f" _(price target: `{price_key}`; {price_err})_"
|
||
if price_err else ""
|
||
)
|
||
```
|
||
|
||
Keep `_resolve_price_target` for the cost_target lookup (still used to bold
|
||
the target row). The `eq_note` no longer renders — price errors should go to
|
||
stderr instead:
|
||
|
||
```python
|
||
if price_err:
|
||
print(f"pragent: {price_err}", file=sys.stderr, flush=True)
|
||
```
|
||
|
||
**Step 5: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_ai_review.py -v -k "collapsible_usage"
|
||
```
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(usage): multi-provider equivalent cost table" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 11: Drop `AI_REVIEW_LABEL` / `AI_USAGE_LABEL` constants + label helpers in `ai_review.py`
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py` (delete constants + `pr_has_label`)
|
||
- Modify: `tests/pilot/test_ai_review.py` (remove or update affected tests)
|
||
|
||
**Step 1: Find affected tests**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && grep -nE "AI_REVIEW_LABEL|AI_USAGE_LABEL|pr_has_label|report_usage|PRAGENT_USAGE_ALWAYS" tests/pilot/*.py pilot/*.py
|
||
```
|
||
|
||
For each hit, decide: delete the test (if the feature is gone) or rewrite
|
||
(if the helper still exists elsewhere). The webhook rewrite (Task 13) drops
|
||
the webhook-side label helpers.
|
||
|
||
**Step 2: Delete the constants + helper from `ai_review.py`**
|
||
|
||
Remove these lines from `pilot/ai_review.py`:
|
||
|
||
```python
|
||
AI_REVIEW_LABEL = "AI-REVIEW"
|
||
AI_USAGE_LABEL = "AI-USAGE"
|
||
```
|
||
|
||
Delete the entire `pr_has_label(...)` function.
|
||
|
||
Delete any remaining references to those names in this file (the
|
||
`pr_has_label` re-reads at render time — both call sites).
|
||
|
||
**Step 3: Run tests, watch for failures**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && python3 -m pytest tests/pilot/test_ai_review.py -q
|
||
```
|
||
|
||
Expected: some failures (existing tests that referenced labels). Fix or
|
||
delete them.
|
||
|
||
**Step 4: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && git add pilot/ai_review.py tests/pilot/test_ai_review.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "refactor(ai_review): drop AI_REVIEW/AI_USAGE label plumbing" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 12: Drop `report_usage` parameter + `PRAGENT_USAGE_ALWAYS` env
|
||
|
||
**Files:**
|
||
- Modify: `pilot/ai_review.py:review_pr` signature (remove `report_usage: bool = False`)
|
||
- Modify: `pilot/ai_review.py` (remove the two env reads + the `pr_has_label` re-reads + the `if report_usage` gates)
|
||
- Modify: `pilot/webhook_server.py:_run_review` (remove `report_usage` arg + log line)
|
||
- Modify: `tests/pilot/test_ai_review.py` (update callers)
|
||
|
||
**Step 1: Update `review_pr` signature**
|
||
|
||
```python
|
||
def review_pr(
|
||
api: str,
|
||
repo: str,
|
||
index: str,
|
||
title: str,
|
||
body: str,
|
||
sha: str,
|
||
token: str,
|
||
ollama_url: str,
|
||
model: str,
|
||
max_tokens: int = 8000,
|
||
max_chars: int = 150000,
|
||
base_ref: str = "",
|
||
) -> bool:
|
||
```
|
||
|
||
Remove `report_usage` from the signature. Inside the function, replace every
|
||
`if report_usage and ...` with `if usage and usage.get("output"):` (the
|
||
condition was equivalent).
|
||
|
||
Remove the two `pr_has_label(api, repo, index, token, AI_USAGE_LABEL)`
|
||
re-reads (one in the salvage branch, one at the end). The label they're
|
||
checking no longer exists.
|
||
|
||
**Step 2: Update the usage-section render**
|
||
|
||
```python
|
||
usage_section = _render_collapsible_usage(usage, model, config=config)
|
||
```
|
||
|
||
Always rendered when usage is non-None. Remove the conditional.
|
||
|
||
**Step 3: Remove env reads**
|
||
|
||
Delete `os.environ.get("PRAGENT_USAGE_ALWAYS")` and the two PRAGENT_USAGE_ALWAYS
|
||
references.
|
||
|
||
**Step 4: Update `_run_review` in webhook_server.py**
|
||
|
||
```python
|
||
try:
|
||
with _review_slots:
|
||
ok = review_pr(
|
||
api=GITEA_API,
|
||
repo=repo,
|
||
index=index,
|
||
title=title,
|
||
body=body,
|
||
sha=sha,
|
||
token=BOT_TOKEN,
|
||
ollama_url=OLLAMA_URL,
|
||
model=OLLAMA_MODEL,
|
||
max_tokens=OLLAMA_MAX_TOKENS,
|
||
max_chars=DIFF_MAX_CHARS,
|
||
base_ref=base_ref,
|
||
)
|
||
print(f"pragent-webhook: reviewed {repo}#{index} sha={sha[:8]} ok={ok}", flush=True)
|
||
```
|
||
|
||
Drop the `report_usage` arg and the trailing `usage={...}` in the log line.
|
||
|
||
**Step 5: Update callers in tests**
|
||
|
||
Find every `review_pr(...)` call in `tests/pilot/test_ai_review.py` and
|
||
remove the `report_usage=` keyword.
|
||
|
||
**Step 6: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests -q
|
||
```
|
||
|
||
**Step 7: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/ai_review.py pilot/webhook_server.py tests/pilot/ && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "refactor(review): always render usage; drop report_usage flag" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 13: Webhook `is_repo_enabled()` helper
|
||
|
||
**Files:**
|
||
- Modify: `pilot/webhook_server.py` (add helper)
|
||
- Modify: `tests/pilot/test_webhook_server.py` (add tests)
|
||
|
||
**Step 1: Add failing tests**
|
||
|
||
```python
|
||
from webhook_server import is_repo_enabled
|
||
|
||
|
||
def test_is_repo_enabled_returns_false_when_404(monkeypatch):
|
||
monkeypatch.setattr("webhook_server.gitea_get",
|
||
lambda *a, **kw: (404, b'{"message":"not found"}'))
|
||
assert is_repo_enabled("api", "owner/repo", "main", "tok") is False
|
||
|
||
|
||
def test_is_repo_enabled_returns_true_when_enabled(monkeypatch):
|
||
body = b'{"content":"' + base64.b64encode(b'{"enabled": true}').decode().encode() + b'"}'
|
||
monkeypatch.setattr("webhook_server.gitea_get",
|
||
lambda *a, **kw: (200, body))
|
||
assert is_repo_enabled("api", "owner/repo", "main", "tok") is True
|
||
|
||
|
||
def test_is_repo_enabled_returns_false_when_disabled(monkeypatch):
|
||
body = b'{"content":"' + base64.b64encode(b'{"enabled": false}').decode().encode() + b'"}'
|
||
monkeypatch.setattr("webhook_server.gitea_get",
|
||
lambda *a, **kw: (200, body))
|
||
assert is_repo_enabled("api", "owner/repo", "main", "tok") is False
|
||
|
||
|
||
def test_is_repo_enabled_returns_false_when_field_missing(monkeypatch):
|
||
body = b'{"content":"' + base64.b64encode(b'{}').decode().encode() + b'"}'
|
||
monkeypatch.setattr("webhook_server.gitea_get",
|
||
lambda *a, **kw: (200, body))
|
||
assert is_repo_enabled("api", "owner/repo", "main", "tok") is False
|
||
```
|
||
|
||
**Step 2: Run, watch fail**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_webhook_server.py -v -k "is_repo_enabled"
|
||
```
|
||
|
||
**Step 3: Implement `is_repo_enabled`**
|
||
|
||
Add to `pilot/webhook_server.py`:
|
||
|
||
```python
|
||
def is_repo_enabled(api: str, repo: str, ref: str, token: str) -> bool:
|
||
"""True iff `.pr-review.json` on `ref` has `"enabled": true`.
|
||
|
||
Reads from the given ref (defaults to the PR's base ref). False on any
|
||
failure: 404, parse error, missing file, missing `enabled`, wrong type.
|
||
Logs the reason to stderr so an operator can debug.
|
||
"""
|
||
code, raw = gitea_get(
|
||
api, repo, "contents/.pr-review.json?ref=" + urllib.parse.quote(ref, safe=""),
|
||
token,
|
||
)
|
||
if code != 200:
|
||
return False
|
||
try:
|
||
data = json.loads(raw)
|
||
content_b64 = data.get("content", "").replace("\n", "")
|
||
decoded = base64.b64decode(content_b64).decode("utf-8", errors="replace")
|
||
cfg = json.loads(decoded)
|
||
except (json.JSONDecodeError, ValueError):
|
||
return False
|
||
return isinstance(cfg, dict) and cfg.get("enabled") is True
|
||
```
|
||
|
||
Imports to add: `base64`, `urllib.parse`.
|
||
|
||
**Step 4: Run, watch pass**
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/webhook_server.py tests/pilot/test_webhook_server.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(webhook): is_repo_enabled reads .pr-review.json:enabled" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 14: Webhook gate `enabled` in `_handle_pull_request`
|
||
|
||
**Files:**
|
||
- Modify: `pilot/webhook_server.py:_handle_pull_request`
|
||
- Modify: `tests/pilot/test_webhook_server.py` (existing handler tests may need updates)
|
||
|
||
**Step 1: Find existing handler tests**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && grep -n "_handle_pull_request\|_labels_have_ai_review" tests/pilot/test_webhook_server.py
|
||
```
|
||
|
||
Update each test to either:
|
||
- pass through the new `is_repo_enabled` monkeypatch (return True), OR
|
||
- assert the new "skip (repo not opted in)" branch.
|
||
|
||
**Step 2: Update `_handle_pull_request`**
|
||
|
||
Replace the existing flow:
|
||
|
||
```python
|
||
def _handle_pull_request(payload: dict) -> tuple[int, str]:
|
||
action = payload.get("action", "")
|
||
pr = payload.get("pull_request") or {}
|
||
repo_obj = payload.get("repository") or {}
|
||
repo = repo_obj.get("full_name") or ""
|
||
|
||
if action in SKIP_ACTIONS:
|
||
return 200, f"ignore action={action}"
|
||
if not repo:
|
||
return 400, "no repository.full_name"
|
||
|
||
base_ref = (pr.get("base") or {}).get("ref", "") or ""
|
||
if not is_repo_enabled(GITEA_API, repo, base_ref or "", BOT_TOKEN):
|
||
return 200, f"skip (repo not opted in) action={action}"
|
||
|
||
if not BOT_TOKEN:
|
||
return 500, "PRAGENT_BOT_TOKEN not set"
|
||
|
||
index = pr.get("number")
|
||
...
|
||
```
|
||
|
||
The label-check lines (`_labels_have_ai_review(labels)`) are deleted
|
||
entirely. The `pr.get("labels")` lookup is no longer needed; remove it.
|
||
|
||
**Step 3: Update `do_POST` log line**
|
||
|
||
```python
|
||
pr0 = payload.get("pull_request") or {}
|
||
repo_full = (payload.get("repository") or {}).get("full_name")
|
||
print(
|
||
f"pragent-webhook: pull_request action={payload.get('action')} repo={repo_full}",
|
||
flush=True,
|
||
)
|
||
```
|
||
|
||
The `ai_review={...}` field is gone.
|
||
|
||
**Step 4: Drop unused imports + constants**
|
||
|
||
Delete from `pilot/webhook_server.py`:
|
||
|
||
```python
|
||
AI_REVIEW_LABEL = "AI-REVIEW"
|
||
AI_USAGE_LABEL = "AI-USAGE"
|
||
```
|
||
|
||
```python
|
||
def _labels_have_ai_review(labels) -> bool:
|
||
return _labels_have(labels, AI_REVIEW_LABEL)
|
||
```
|
||
|
||
(If `_labels_have` is unused after this, delete it too.)
|
||
|
||
**Step 5: Run, watch pass**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && python3 -m pytest tests/pilot/test_webhook_server.py -v
|
||
```
|
||
|
||
**Step 6: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragent && git add pilot/webhook_server.py tests/pilot/test_webhook_server.py && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "feat(webhook): gate on .pr-review.json:enabled, drop labels" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 15: README + `pilot/README-webhook.md` onboarding rewrite
|
||
|
||
**Files:**
|
||
- Modify: `README.md`
|
||
- Modify: `pilot/README-webhook.md`
|
||
|
||
**Step 1: Rewrite the README's "Setup" section**
|
||
|
||
Find:
|
||
|
||
```markdown
|
||
## Setup
|
||
|
||
Onboarding a repo, once the service is running for that owner:
|
||
|
||
1. add `pragent-bot` as a **Write** collaborator
|
||
2. create the `AI-REVIEW` label
|
||
3. label a PR
|
||
```
|
||
|
||
Replace with:
|
||
|
||
```markdown
|
||
## Setup
|
||
|
||
Onboarding a repo, once the service is running for that owner:
|
||
|
||
1. add `pragent-bot` as a **Write** collaborator
|
||
2. commit `.pr-review.json: {"enabled": true}` to the repo's default branch
|
||
3. open a PR
|
||
```
|
||
|
||
Remove the "optional token/cost reporting via an `AI-USAGE` label" bullet
|
||
under "What works today."
|
||
|
||
Find the flow diagram:
|
||
|
||
```text
|
||
PR labelled AI-REVIEW
|
||
│ Gitea webhook (HMAC-verified, body-capped, concurrency-bounded)
|
||
▼
|
||
review_pr()
|
||
1. dedupe ...
|
||
2. fetch diff + .pr-review.json from the BASE branch
|
||
...
|
||
```
|
||
|
||
Update the first line to:
|
||
|
||
```text
|
||
PR opened on repo with `.pr-review.json:enabled = true`
|
||
```
|
||
|
||
Add an early step to the numbered list:
|
||
|
||
```
|
||
1. opt-in .pr-review.json:enabled=true on base? if not, skip.
|
||
2. dedupe already reviewed this exact sha? stop.
|
||
3. fetch diff + .pr-review.json from the BASE branch
|
||
...
|
||
```
|
||
|
||
Re-number the remaining steps.
|
||
|
||
**Step 2: Rewrite `pilot/README-webhook.md` onboarding**
|
||
|
||
Find any reference to "create the AI-REVIEW label" or "label a PR" and replace
|
||
with "commit `.pr-review.json: {"enabled": true}` to the default branch".
|
||
|
||
**Step 3: Update the "What it costs" section**
|
||
|
||
Replace "calibrated against runs measured through the `AI-USAGE` label" with
|
||
"calibrated against runs measured through the usage telemetry".
|
||
|
||
**Step 4: Run a final grep to catch stragglers**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && grep -nE "AI-REVIEW|AI_USAGE_LABEL|AI-USAGE" README.md pilot/README-webhook.md
|
||
```
|
||
|
||
Expected: no hits. If any remain, fix them.
|
||
|
||
**Step 5: Commit**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && git add README.md pilot/README-webhook.md && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "docs: onboarding uses .pr-review.json:enabled, not labels" --no-verify
|
||
```
|
||
|
||
---
|
||
|
||
## Task 16: Final verification — full test suite + manual smoke
|
||
|
||
**Step 1: Run the full test suite**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && python3 -m pytest tests -v
|
||
```
|
||
|
||
Expected: all green. Capture the pass count for the commit message.
|
||
|
||
**Step 2: Smoke-test the cost-model CLI**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && python3 pilot/cost_model.py \
|
||
--prs-per-month 350 --models claude-sonnet-5,gpt-5,gemini-2.5-pro,grok-4.5
|
||
```
|
||
|
||
Expected: a report that lists each model row + an `Observed runs` section
|
||
that mentions the existing measurement.
|
||
|
||
**Step 3: Lint the codebase**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && python3 -m pyflakes pilot/ tests/pilot/
|
||
```
|
||
|
||
Expected: no errors (warnings are OK if they're for known test imports).
|
||
|
||
**Step 4: Final commit (if any fixups needed)**
|
||
|
||
If anything came up in Steps 1–3, fix it and:
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && git add -A && \
|
||
git -c user.name=claude -c user.email=noreply@anthropic.com commit -m "chore: post-update polish" --no-verify
|
||
```
|
||
|
||
Otherwise, no commit.
|
||
|
||
**Step 5: Push (manual)**
|
||
|
||
```bash
|
||
cd ~/Projects/pragment && git push origin main
|
||
```
|
||
|
||
(Requires the user's remote + credentials. Push only when explicitly asked.)
|
||
|
||
---
|
||
|
||
## Summary
|
||
|
||
- 16 tasks, each independently commitable.
|
||
- Order is dependency-driven:
|
||
P1 items (cost model, fmt_tokens, severity levels) ship first (Tasks 1–4).
|
||
P2 items (summary fields, merge confidence, multi-provider render) follow
|
||
(Tasks 5–10).
|
||
P0 (label removal + repo opt-in) ships last so the gate change ships with
|
||
the docs (Tasks 11–15).
|
||
- Test-first throughout. Each task writes its failing test, runs it, then
|
||
implements.
|
||
- Final task is the only one that does cross-cutting verification. |