gitea_admin/pragent
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Merge feature/pragent-update: 16-task pragent update

Browse Source 
Removes AI-REVIEW/AI-USAGE label gates (replaced with .pr-review.json:enabled=true on base ref).
Adds multi-provider cost table, fmt_tokens, trivial/info severities, walkthrough/risk_verdict/test_coverage
sections, per-PR merge confidence 1-5, always-on usage rendering, README rewrite.

362 tests pass. Caveat: pilot/feedback*.py modules referenced by webhook_server.py remain untracked.
This commit is contained in:
Marcos
2026-08-22 01:38:49 +00:00
parent 99014a4cd3 661199dad2
commit 8e8ae54669
16 changed files with 1215 additions and 284 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.opencode/agents/code-quality.md
+6
View File
@@ -68,6 +68,12 @@ Return STRICT JSON only — same shape as the pragent primary's findings:
} }
``` ```
The full review-level JSON shape (used by the pragent primary) also
includes three optional top-level fields — `walkthrough` (list[str]),
`risk_verdict` (str), and `test_coverage` (str) — that the synthesizer
fills in across all lenses. Lens output is free to omit them; the parser
defaults to `[]` / `""` when absent (backward compatible).
`ruleId` examples: `QUALITY_DEAD_CODE`, `QUALITY_HIDDEN_COMPLEXITY`, `ruleId` examples: `QUALITY_DEAD_CODE`, `QUALITY_HIDDEN_COMPLEXITY`,
`QUALITY_INVARIANT_DROP`, `QUALITY_NAMING_CONTRADICTS`, `QUALITY_INVARIANT_DROP`, `QUALITY_NAMING_CONTRADICTS`,
`QUALITY_SUPPRESSED_ERROR`, `QUALITY_DUPLICATED_LOGIC`. One stable `QUALITY_SUPPRESSED_ERROR`, `QUALITY_DUPLICATED_LOGIC`. One stable
.opencode/agents/docs.md
+6
View File
@@ -67,6 +67,12 @@ Return STRICT JSON only — same shape as the pragent primary's findings:
} }
``` ```
The full review-level JSON shape (used by the pragent primary) also
includes three optional top-level fields — `walkthrough` (list[str]),
`risk_verdict` (str), and `test_coverage` (str) — that the synthesizer
fills in across all lenses. Lens output is free to omit them; the parser
defaults to `[]` / `""` when absent (backward compatible).
`ruleId` examples: `DOCS_README_DRIFT`, `DOCS_FENCE_BROKEN`, `ruleId` examples: `DOCS_README_DRIFT`, `DOCS_FENCE_BROKEN`,
`DOCS_ENV_UNDOCUMENTED`, `DOCS_LINK_ROT`, `DOCS_NO_CHANGELOG`. Use one `DOCS_ENV_UNDOCUMENTED`, `DOCS_LINK_ROT`, `DOCS_NO_CHANGELOG`. Use one
stable ruleId per recurring pattern — it's how the synthesizer dedups stable ruleId per recurring pattern — it's how the synthesizer dedups
.opencode/agents/perf.md
+6
View File
@@ -48,3 +48,9 @@ O(n²) over bounded small n, `low` for redundant-but-rare work.
``` ```
`line` must be a post-change line. No prose outside JSON. `line` must be a post-change line. No prose outside JSON.
The full review-level JSON shape (used by the pragent primary) also
includes three optional top-level fields — `walkthrough` (list[str]),
`risk_verdict` (str), and `test_coverage` (str) — that the synthesizer
fills in across all lenses. Lens output is free to omit them; the parser
defaults to `[]` / `""` when absent (backward compatible).
.opencode/agents/pragent.md
+16
View File
@@ -160,6 +160,12 @@ containing STRICT JSON, nothing else after it:
"risks": [ "risks": [
"Bullets detailing potential bugs, edge cases, lifecycle issues, or performance risks found across the diff" "Bullets detailing potential bugs, edge cases, lifecycle issues, or performance risks found across the diff"
], ],
"walkthrough": [
"a.py: adds X — short plain-prose bullet, file- or change-grouped",
"b.py: refactors Y"
],
"risk_verdict": "Low|Medium|High|Critical risk: <one-line concrete reason>",
"test_coverage": "Tests added" | "Tests changed" | "No tests for behavioral change" | "No test files in repo",
"findings": [ "findings": [
{ {
"severity": "critical|high|medium|low|info|nit", "severity": "critical|high|medium|low|info|nit",
@@ -178,6 +184,16 @@ Rules:
- `summary_changes` (2–4 bullets) goes into the **Summary of Changes** section. - `summary_changes` (2–4 bullets) goes into the **Summary of Changes** section.
`risks` (bullets) goes into **Key Risks & Concerns**. Both are required; `risks` (bullets) goes into **Key Risks & Concerns**. Both are required;
empty arrays are fine when nothing applies. empty arrays are fine when nothing applies.
- `walkthrough` (2–6 bullets, file- or change-grouped) is the **Walkthrough**
section: what the PR does, where, in plain prose. Default to `[]` for a
trivial diff. Backward compatible — parsers default to `[]` if absent.
- `risk_verdict` (exactly one line) goes into the **Risk Verdict** section.
Lead with `Low|Medium|High|Critical risk:` followed by a concrete reason.
Default to `""` when not applicable. Backward compatible.
- `test_coverage` (short string) goes into the **Test Coverage** section.
Use exactly one of `"Tests added"`, `"Tests changed"`,
`"No tests for behavioral change"`, `"No test files in repo"`. Default to `""`.
Backward compatible.
- `suggestion` is the literal new code that replaces the flagged line(s). Minimal — - `suggestion` is the literal new code that replaces the flagged line(s). Minimal —
just the changed lines, indented as they'd appear in the file. Empty string `""` just the changed lines, indented as they'd appear in the file. Empty string `""`
when no safe textual replacement exists (e.g. missing test, architectural note). when no safe textual replacement exists (e.g. missing test, architectural note).
.opencode/agents/security.md
+6
View File
@@ -51,3 +51,9 @@ security findings only:
`line` must be a post-change (context or `+`) line. Empty `suggestion` when no `line` must be a post-change (context or `+`) line. Empty `suggestion` when no
safe replacement. No prose outside the JSON block. safe replacement. No prose outside the JSON block.
The full review-level JSON shape (used by the pragent primary) also
includes three optional top-level fields — `walkthrough` (list[str]),
`risk_verdict` (str), and `test_coverage` (str) — that the synthesizer
fills in across all lenses. Lens output is free to omit them; the parser
defaults to `[]` / `""` when absent (backward compatible).
.opencode/agents/tests.md
+6
View File
@@ -48,3 +48,9 @@ replacement); include a sketch only if a one-line test is obvious.
``` ```
`line` must be a post-change line in a source or test file. No prose outside JSON. `line` must be a post-change line in a source or test file. No prose outside JSON.
The full review-level JSON shape (used by the pragent primary) also
includes three optional top-level fields — `walkthrough` (list[str]),
`risk_verdict` (str), and `test_coverage` (str) — that the synthesizer
fills in across all lenses. Lens output is free to omit them; the parser
defaults to `[]` / `""` when absent (backward compatible).
README.md
+23 -18
View File
@@ -3,7 +3,8 @@
An AI pull-request reviewer for Gitea that posts **inline comments with suggested An AI pull-request reviewer for Gitea that posts **inline comments with suggested
fixes**, not a wall of prose — and reports what each review cost. fixes**, not a wall of prose — and reports what each review cost.
Label a PR `AI-REVIEW`. A webhook wakes a service that checks the repo out at the A webhook wakes for any PR on a repo whose default branch carries a
`.pr-review.json` with `"enabled": true`. The service checks the repo out at the
PR's head commit, reads the changed files *and the code around them*, runs the PR's head commit, reads the changed files *and the code around them*, runs the
repo's own linters, and posts a review anchored to real lines. repo's own linters, and posts a review anchored to real lines.
@@ -40,14 +41,16 @@ built — see [`docs/plans/`](docs/plans/).
What works today: What works today:
- a central webhook service, so onboarding a repo is *add the bot + add the label* - a central webhook service, so onboarding a repo is *add the bot + commit
`.pr-review.json:enabled = true`*
- whole-repo context: the reviewer reads callers and types, not just the hunk - whole-repo context: the reviewer reads callers and types, not just the hunk
- inline comments with language-highlighted suggested fixes, anchored to - inline comments with language-highlighted suggested fixes, anchored to
post-change lines and validated in Python before posting post-change lines and validated in Python before posting
- per-commit dedupe, and prior reviews fed back so a re-push synthesises rather - per-commit dedupe, and prior reviews fed back so a re-push synthesises rather
than repeats than repeats
- `.pr-review.json` for per-repo focus and house rules - `.pr-review.json` for per-repo focus and house rules (also the opt-in flag)
- optional token/cost reporting via an `AI-USAGE` label - token-usage reporting on every review, measured from opencode `step_finish`
events
- containment against hostile PR content (see [Security](#security)) - containment against hostile PR content (see [Security](#security))
Not yet: status checks, fail-close, attention tiering enforced in code (it is Not yet: status checks, fail-close, attention tiering enforced in code (it is
@@ -56,21 +59,22 @@ currently a skill the agent follows), multi-model routing.
## How a review runs ## How a review runs
``` ```
PR labelled AI-REVIEW PR opened on repo with `.pr-review.json:enabled = true`
│ Gitea webhook (HMAC-verified, body-capped, concurrency-bounded) │ Gitea webhook (HMAC-verified, body-capped, concurrency-bounded)
▼ ▼
review_pr() review_pr()
1. dedupe already reviewed this exact sha? stop. 1. opt-in .pr-review.json:enabled=true on base? if not, skip.
2. fetch diff + .pr-review.json from the BASE branch 2. dedupe already reviewed this exact sha? stop.
3. checkout repo archive at head sha → temp workdir 3. fetch diff + .pr-review.json from the BASE branch
4. sanitize delete author-controlled agent-instruction files 4. checkout repo archive at head sha → temp workdir
5. brief .pragent/brief.md, untrusted parts explicitly fenced 5. sanitize delete author-controlled agent-instruction files
6. review opencode agent: read code, run linters, emit findings JSON 6. brief .pragent/brief.md, untrusted parts explicitly fenced
7. anchor validate every line against the diff's post-change lines 7. review opencode agent: read code, run linters, emit findings JSON
8. post inline comments + summary, as pragent-bot 8. anchor validate every line against the diff's post-change lines
9. post inline comments + summary, as pragent-bot
``` ```
Steps 1, 2, 7 and 8 are deterministic Python. The model's only job is step 6 — Steps 1, 3, 8 and 9 are deterministic Python. The model's only job is step 7 —
producing correct findings. It never talks to Gitea, and a finding whose line producing correct findings. It never talks to Gitea, and a finding whose line
does not validate becomes a summary bullet rather than a misplaced comment. does not validate becomes a summary bullet rather than a misplaced comment.
@@ -79,8 +83,8 @@ does not validate becomes a summary bullet rather than a misplaced comment.
Onboarding a repo, once the service is running for that owner: Onboarding a repo, once the service is running for that owner:
1. add `pragent-bot` as a **Write** collaborator 1. add `pragent-bot` as a **Write** collaborator
2. create the `AI-REVIEW` label 2. commit `.pr-review.json: {"enabled": true}` to the repo's default branch
3. label a PR 3. open a PR
Standing up the service itself — the webhook, the image, the Gitea SSRF Standing up the service itself — the webhook, the image, the Gitea SSRF
allow-list, the per-owner webhook registration — is in allow-list, the per-owner webhook registration — is in
@@ -136,8 +140,9 @@ concurrency. Full threat model and residual risks: `pilot/README-webhook.md`.
The pilot runs against a self-hosted model and bills nothing per token, but the The pilot runs against a self-hosted model and bills nothing per token, but the
token *work* is real. `pilot/cost_model.py` prices it against published API token *work* is real. `pilot/cost_model.py` prices it against published API
rates, calibrated against runs measured through the `AI-USAGE` label rates, calibrated against runs measured through the usage telemetry
(`OBSERVED_RUNS` in that file — append to it, don't guess). (`OBSERVED_RUNS` in that file — append to it, don't guess). Tokens are summed
from opencode `step_finish` events per review.
Two measured reviews of a ~1100-line PR in this repo: 28 and 31 agent steps, Two measured reviews of a ~1100-line PR in this repo: 28 and 31 agent steps,
~2.1M input tokens each, **zero cache reads or writes**. The demo repo's PR, same ~2.1M input tokens each, **zero cache reads or writes**. The demo repo's PR, same
pilot/README-webhook.md
+41 -44
View File
@@ -1,30 +1,31 @@
# pragent pilot — central webhook service # pragent pilot — central webhook service
The CI-step pilot (`pilot/README.md`) needs a workflow file + secret + label per The CI-step pilot (`pilot/README.md`) needs a workflow file + secret per repo.
repo. The **central webhook service** removes the workflow file, the secret, and The **central webhook service** removes the workflow file, the secret, and the
the runner dependency: a Gitea webhook posts PR events to an always-on in-cluster runner dependency: a Gitea webhook posts PR events to an always-on in-cluster
service, which gates on the `AI-REVIEW` label and runs the same review core. service, which gates on `.pr-review.json:enabled = true` and runs the same review
core.
## Architecture ## Architecture
``` ```
PR opened/pushed/labeled/edited/… (any repo under a covered owner) PR opened/pushed/edited/… (any repo under a covered owner)
│ Gitea user-level webhook (events: pull_request) │ Gitea user-level webhook (events: pull_request)
▼ ▼
Service pragent-webhook.pragent.svc.cluster.local (ClusterIP, ns pragent) Service pragent-webhook.pragent.svc.cluster.local (ClusterIP, ns pragent)
│ body-size cap → HMAC-verify (X-Gitea-Signature) │ body-size cap → HMAC-verify (X-Gitea-Signature)
│ → gate: action ≠ closed AND pull_request.labels ∋ AI-REVIEW │ → gate: action ≠ closed AND .pr-review.json:enabled = true on base
│ → claim (repo, index, sha) in-flight (closes the dedupe race) │ → claim (repo, index, sha) in-flight (closes the dedupe race)
│ → bounded worker (PRAGENT_MAX_CONCURRENT_REVIEWS, default 2) │ → bounded worker (PRAGENT_MAX_CONCURRENT_REVIEWS, default 2)
│ (report_usage ← pull_request.labels ∋ AI-USAGE, optional)
▼ ▼
ai_review.review_pr() (same core the CI-step uses) ai_review.review_pr() (same core the CI-step uses)
1. fetch existing reviews → dedupe: skip if a review already carries 1. opt-in .pr-review.json:enabled = true on base? if not, skip.
<!-- pragent:sha=<this sha> --> (no duplicate on label-toggle / re-fire) 2. fetch existing reviews → dedupe: skip if a review already carries
2. fetch PR diff → GET .../pulls/{i}.diff <!-- pragent:sha=<this sha> --> (no duplicate on title/body-edit re-fire)
3. fetch .pr-review.json @ head ref (optional repo-local focus/config) 3. fetch PR diff → GET .../pulls/{i}.diff
4. prior review bodies → fed as "already said" context (light §6.1) 4. fetch .pr-review.json @ base ref (the opt-in flag + repo-local focus/config)
5. PRAGENT_ENGINE=opencode (default): 5. prior review bodies → fed as "already said" context (light §6.1)
6. PRAGENT_ENGINE=opencode (default):
a. fetch repo archive @ head sha → /tmp/pragent-work/<repo>-<sha> a. fetch repo archive @ head sha → /tmp/pragent-work/<repo>-<sha>
(symlink-escape + traversal rejected on untar) (symlink-escape + traversal rejected on untar)
a2. sanitize the workdir: delete author-controlled agent-instruction a2. sanitize the workdir: delete author-controlled agent-instruction
@@ -40,8 +41,8 @@ ai_review.review_pr() (same core the CI-step uses)
diffs, and emits: {"summary":..., "findings":[{severity,path,line, diffs, and emits: {"summary":..., "findings":[{severity,path,line,
problem,fix,suggestion,reference}]} problem,fix,suggestion,reference}]}
(=ollama: legacy single POST to http://<model-proxy-host>:8789/v1/messages) (=ollama: legacy single POST to http://<model-proxy-host>:8789/v1/messages)
6. parse diff hunks → valid (path, new_line) anchors (RIGHT side) 7. parse diff hunks → valid (path, new_line) anchors (RIGHT side)
7. post review → POST .../pulls/{i}/reviews (event: COMMENT) as pragent-bot 8. post review → POST .../pulls/{i}/reviews (event: COMMENT) as pragent-bot
- prose summary → review body intro - prose summary → review body intro
- anchored findings → inline line comments, body wraps `suggestion` in a - anchored findings → inline line comments, body wraps `suggestion` in a
language-tagged fenced code block (Gitea syntax-highlights it; Gitea language-tagged fenced code block (Gitea syntax-highlights it; Gitea
@@ -59,18 +60,18 @@ of repeating (light version of framework §6.1).
1. Add `pragent-bot` as collaborator with **Write** (so it can read the diff and 1. Add `pragent-bot` as collaborator with **Write** (so it can read the diff and
post the review). The bot stays a normal user — it is **not** a site admin. post the review). The bot stays a normal user — it is **not** a site admin.
2. Create the `AI-REVIEW` label on the repo (one-time; `pragent-bot`'s 2. Commit `.pr-review.json: {"enabled": true}` to the repo's default branch
`write:issue` scope can do it once it's a collaborator). (so every PR on the repo is auto-reviewed).
3. Label a PR `AI-REVIEW`. 3. Open a PR.
No workflow file, no repo secret, no act-runner needed. (The owner must already No workflow file, no repo secret, no act-runner, no label needed. (The owner
be covered by a user-level webhook — see below. If not, do the one-time must already be covered by a user-level webhook — see below. If not, do the
per-owner setup first.) one-time per-owner setup first.)
## AI-USAGE label — token-usage reporting (optional, opt-in) ## Token-usage reporting (always on)
A review always fires on `AI-REVIEW`. Adding a second label **`AI-USAGE`** on Every opencode review now appends a token-usage report — no label, no env var
the same PR opts the review into appending a token-usage report: needed:
- a `## 🔋 AI usage` section on the review summary body with the **measured** - a `## 🔋 AI usage` section on the review summary body with the **measured**
review total — input / output / reasoning / cache read+write / total tokens, review total — input / output / reasoning / cache read+write / total tokens,
@@ -88,12 +89,8 @@ rendered-body weight (`len(problem)+len(fix)+len(suggestion)`) — an honest
attribution, labelled as such. The totals are real measurements summed from attribution, labelled as such. The totals are real measurements summed from
opencode's `step_finish` events. opencode's `step_finish` events.
`PRAGENT_USAGE_ALWAYS=1` on the Deployment forces usage reporting on for every No-op on the ollama fallback (no usage available). The usage section is part
review (testing / a future default-on) regardless of the label. of the review body, so it's covered by the existing sha-marker dedupe.
Without `AI-USAGE` (regression): no usage section, no 🪙 lines — behaviour
identical to before the feature. The usage section is part of the review body,
so it's covered by the existing sha-marker dedupe.
## Repo-provided static context (`ADDITIONAL_CONTEXT_URL`) ## Repo-provided static context (`ADDITIONAL_CONTEXT_URL`)
@@ -156,17 +153,17 @@ curl -u techspark -X PUT \
The receiver uses a **denylist**, not an allowlist: it reviews on every The receiver uses a **denylist**, not an allowlist: it reviews on every
`pull_request` action **except `closed`** — `opened`, `reopened`, `pull_request` action **except `closed`** — `opened`, `reopened`,
`synchronize`/`synchronized`, `labeled`/`label_updated`, `edited` (title/body), `synchronize`/`synchronized`, `edited` (title/body), `ready_for_review`
`ready_for_review` (draft→ready), `assigned`, `review_requested`, `milestone`, (draft→ready), `assigned`, `review_requested`, `milestone`, … . This is safe
… . This is safe because of two downstream gates: because of two downstream gates:
- the **AI-REVIEW label gate** — payload `labels` reflect current state, so an - the **opt-in gate** — `.pr-review.json:enabled = true` is read from the base
`unlabeled` that *removed* AI-REVIEW fails the gate (no review); an branch, so only repos that opted in get reviewed. A repo that deletes the
`unlabeled` of another label still passes; file between pushes opts out;
- the **sha dedupe** — any same-sha re-fire (title edit, assignee, milestone, - the **sha dedupe** — any same-sha re-fire (title edit, assignee, milestone…)
a label toggle of another label…) is skipped, so the only newly-effective is skipped, so the only newly-effective actions are ones that change the head
actions are ones that change the head sha (`synchronize`, already covered) or sha (`synchronize`, already covered) or move a draft to ready
move a draft to ready (`ready_for_review`) on an un-reviewed sha. (`ready_for_review`) on an un-reviewed sha.
## Threat model ## Threat model
@@ -209,8 +206,8 @@ Additionally: the repo archive is untarred with symlink-escape and
parent-traversal rejection (`_extract_tar_strip_one`), the container runs as parent-traversal rejection (`_extract_tar_strip_one`), the container runs as
uid 10001, and the webhook caps request bodies (`PRAGENT_MAX_BODY_BYTES`, uid 10001, and the webhook caps request bodies (`PRAGENT_MAX_BODY_BYTES`,
default 10 MiB) and concurrent reviews (`PRAGENT_MAX_CONCURRENT_REVIEWS`, default 10 MiB) and concurrent reviews (`PRAGENT_MAX_CONCURRENT_REVIEWS`,
default 2 — each review forks an opencode process, so unbounded threads were a default 2 — each review forks an opencode process, so unbounded threads would be
self-inflicted fork bomb on a label-ten-PRs burst). a self-inflicted fork bomb on any burst of concurrent PRs).
**Residual risk, accepted for a pilot:** the agent still *executes* hostile repo **Residual risk, accepted for a pilot:** the agent still *executes* hostile repo
content indirectly (running the repo's own linters on it) inside a container content indirectly (running the repo's own linters on it) inside a container
@@ -227,7 +224,7 @@ so the `/tmp/pragent-work` emptyDir is writable.
## Multi-lens pipeline (5 default lenses, on by default) ## Multi-lens pipeline (5 default lenses, on by default)
Default `AI-REVIEW` runs spawn **one opencode subprocess per lens in parallel** Default reviews spawn **one opencode subprocess per lens in parallel**
and synthesize the merged findings before posting. Cheaper than 5 sequential and synthesize the merged findings before posting. Cheaper than 5 sequential
reviews because the headroom proxy caches the byte-identical brief across reviews because the headroom proxy caches the byte-identical brief across
lens calls (lenses 2..N hit cache). lens calls (lenses 2..N hit cache).
@@ -483,7 +480,7 @@ cramped model call. `pilot/opencode_review.py` is the glue:
It does **no Gitea I/O and no parsing** — `review_pr` parses the stdout into It does **no Gitea I/O and no parsing** — `review_pr` parses the stdout into
`(summary, findings)`, validates findings against diff anchors, and posts. So `(summary, findings)`, validates findings against diff anchors, and posts. So
all v2 logic (dedupe marker, anchor validation, language-tagged suggestion all v2 logic (dedupe marker, anchor validation, language-tagged suggestion
fencing, posting, optional AI-USAGE attribution) is reused and never depends on fencing, posting, token-usage attribution) is reused and never depends on
the model remembering it. the model remembering it.
The factory lives in the pragent repo root: `opencode.json` (provider/model/ The factory lives in the pragent repo root: `opencode.json` (provider/model/
pilot/ai_review.py
+281 -94
View File
@@ -58,20 +58,18 @@ import urllib.error
import urllib.parse import urllib.parse
import urllib.request import urllib.request
REVIEW_HEADER = "🤖 **AI Review** · pragent pilot · {model} · `{sha}`" REVIEW_HEADER = "🤖 **AI Review** · pragent pilot · {model} · `{sha}` · Merge confidence: {confidence}"
# Hidden marker the dedupe pass scans for. Full sha so a re-push (new sha) is # Hidden marker the dedupe pass scans for. Full sha so a re-push (new sha) is
# never mistaken for an already-reviewed commit, and a label-toggle (same sha) # never mistaken for an already-reviewed commit, and a label-toggle (same sha)
# is correctly skipped. # is correctly skipped.
SHA_MARKER = "<!-- pragent:sha={sha} -->" SHA_MARKER = "<!-- pragent:sha={sha} -->"
_SHA_MARKER_RE = re.compile(r"<!-- pragent:sha=([0-9a-f]{7,40}) -->") _SHA_MARKER_RE = re.compile(r"<!-- pragent:sha=([0-9a-f]{7,40}) -->")
AI_REVIEW_LABEL = "AI-REVIEW" SEVERITIES = ("critical", "high", "medium", "low", "trivial", "info")
# Opt-in label for the token-usage block. Read at render time — see pr_has_label.
AI_USAGE_LABEL = "AI-USAGE"
SEVERITIES = ("critical", "high", "medium", "low")
# Severity rank — higher = more severe. Used by `apply_repo_config` to drop # Severity rank — higher = more severe. Used by `apply_repo_config` to drop
# findings below `severity_threshold`. Critical=3, high=2, medium=1, low=0. # findings below `severity_threshold`. critical=4, high=3, medium=2, low=1,
SEVERITY_RANK = {"low": 0, "medium": 1, "high": 2, "critical": 3} # trivial=0, info=-1.
SEVERITY_RANK = {"info": -1, "trivial": 0, "low": 1, "medium": 2, "high": 3, "critical": 4}
REPO_CONFIG_FILE = ".pr-review.json" REPO_CONFIG_FILE = ".pr-review.json"
# Style → (default max_findings, default severity_threshold). Strict is # Style → (default max_findings, default severity_threshold). Strict is
@@ -90,6 +88,14 @@ STYLE_DEFAULTS: dict[str, tuple[int, str]] = {
# (env) or `.pr-review.json:cost_target` (per repo). # (env) or `.pr-review.json:cost_target` (per repo).
DEFAULT_PRICE_TARGET = "claude-sonnet-5" DEFAULT_PRICE_TARGET = "claude-sonnet-5"
# Default roster of paid providers shown in the equivalent-cost table when
# `.pr-review.json` does not pin `compare_against`. The pilot is free-tier only,
# so this list is the operator's budgeting signal — it answers "what would this
# have cost on a mainstream paid API?". Override per-repo via
# `.pr-review.json:compare_against` (capped at 12 entries; unknown keys are
# dropped with a stderr line at parse time).
DEFAULT_COMPARE_AGAINST = ("claude-sonnet-5", "gpt-5", "gemini-2.5-pro", "grok-4.5")
SYSTEM_PROMPT = """You are a senior, pragmatic code reviewer. Review the pull request diff below. SYSTEM_PROMPT = """You are a senior, pragmatic code reviewer. Review the pull request diff below.
Report ONLY real, actionable issues: correctness bugs, security problems, risky Report ONLY real, actionable issues: correctness bugs, security problems, risky
@@ -101,14 +107,17 @@ Output STRICT JSON only — no prose, no markdown fences. Shape:
{ {
"findings": [ "findings": [
{ {
"severity": "critical|high|medium|low", "severity": "critical|high|medium|low|trivial|info",
"path": "file path exactly as it appears in the diff (`+++ b/` side)", "path": "file path exactly as it appears in the diff (`+++ b/` side)",
"line": <int, the NEW-file line number the issue is on, within the diff>, "line": <int, the NEW-file line number the issue is on, within the diff>,
"problem": "one line: what is wrong", "problem": "one line: what is wrong",
"fix": "one line: how to fix it", "fix": "one line: how to fix it",
"suggestion": "<exact replacement lines for that location, or empty string if you cannot produce safe replacement code>" "suggestion": "<exact replacement lines for that location, or empty string if you cannot produce safe replacement code>"
} }
] ],
"walkthrough": ["2-6 short bullets, file- or change-grouped, plain prose"],
"risk_verdict": "Low|Medium|High|Critical risk: <one-line concrete reason>",
"test_coverage": "Tests added" | "Tests changed" | "No tests for behavioral change" | "No test files in repo"
} }
Rules: Rules:
@@ -120,6 +129,15 @@ Rules:
Keep it minimal — just the changed lines, indented as they would appear in the Keep it minimal — just the changed lines, indented as they would appear in the
file. Leave it empty ("") if a safe textual replacement is not possible (e.g. file. Leave it empty ("") if a safe textual replacement is not possible (e.g.
a missing test, an architectural note). a missing test, an architectural note).
- `walkthrough`: 2-6 short bullets, file- or change-grouped, plain prose.
Default to `[]` when the diff is trivial. Backward compatible: parsers
default to `[]` if absent.
- `risk_verdict`: exactly one line. Lead with "Low|Medium|High|Critical risk:"
followed by a concrete reason. Default to `""` when not applicable.
Backward compatible: parsers default to `""` if absent.
- `test_coverage`: short string. One of "Tests added" / "Tests changed" /
"No tests for behavioral change" / "No test files in repo". Default to `""`
when not applicable. Backward compatible: parsers default to `""` if absent.
- Skip nitpicks, pure formatting, and praise. At most ~15 findings, highest - Skip nitpicks, pure formatting, and praise. At most ~15 findings, highest
severity first. severity first.
- If the diff is clean, output: {"findings": []} - If the diff is clean, output: {"findings": []}
@@ -141,6 +159,26 @@ def truncate_diff(text: str, max_chars: int) -> tuple[str, bool, int]:
return text[:max_chars] + f"\n\n[diff truncated at {max_chars} characters]\n", True, orig_len return text[:max_chars] + f"\n\n[diff truncated at {max_chars} characters]\n", True, orig_len
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)"
def parse_text_blocks(content: list) -> str: def parse_text_blocks(content: list) -> str:
"""Join `type:"text"` blocks from an Anthropic /v1/messages response. """Join `type:"text"` blocks from an Anthropic /v1/messages response.
@@ -158,33 +196,6 @@ def parse_text_blocks(content: list) -> str:
return "\n".join(out).strip() return "\n".join(out).strip()
def pr_has_label(api: str, repo: str, index: str, token: str, label: str) -> bool:
"""True if the PR currently carries `label`. False on any failure.
Read at RENDER time, not at review start. A reviewer labels AI-REVIEW and
AI-USAGE seconds apart; the review claims on the first event and the
second is dropped by the in-flight dedupe, so the trigger payload never
sees the opt-in. Re-reading when the review begins is no better — that is
still milliseconds after the first click. Only a read taken once the
review has finished (a minute or more later) reliably sees the label.
"""
try:
code, raw = gitea_get(api, repo, f"issues/{index}/labels", token)
if code >= 300:
return False
data = json.loads(raw.decode() or "[]")
except Exception as e:
print(f"pragent: could not re-read labels for {repo}#{index}: {e}",
file=sys.stderr, flush=True)
return False
if not isinstance(data, list):
return False
return any(
(isinstance(x, dict) and x.get("name") == label) or x == label
for x in data
)
def _int_env(name: str, default: int) -> int: def _int_env(name: str, default: int) -> int:
"""Read an int from the environment, falling back on anything unparseable. """Read an int from the environment, falling back on anything unparseable.
@@ -204,6 +215,49 @@ def _int_env(name: str, default: int) -> int:
return default return default
# 1-5 merge-verdict score (higher = safer). Buckets:
# 5 = clean (or low/info/trivial only — nothing worth blocking on)
# 4 = medium present
# 3 = high present (operator should at least look)
# 1 = critical present (block the merge by default)
# Cross-lens agreement on any finding takes one more off, floored at 1.
_CONFIDENCE_BADGE = {5: "🟢", 4: "🟢", 3: "🟡", 2: "🟠", 1: "🔴"}
def merge_confidence(findings: list[dict], *, multi_lens_observed: bool = False) -> int:
"""1-5 merge verdict: higher = safer.
Tier drops driven by the most severe finding present:
- critical → 1
- high → 3
- medium → 4
- else → 5 (low / trivial / info / unknown → no drop)
An extra -1 when cross-lens agreement was observed on any finding
(``multi_lens_observed``). The flag is passed in explicitly because the
raw ``_multi_lens`` marker is stripped from findings by the time they
reach this function — first by ``opencode_review.run_lenses_review``
(the ``_``-prefix scrub) and again by ``_normalize_finding`` (the
7-key schema rebuild). The caller (``review_pr``) must capture the
signal before those strips fire. Final score is clamped to [1, 5] so
a critical + multi_lens combo doesn't go negative.
"""
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 multi_lens_observed:
score -= 1
return max(1, min(5, score))
def format_review_body( def format_review_body(
findings: str, findings: str,
model: str, model: str,
@@ -215,15 +269,26 @@ def format_review_body(
risks: list[str] | None = None, risks: list[str] | None = None,
findings_for_table: list[dict] | None = None, findings_for_table: list[dict] | None = None,
inline_count: int = 0, inline_count: int = 0,
confidence: int = 5,
walkthrough: list[str] | None = None,
risk_verdict: str = "",
test_coverage: str = "",
) -> str: ) -> str:
"""Format the posted review summary body. """Format the posted review summary body.
Layout (per the operator's format guide): Layout (per the operator's format guide):
* Header line (``🤖 AI Review …``). * Header line (``🤖 AI Review …``) including the merge-confidence badge.
* **Summary of Changes** — 2–4 bullets of what the PR introduces * **Summary of Changes** — 2–4 bullets of what the PR introduces
(`summary_changes`); falls back to the opencode prose `summary` if (`summary_changes`); falls back to the opencode prose `summary` if
the agent didn't emit the list. the agent didn't emit the list.
* **Risk Verdict** — one-line "<level> risk: <reason>" verdict
(`risk_verdict`); omitted when empty.
* **Walkthrough** — up to 6 file- or change-grouped bullets
(`walkthrough`); the file part is wrapped in backticks so paths
render as code in Gitea. Omitted when empty.
* **Test Coverage** — short `test_coverage` string ("Tests added" /
etc.); omitted when empty.
* **Key Risks & Concerns** — bullets of potential bugs/edge cases * **Key Risks & Concerns** — bullets of potential bugs/edge cases
found across the diff (`risks`). found across the diff (`risks`).
* **Findings Overview** — a Markdown table (severity / location / * **Findings Overview** — a Markdown table (severity / location /
@@ -234,11 +299,22 @@ def format_review_body(
the body stays scannable; cost lines stay inside it. the body stays scannable; cost lines stay inside it.
* Hidden SHA marker — for the dedupe pass. * Hidden SHA marker — for the dedupe pass.
`confidence` is a 1-5 merge verdict rendered as `<N>/5 <badge>` in the
header. Clamped to [1, 5] so a stray value (e.g. 0 from a missing
finding list) doesn't print a broken badge.
Empty `summary_changes` + empty `risks` + empty `summary` collapse into Empty `summary_changes` + empty `risks` + empty `summary` collapse into
a single "Summary of Changes: _no summary provided._" line so the body a single "Summary of Changes: _no summary provided._" line so the body
never looks half-rendered. never looks half-rendered.
""" """
header = REVIEW_HEADER.format(model=model, sha=sha[:8] if sha else "unknown") score = max(1, min(5, confidence))
badge = _CONFIDENCE_BADGE.get(score, "🟢")
confidence_str = f"{score}/5 {badge}"
header = REVIEW_HEADER.format(
model=model,
sha=sha[:8] if sha else "unknown",
confidence=confidence_str,
)
parts: list[str] = [header] parts: list[str] = [header]
# --- Summary of Changes --- # --- Summary of Changes ---
@@ -252,6 +328,32 @@ def format_review_body(
else: else:
parts.append("### Summary of Changes\n\n_No summary provided._") parts.append("### Summary of Changes\n\n_No summary provided._")
# --- Risk Verdict ---
if risk_verdict:
parts.append(f"### Risk Verdict\n\n{risk_verdict}")
# --- Walkthrough ---
wt = list(walkthrough or [])
if wt:
wt = wt[:6]
rendered = []
for item in wt:
# Items typically look like "a.py — adds X" (em-dash separator).
# Wrap the file path in backticks so it renders as code in the
# Gitea markdown body; leave the description as plain prose. When
# no separator is present, render the whole line as plain prose
# (the agent's "plain prose" fallback for change-grouped bullets).
if " — " in item:
path, _, rest = item.partition(" — ")
rendered.append(f"- `{path}` — {rest}")
else:
rendered.append(f"- {item}")
parts.append(f"### Walkthrough\n\n" + "\n".join(rendered))
# --- Test Coverage ---
if test_coverage:
parts.append(f"### Test Coverage\n\n{test_coverage}")
# --- Key Risks & Concerns --- # --- Key Risks & Concerns ---
rs = list(risks or []) rs = list(risks or [])
if rs: if rs:
@@ -652,45 +754,59 @@ def salvage_summary(text: str, max_chars: int = SALVAGE_MAX_CHARS) -> str:
) )
def parse_review_output(text: str) -> tuple[str, list[dict], list[str], list[str]]: def parse_review_output(
"""Parse the opengine's stdout into (summary, findings, summary_changes, risks). text: str,
) -> tuple[str, list[dict], list[str], list[str], list[str], str, str]:
"""Parse the opengine's stdout into a 7-tuple:
(summary, findings, summary_changes, risks,
walkthrough, risk_verdict, test_coverage)
Accepts `{"summary": "...", "summary_changes": [...], "risks": [...], Accepts `{"summary": "...", "summary_changes": [...], "risks": [...],
"findings": [...]}` (the opencode pragent agent), `{"findings": [...]}`, "walkthrough": [...], "risk_verdict": "...", "test_coverage": "...",
or a bare `[...]` of finding dicts. `summary_changes` and `risks` default "findings": [...]}` (the opencode pragent agent), the legacy 4-field
to empty lists; older outputs without them still parse fine. Uses the shape, or a bare `[...]` of finding dicts. The three new fields
LAST fenced block (the pragent agent emits JSON as the final block), with (`walkthrough`, `risk_verdict`, `test_coverage`) default to empty
a tolerant fallback that scans for the last balanced object/array in the list / empty strings when absent — older outputs and the bare-array
prose tail. Never raises. shape stay backward compatible.
Uses the LAST fenced block (the pragent agent emits JSON as the final
block), with a tolerant fallback that scans for the last balanced
object/array in the prose tail. Never raises.
""" """
blob = _last_json_block(text) blob = _last_json_block(text)
if blob is None: if blob is None:
return "", [], [], [] return "", [], [], [], [], "", ""
try: try:
data = json.loads(blob) data = json.loads(blob)
except json.JSONDecodeError: except json.JSONDecodeError:
return "", [], [], [] return "", [], [], [], [], "", ""
summary = "" summary = ""
summary_changes: list[str] = [] summary_changes: list[str] = []
risks: list[str] = [] risks: list[str] = []
walkthrough: list[str] = []
risk_verdict = ""
test_coverage = ""
findings_raw = None findings_raw = None
if isinstance(data, dict): if isinstance(data, dict):
summary = str(data.get("summary", "") or "").strip() summary = str(data.get("summary", "") or "").strip()
summary_changes = _string_list(data.get("summary_changes")) summary_changes = _string_list(data.get("summary_changes"))
risks = _string_list(data.get("risks")) risks = _string_list(data.get("risks"))
walkthrough = _string_list(data.get("walkthrough"))
risk_verdict = str(data.get("risk_verdict", "") or "").strip()
test_coverage = str(data.get("test_coverage", "") or "").strip()
findings_raw = data.get("findings") findings_raw = data.get("findings")
elif isinstance(data, list): elif isinstance(data, list):
# Bare array: each item is a finding; no summary/sections. # Bare array: each item is a finding; no summary/sections.
findings_raw = data findings_raw = data
else: else:
return "", [], [], [] return "", [], [], [], [], "", ""
out = [] out = []
if isinstance(findings_raw, list): if isinstance(findings_raw, list):
for f in findings_raw: for f in findings_raw:
n = _normalize_finding(f) n = _normalize_finding(f)
if n is not None: if n is not None:
out.append(n) out.append(n)
return summary, out, summary_changes, risks return summary, out, summary_changes, risks, walkthrough, risk_verdict, test_coverage
def _string_list(value) -> list[str]: def _string_list(value) -> list[str]:
@@ -937,16 +1053,24 @@ _SEVERITY_EMOJI = {
"high": "🔴", "high": "🔴",
"medium": "🟡", "medium": "🟡",
"low": "🔵", "low": "🔵",
"trivial": "⚪",
"info": "⚪", "info": "⚪",
"nit": "⚪", "nit": "⚪",
} }
# Severities whose own name is rendered verbatim (uppercased) in the badge.
# Anything outside this set falls back to "INFO" so the badge label stays
# a clean short token regardless of what the model emits.
_BADGED_SEVERITY_LABELS = frozenset({
"critical", "high", "medium", "low", "trivial", "info", "nit",
})
def _severity_badge(severity: str) -> str: def _severity_badge(severity: str) -> str:
"""Render the severity as emoji + uppercase label (e.g. ``🔴 [HIGH]``).""" """Render the severity as emoji + uppercase label (e.g. ``🔴 [HIGH]``)."""
sev = (severity or "").lower() sev = (severity or "").lower()
emoji = _SEVERITY_EMOJI.get(sev, "⚪") emoji = _SEVERITY_EMOJI.get(sev, "⚪")
label = sev.upper() if sev in {"critical", "high", "medium", "low"} else "INFO" label = sev.upper() if sev in _BADGED_SEVERITY_LABELS else "INFO"
return f"{emoji} [{label}]" return f"{emoji} [{label}]"
@@ -1013,7 +1137,7 @@ def inline_comment_body(f: dict) -> str:
tok = f.get("_tok_attrib") tok = f.get("_tok_attrib")
if tok is not None: if tok is not None:
pct = (f.get("_tok_pct", 0.0) or 0.0) * 100 pct = (f.get("_tok_pct", 0.0) or 0.0) * 100
body += f"\n\n🪙 ~{tok} tok ({pct:.0f}% · attributed output)" body += f"\n\n🪙 ~{fmt_tokens(tok)} tok ({pct:.0f}% · attributed output)"
return body return body
@@ -1065,9 +1189,13 @@ def findings_table(findings: list[dict]) -> str:
def _render_collapsible_usage(usage: dict | None, model: str, config: dict | None) -> str: def _render_collapsible_usage(usage: dict | None, model: str, config: dict | None) -> str:
"""Render the telemetry as a collapsible ``<details>`` block. """Render the telemetry as a collapsible ``<details>`` block.
Empty string when `usage` is None. The cost-equivalent line is always Empty string when `usage` is None. The equivalent-cost table is the
shown (it's the operator's budgeting signal). The `actual` line is shown operator's budgeting signal — the pilot runs on a free tier, so the
but the FREE-TIER note is collapsed into a single short clause. `actual` line is $0.00; the table shows what the same measured tokens
would bill on mainstream paid APIs (configurable via `compare_against`,
defaulting to ``DEFAULT_COMPARE_AGAINST``). The row matching `cost_target`
is bolded so the price target stands out. The whole table is omitted when
every row would be $0 (no work done).
""" """
if not usage: if not usage:
return "" return ""
@@ -1076,15 +1204,34 @@ def _render_collapsible_usage(usage: dict | None, model: str, config: dict | Non
actual = usage.get("cost") or 0.0 actual = usage.get("cost") or 0.0
actual_s = f"${actual:.4f}" if actual else "$0.00" actual_s = f"${actual:.4f}" if actual else "$0.00"
actual_note = " (headroom glm-5.2:cloud — free tier)" if not actual else "" actual_note = " (headroom glm-5.2:cloud — free tier)" if not actual else ""
price_key, price_err = _resolve_price_target(config) cost_target, price_err = _resolve_price_target(config)
from cost_model import PRICES if price_err:
eq = equivalent_cost(usage, price_key) # Surface config typos loudly but do not pollute the posted summary
eq_s = f"${eq:.4f}" if eq else "$0.00" # body — typos at the table-row level would render as English
eq_label = PRICES[price_key].name # mid-table and look like a model error.
eq_note = ( print(f"pragent: {price_err}", file=sys.stderr, flush=True)
f" _(price target: `{price_key}`; {price_err})_" # Lazy: cost_model has no dep on ai_review, and the ollama path
if price_err else "" # never reaches this branch.
) from cost_model import PRICES as _PRICES
cfg = config or {}
compare: list[str] = list(cfg.get("compare_against") or DEFAULT_COMPARE_AGAINST)
# Always include the resolved cost_target (env + config), even when the
# operator pinned a different `compare_against` roster — the price target
# row is the one maintainers eyeball against. Skip silently if the key
# isn't a known Price (e.g. a typo that slipped past stderr earlier).
if cost_target in _PRICES and cost_target not in compare:
compare.append(cost_target)
eq_rows: list[str] = []
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 ""
eq_rows.append(f"| {bold}{label}{bold} | {cost_str} |")
in_tok = usage.get("input", 0) in_tok = usage.get("input", 0)
out_tok = usage.get("output", 0) out_tok = usage.get("output", 0)
reason_tok = usage.get("reasoning", 0) reason_tok = usage.get("reasoning", 0)
@@ -1102,11 +1249,19 @@ def _render_collapsible_usage(usage: dict | None, model: str, config: dict | Non
"<summary>🔋 AI Usage & Run Details</summary>", "<summary>🔋 AI Usage & Run Details</summary>",
"", "",
f"- **Model / Engine**: `{model}` · opencode · {usage.get('steps', 0)} steps · {dur_s}", f"- **Model / Engine**: `{model}` · opencode · {usage.get('steps', 0)} steps · {dur_s}",
f"- **Total Tokens**: {in_tok} in / {out_tok} out ({reason_tok} reasoning, cache {cache_r} read / {cache_w} write, {total} total)", f"- **Total Tokens**: {fmt_tokens(in_tok)} in / {fmt_tokens(out_tok)} out "
f"- **Est. cost on {eq_label}**: {eq_s}{eq_note}", f"({fmt_tokens(reason_tok)} reasoning, cache {fmt_tokens(cache_r)} read / "
f"{fmt_tokens(cache_w)} write, {fmt_tokens(total)} total)",
f"- **Actual**: {actual_s}{actual_note}", f"- **Actual**: {actual_s}{actual_note}",
f"- **Scope**: {scope}", f"- **Scope**: {scope}",
] ]
if eq_rows:
lines.append("")
lines.append("- **Equivalent cost on paid providers** (this run's tokens):")
lines.append("")
lines.append("| Provider | Cost |")
lines.append("|---|---:|")
lines.extend(eq_rows)
# Multi-lens fan-out: surface the lens roster + summed steps so the user # Multi-lens fan-out: surface the lens roster + summed steps so the user
# can see which lenses contributed (and that triage didn't drop them all). # can see which lenses contributed (and that triage didn't drop them all).
lenses = usage.get("lenses") lenses = usage.get("lenses")
@@ -1246,6 +1401,35 @@ def parse_repo_config(raw: str) -> dict:
if tr is not None: if tr is not None:
out["triage"] = tr out["triage"] = tr
# Repo-level kill-switch: `enabled: false` lets a maintainer pause the bot
# for this repo without removing the file (handy during a flaky provider
# outage). Always written so callers can do `cfg.get("enabled") is False`
# without a separate default — the file itself is committed, so we treat
# absent / wrong-type as an explicit off rather than as "config missing".
en = data.get("enabled")
out["enabled"] = en if isinstance(en, bool) else False
# Compare-against roster: list of `cost_model.PRICES` keys the render layer
# uses to print equivalent-cost lines (one per key) for maintainer
# budgeting. Unknown keys are dropped with a stderr line so a typo is loud.
# Lazy import: `cost_model` has no dep on `ai_review`, and the ollama
# fallback path never hits this branch — keep import-time cost low there.
from cost_model import PRICES as _PRICES
ca = data.get("compare_against")
if isinstance(ca, list):
cleaned: list[str] = []
for x in ca:
if isinstance(x, str) and x.strip() in _PRICES:
cleaned.append(x.strip())
elif isinstance(x, str):
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]
return out return out
@@ -1814,7 +1998,6 @@ def review_pr(
model: str, model: str,
max_tokens: int = 8000, max_tokens: int = 8000,
max_chars: int = 150000, max_chars: int = 150000,
report_usage: bool = False,
base_ref: str = "", base_ref: str = "",
) -> bool: ) -> bool:
"""Run one review and post it as `pragent-bot`. """Run one review and post it as `pragent-bot`.
@@ -1829,10 +2012,11 @@ def review_pr(
from the PR head) so a PR cannot ship its own reviewer instructions; empty from the PR head) so a PR cannot ship its own reviewer instructions; empty
means "the repo's default branch". means "the repo's default branch".
`report_usage`: when True (PR carries the `AI-USAGE` label), the opencode The opencode engine's measured token/cost usage is always rendered as a
engine's measured token/cost usage is rendered as a `## 🔋 AI usage` section `## 🔋 AI usage` section on the review body and an attributed `🪙 ~N tok`
on the review body and an attributed `🪙 ~N tok` line on each inline line on each inline comment when usage data is available (i.e. when the
comment. No-op on the ollama fallback (no usage available). opencode subprocess returned a `usage` dict). No-op on the ollama fallback
(no usage available — `usage` is None).
Returns True on success (including a deliberate skip), False on failure Returns True on success (including a deliberate skip), False on failure
(failure note posted when possible). Never raises — fail-open by design. (failure note posted when possible). Never raises — fail-open by design.
@@ -1913,26 +2097,19 @@ def review_pr(
compression_note=compression_note, compression_note=compression_note,
additional_context=additional_context, additional_context=additional_context,
) )
review_summary, findings, summary_changes, risks = parse_review_output(stdout) review_summary, findings, summary_changes, risks, _walkthrough, _risk_verdict, _test_coverage = parse_review_output(stdout)
if not findings and not review_summary: if not findings and not review_summary:
# The findings JSON was missing or malformed. Don't discard the # The findings JSON was missing or malformed. Don't discard the
# run: salvage the prose, keep the usage report (the label asked # run: salvage the prose, keep the usage report (the tokens were
# for it, and the tokens were spent either way), and log enough # spent either way), and log enough of the raw output to
# of the raw output to diagnose why the agent went off-format. # diagnose why the agent went off-format.
print( print(
f"pragent: {repo}#{index} sha={sha[:8]} unparseable output " f"pragent: {repo}#{index} sha={sha[:8]} unparseable output "
f"({len(stdout)} chars); tail: {stdout[-600:]!r}", f"({len(stdout)} chars); tail: {stdout[-600:]!r}",
file=sys.stderr, flush=True, file=sys.stderr, flush=True,
) )
salvaged = salvage_summary(stdout) salvaged = salvage_summary(stdout)
# The AI-USAGE opt-in is re-checked HERE, at render time: the label usage_section = _render_collapsible_usage(usage, model, config=config) if usage else ""
# is usually applied moments after AI-REVIEW, long after this
# review was claimed and its trigger payload frozen.
if not report_usage:
report_usage = pr_has_label(api, repo, index, token, AI_USAGE_LABEL)
if report_usage and usage and usage.get('output'):
compute_attribution(findings, usage['output'])
usage_section = _render_collapsible_usage(usage, model, config=config) if report_usage else ""
post_review(api, repo, index, token, format_review_body( post_review(api, repo, index, token, format_review_body(
salvaged or "AI review produced no parseable output.", salvaged or "AI review produced no parseable output.",
model, sha, usage_section=usage_section)) model, sha, usage_section=usage_section))
@@ -1955,6 +2132,15 @@ def review_pr(
}) })
except Exception: except Exception:
changed_paths = [] changed_paths = []
# Capture cross-lens agreement BEFORE apply_repo_config — by the time
# findings land in `review_pr` the `_multi_lens` marker has already
# been scrubbed (once by `opencode_review.run_lenses_review`'s
# `_`-prefix strip, again by `_normalize_finding`'s 7-key rebuild),
# so `merge_confidence` cannot read it off the dict. We scan here as
# the convergence point for both engine paths; in practice the kwarg
# currently always passes False, but the structural plumbing is
# correct for any future code path that preserves the flag.
multi_lens = any(f.get("_multi_lens") for f in findings)
kept, _dropped = apply_repo_config(findings, config, changed_paths=changed_paths) kept, _dropped = apply_repo_config(findings, config, changed_paths=changed_paths)
findings = kept findings = kept
if _dropped: if _dropped:
@@ -1968,18 +2154,10 @@ def review_pr(
) )
# Compute attribution so inline comments + the table can show per-comment # Compute attribution so inline comments + the table can show per-comment
# estimates. Only meaningful when we have measured usage AND the PR asked # estimates. Only meaningful when we have measured usage.
# for it. if usage and usage.get("output"):
if report_usage and usage and usage.get("output"):
compute_attribution(findings, usage["output"]) compute_attribution(findings, usage["output"])
# The AI-USAGE opt-in is re-checked HERE, at render time: the label usage_section = _render_collapsible_usage(usage, model, config=config) if usage else ""
# is usually applied moments after AI-REVIEW, long after this
# review was claimed and its trigger payload frozen.
if not report_usage:
report_usage = pr_has_label(api, repo, index, token, AI_USAGE_LABEL)
if report_usage and usage and usage.get('output'):
compute_attribution(findings, usage['output'])
usage_section = _render_collapsible_usage(usage, model, config=config) if report_usage else ""
# Anchor against the RAW diff, never the compressed one. Compression # Anchor against the RAW diff, never the compressed one. Compression
# drops context lines, so a finding on a line that survived in the file # drops context lines, so a finding on a line that survived in the file
@@ -1997,6 +2175,14 @@ def review_pr(
summary_parts = [] summary_parts = []
if bullets: if bullets:
summary_parts.append("### Unanchored Notes\n\n" + bullets) summary_parts.append("### Unanchored Notes\n\n" + bullets)
# 1-5 merge verdict for the header badge. Computed AFTER filtering +
# anchoring so the verdict reflects what the operator sees (a critical
# finding that fails to anchor is still a critical finding). The
# default 5 keeps any failure path (e.g. empty findings) green.
# Cross-lens agreement is passed in via kwarg (see multi_lens scan
# above) because the `_multi_lens` flag is stripped before findings
# reach this call.
confidence = merge_confidence(findings, multi_lens_observed=multi_lens)
summary_body = format_review_body( summary_body = format_review_body(
"\n\n".join(summary_parts), model, sha, "\n\n".join(summary_parts), model, sha,
summary=review_summary, summary=review_summary,
@@ -2005,6 +2191,7 @@ def review_pr(
risks=risks, risks=risks,
findings_for_table=findings, findings_for_table=findings,
inline_count=len(anchored), inline_count=len(anchored),
confidence=confidence,
) )
post_inline_review(api, repo, index, token, summary_body, anchored) post_inline_review(api, repo, index, token, summary_body, anchored)
pilot/cost_model.py
+12 -3
View File
@@ -82,6 +82,15 @@ PRICES: dict[str, Price] = {
"gpt-5.6-sol": Price("GPT-5.6 Sol", 5.00, 30.00, 5.00, 0.50), "gpt-5.6-sol": Price("GPT-5.6 Sol", 5.00, 30.00, 5.00, 0.50),
"gpt-5.6-terra": Price("GPT-5.6 Terra", 2.00, 12.00, 2.00, 0.20), "gpt-5.6-terra": Price("GPT-5.6 Terra", 2.00, 12.00, 2.00, 0.20),
"gpt-5.6-luna": Price("GPT-5.6 Luna", 0.20, 1.20, 0.20, 0.02), "gpt-5.6-luna": Price("GPT-5.6 Luna", 0.20, 1.20, 0.20, 0.02),
# OpenAI — cached_input 0.1x, no separate cache_write
"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 — 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 — cache_write = input
"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),
} }
@@ -171,7 +180,7 @@ DEFAULT_TIERS = [
# Observed runs — the calibration anchor # Observed runs — the calibration anchor
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Real usage reported by the AI-USAGE label, summed from opencode's step_finish # Real usage reported by opencode's step_finish events. Keep this list
# events. Keep this list append-only: it is the only thing separating this model # events. Keep this list append-only: it is the only thing separating this model
# from a guess, and the first entry corrected the tier assumptions by ~15x. # from a guess, and the first entry corrected the tier assumptions by ~15x.
OBSERVED_RUNS: list[dict] = [ OBSERVED_RUNS: list[dict] = [
@@ -357,10 +366,10 @@ def report(tiers: list[Tier], prs_per_month: int, caching: bool, models: list[st
def observed_report(models: list[str]) -> str: def observed_report(models: list[str]) -> str:
"""Price the runs actually measured through the AI-USAGE label.""" """Price the runs actually measured through the opencode usage telemetry."""
if not OBSERVED_RUNS: if not OBSERVED_RUNS:
return "No observed runs recorded yet." return "No observed runs recorded yet."
lines = ["Observed runs (measured via the AI-USAGE label)"] lines = ["Observed runs (measured via opencode step_finish events)"]
for run in OBSERVED_RUNS: for run in OBSERVED_RUNS:
u = observed_usage(run) u = observed_usage(run)
lines.append( lines.append(
pilot/opencode_review.py
+100 -1
View File
@@ -55,6 +55,8 @@ import time
import urllib.error import urllib.error
import urllib.request import urllib.request
from ai_review import _SEVERITY_EMOJI, is_test_path
# Where the factory lives (opencode.json + .opencode/). Default: the pragent # Where the factory lives (opencode.json + .opencode/). Default: the pragent
# repo root (this file is at <root>/pilot/opencode_review.py). # repo root (this file is at <root>/pilot/opencode_review.py).
_DEFAULT_FACTORY = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) _DEFAULT_FACTORY = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
@@ -905,6 +907,86 @@ def synthesize(
return deduped[:per_pr_cap] return deduped[:per_pr_cap]
def _synthesize_summary_fields(
findings: list[dict],
diff: str,
changed_paths: list[str] | None = None,
) -> tuple[list[str], str, str]:
"""Synthesize review-level meta from the merged findings + diff.
Returns (walkthrough, risk_verdict, test_coverage) — the three new
top-level fields in the pragent review JSON shape
(`ai_review.parse_review_output` extracts them as the 5th, 6th, and
7th tuple elements, defaulting to `[]` / `""` when missing).
Real implementation (Task 8). Python fallback used when the lens
fan-out path is engaged (the synthesized JSON fence in `run_lenses_review`
has no model to call, so we build these fields deterministically from
the merged findings + the diff):
- walkthrough: one line per changed file. When findings exist, group
by path and pick the peak-severity problem as the headline; when
no findings exist, just announce "changed".
- risk_verdict: a one-line verdict driven by the highest severity
bucket that has any findings ("Critical risk" / "High risk" /
"Medium risk" / "Low risk").
- test_coverage: "Tests changed" if any changed path matches
`is_test_path`, else "No tests for behavioral change in `<path>`."
pointing at the first non-test path.
"""
# None-safe: callers occasionally pass None when the upstream merger
# short-circuited. Treat as empty so the for-loop and group-by below
# never crash.
findings = findings or []
# 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_lines = (peak.get("problem") or "").splitlines()
problem = problem_lines[0][:80].strip() if problem_lines else ""
emoji = _SEVERITY_EMOJI.get(peak.get("severity", "low"), "⚪")
walkthrough.append(f"`{path}` — {emoji} {problem}")
else:
files = changed_paths if changed_paths is not None else 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
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Per-lens subprocess + parallel fan-out # Per-lens subprocess + parallel fan-out
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1304,10 +1386,27 @@ def run_lenses_review(
{k: v for k, v in f.items() if not k.startswith("_")} {k: v for k, v in f.items() if not k.startswith("_")}
for f in merged for f in merged
] ]
# Synthesize the review-level meta (walkthrough / risk_verdict /
# test_coverage) from the merged findings + diff. Real implementation
# arrives in Task 8; the stub keeps the synthesized JSON shape stable
# so ai_review.parse_review_output can extract the three new fields
# (it defaults them to [] / "" when missing — backward compatible).
walkthrough, risk_verdict, test_coverage = _synthesize_summary_fields(
merged, diff, changed_paths=changed_paths,
)
synthesized_payload = {
"summary": summary,
"summary_changes": [],
"risks": [],
"walkthrough": walkthrough,
"risk_verdict": risk_verdict,
"test_coverage": test_coverage,
"findings": clean_findings,
}
text = ( text = (
f"{summary}\n\n" f"{summary}\n\n"
f"## Findings (multi-lens)\n\n" f"## Findings (multi-lens)\n\n"
f"```json\n{json.dumps({'summary': summary, 'findings': clean_findings}, indent=2)}\n```\n" f"```json\n{json.dumps(synthesized_payload, indent=2)}\n```\n"
) )
if merged_usage is not None: if merged_usage is not None:
merged_usage["duration_s"] = round(time.monotonic() - t0, 1) merged_usage["duration_s"] = round(time.monotonic() - t0, 1)
pilot/webhook_server.py
+80 -51
View File
@@ -2,13 +2,15 @@
"""pragent pilot — central webhook receiver. """pragent pilot — central webhook receiver.
A stdlib-only HTTP server that Gitea posts user-webhook events to. It gates on A stdlib-only HTTP server that Gitea posts user-webhook events to. It gates on
the `AI-REVIEW` PR label, then runs the same review core (`ai_review.review_pr`) the PR's base ref having `.pr-review.json` with `"enabled": true`, then runs
the CI-step pilot uses, posting findings back as `pragent-bot`. the same review core (`ai_review.review_pr`) the CI-step pilot uses, posting
findings back as `pragent-bot`.
Per-owner setup: one Gitea **user-level webhook** per repo-owner fires for every Per-owner setup: one Gitea **user-level webhook** per repo-owner fires for every
repo that owner has; this service filters to labeled PRs. (Gitea 1.26.1 system repo that owner has; this service filters to opted-in PRs. (Gitea 1.26.1 system
webhooks are broken — see pilot/README-webhook.md.) Onboarding a repo = add the webhooks are broken — see pilot/README-webhook.md.) Onboarding a repo = add the
bot as a Write collaborator + create the label + label a PR. bot as a Write collaborator + commit a `.pr-review.json` with `"enabled": true`
on the base ref.
Stdlib only — no pip install, runs on python:3-slim with the scripts mounted. Stdlib only — no pip install, runs on python:3-slim with the scripts mounted.
@@ -34,26 +36,32 @@ Env:
(optional) request-body cap, default 10 MiB (optional) request-body cap, default 10 MiB
""" """
import base64
import hashlib import hashlib
import hmac import hmac
import json import json
import os import os
import threading import threading
import urllib.parse
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from ai_review import review_pr from ai_review import gitea_get, review_pr
try:
import feedback_harvest # optional — absent in CI-step pod, present in
# central webhook service. Harvesting is the
# collection side of the feedback loop.
except ImportError:
feedback_harvest = None
# Pull-request webhook `action` values. We fire on EVERY pull_request action # Pull-request webhook `action` values. We fire on EVERY pull_request action
# except `closed` (no point reviewing a closed/merged PR) — the AI-REVIEW label # except `closed` (no point reviewing a closed/merged PR) — the
# gate + sha dedupe downstream make broadening safe: a same-sha re-fire (title # `.pr-review.json:enabled` gate + sha dedupe downstream make broadening safe:
# edit, assignee, milestone, label toggle of another label…) is skipped by # a same-sha re-fire (title edit, assignee, milestone, label toggle…) is
# `review_pr`'s dedupe, and an `unlabeled` event that removed AI-REVIEW fails # skipped by `review_pr`'s dedupe. Gitea emits GitHub-style `action` names
# the label gate (payload `labels` reflect current state). Gitea emits # (`labeled`, `synchronize`) even though the `X-Gitea-Event-Type` header uses
# GitHub-style `action` names (`labeled`, `synchronize`) even though the # `label_updated` / `synchronized`.
# `X-Gitea-Event-Type` header uses `label_updated` / `synchronized`.
SKIP_ACTIONS = {"closed"} SKIP_ACTIONS = {"closed"}
AI_REVIEW_LABEL = "AI-REVIEW"
AI_USAGE_LABEL = "AI-USAGE"
GITEA_API = os.environ.get("GITEA_API", "http://gitea-http.gitea.svc.cluster.local:3000") GITEA_API = os.environ.get("GITEA_API", "http://gitea-http.gitea.svc.cluster.local:3000")
BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "") BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "")
@@ -65,6 +73,9 @@ WEBHOOK_SECRET = os.environ.get("WEBHOOK_SECRET", "").encode()
PORT = int(os.environ.get("WEBHOOK_PORT", "8080")) PORT = int(os.environ.get("WEBHOOK_PORT", "8080"))
MAX_CONCURRENT = max(1, int(os.environ.get("PRAGENT_MAX_CONCURRENT_REVIEWS", "2"))) MAX_CONCURRENT = max(1, int(os.environ.get("PRAGENT_MAX_CONCURRENT_REVIEWS", "2")))
MAX_BODY_BYTES = int(os.environ.get("PRAGENT_MAX_BODY_BYTES", str(10 * 1024 * 1024))) MAX_BODY_BYTES = int(os.environ.get("PRAGENT_MAX_BODY_BYTES", str(10 * 1024 * 1024)))
# Feedback DB — SQLite mounted at PRAGENT_FEEDBACK_DB. Empty / unset =
# feedback collection disabled (CI-step path doesn't have it).
FEEDBACK_DB = os.environ.get("PRAGENT_FEEDBACK_DB", "")
# Bound on reviews running at once. Every review forks an opencode process that # Bound on reviews running at once. Every review forks an opencode process that
# untars a repo, reads files and shells out to linters, so an unbounded thread # untars a repo, reads files and shells out to linters, so an unbounded thread
@@ -76,28 +87,36 @@ _review_slots = threading.Semaphore(MAX_CONCURRENT)
# Reviews currently accepted or running, keyed (repo, index, sha). The # Reviews currently accepted or running, keyed (repo, index, sha). The
# sha-marker dedupe in `review_pr` reads Gitea *before* posting, so two # sha-marker dedupe in `review_pr` reads Gitea *before* posting, so two
# deliveries for the same commit in flight together both see "not yet reviewed" # deliveries for the same commit in flight together both see "not yet reviewed"
# and both post — the classic check-then-act race, and label-toggling is exactly # and both post — the classic check-then-act race. Common triggers are Gitea
# the kind of thing that fires two deliveries a second apart. This set closes # retries after a slow 202 response and bursty re-fires from a rapid title /
# the window inside one process. # assign / label toggle. This set closes the window inside one process.
_inflight: set[tuple[str, str, str]] = set() _inflight: set[tuple[str, str, str]] = set()
_inflight_lock = threading.Lock() _inflight_lock = threading.Lock()
def _labels_have(labels, name: str) -> bool: def is_repo_enabled(api: str, repo: str, ref: str, token: str) -> bool:
"""True if the Gitea PR `labels` list (dicts with `name`, or bare strings) """True iff `.pr-review.json` on `ref` has `"enabled": true`.
contains `name`."""
if not isinstance(labels, list):
return False
for lab in labels:
if isinstance(lab, dict) and lab.get("name") == name:
return True
if isinstance(lab, str) and lab == name:
return True
return False
Reads from the given ref (typically the PR's base ref). False on any
def _labels_have_ai_review(labels) -> bool: failure: 404, parse error, missing file, missing `enabled`, wrong type.
return _labels_have(labels, AI_REVIEW_LABEL) The bool-coerce of `.get("enabled") is True` rejects the common
gotchas (`null`, `1`, `"yes"`, missing field all yield False).
"""
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
def _verify_signature(raw_body: bytes, headers) -> bool: def _verify_signature(raw_body: bytes, headers) -> bool:
@@ -126,10 +145,6 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
if not repo: if not repo:
return 400, "no repository.full_name" return 400, "no repository.full_name"
labels = pr.get("labels")
if not _labels_have_ai_review(labels):
return 200, f"ignore (no {AI_REVIEW_LABEL} label) action={action}"
index = pr.get("number") index = pr.get("number")
if index is None: if index is None:
return 400, "no pull_request.number" return 400, "no pull_request.number"
@@ -140,26 +155,22 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
base_ref = (pr.get("base") or {}).get("ref", "") or "" 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: if not BOT_TOKEN:
return 500, "PRAGENT_BOT_TOKEN not set" return 500, "PRAGENT_BOT_TOKEN not set"
# AI-USAGE label (opt-in) → append the token-usage section + per-comment 🪙
# lines to the review. PRAGENT_USAGE_ALWAYS forces it on for testing / a
# future default-on.
report_usage = _labels_have(labels, AI_USAGE_LABEL) or bool(
os.environ.get("PRAGENT_USAGE_ALWAYS")
)
key = (repo, str(index), sha) key = (repo, str(index), sha)
if not _claim(key): if not _claim(key):
return 200, f"ignore (already in flight) {repo}#{index} sha={sha[:8]}" return 200, f"ignore (already in flight) {repo}#{index} sha={sha[:8]}"
threading.Thread( threading.Thread(
target=_run_review, target=_run_review,
args=(key, title, body, report_usage, base_ref), args=(key, title, body, base_ref),
daemon=True, daemon=True,
).start() ).start()
return 202, f"reviewing {repo}#{index} action={action} sha={sha[:8]} usage={report_usage}" return 202, f"reviewing {repo}#{index} action={action} sha={sha[:8]}"
def _claim(key: tuple[str, str, str]) -> bool: def _claim(key: tuple[str, str, str]) -> bool:
@@ -177,9 +188,30 @@ def _release(key: tuple[str, str, str]) -> None:
def _run_review( def _run_review(
key: tuple[str, str, str], title: str, body: str, report_usage: bool, base_ref: str key: tuple[str, str, str], title: str, body: str, base_ref: str
) -> None: ) -> None:
repo, index, sha = key repo, index, sha = key
# Harvest reactions on PRIOR bot comments on this PR (best-effort —
# piggy-backs the webhook path so we don't need a separate cron).
# Disabled if feedback_harvest isn't importable (CI-step image) or
# FEEDBACK_DB isn't set.
if FEEDBACK_DB and feedback_harvest is not None:
try:
hstats = feedback_harvest.harvest_for_pr(
api=GITEA_API, token=BOT_TOKEN,
repo=repo, pr_index=int(index), db_path=FEEDBACK_DB,
)
print(
f"pragent-webhook: harvested {repo}#{index} "
f"reviews={hstats['reviews_seen']} "
f"findings={hstats['findings_seen']} "
f"reactions={hstats['reactions_recorded']}",
flush=True,
)
except Exception as e:
# Harvest must never abort a review.
print(f"pragent-webhook: harvest failed for {repo}#{index}: {e}", flush=True)
try: try:
with _review_slots: with _review_slots:
ok = review_pr( ok = review_pr(
@@ -194,10 +226,9 @@ def _run_review(
model=OLLAMA_MODEL, model=OLLAMA_MODEL,
max_tokens=OLLAMA_MAX_TOKENS, max_tokens=OLLAMA_MAX_TOKENS,
max_chars=DIFF_MAX_CHARS, max_chars=DIFF_MAX_CHARS,
report_usage=report_usage,
base_ref=base_ref, base_ref=base_ref,
) )
print(f"pragent-webhook: reviewed {repo}#{index} sha={sha[:8]} ok={ok} usage={report_usage}", flush=True) print(f"pragent-webhook: reviewed {repo}#{index} sha={sha[:8]} ok={ok}", flush=True)
except Exception as e: # review_pr is fail-open, but guard the thread anyway except Exception as e: # review_pr is fail-open, but guard the thread anyway
print(f"pragent-webhook: thread crashed for {repo}#{index}: {e}", flush=True) print(f"pragent-webhook: thread crashed for {repo}#{index}: {e}", flush=True)
finally: finally:
@@ -255,11 +286,9 @@ class Handler(BaseHTTPRequestHandler):
self._send(200, f"ignore event={event}") self._send(200, f"ignore event={event}")
return return
pr0 = payload.get("pull_request") or {} repo_full = (payload.get("repository") or {}).get("full_name")
print( print(
f"pragent-webhook: pull_request action={payload.get('action')} " f"pragent-webhook: pull_request action={payload.get('action')} repo={repo_full}",
f"repo={(payload.get('repository') or {}).get('full_name')} "
f"ai_review={_labels_have_ai_review(pr0.get('labels'))}",
flush=True, flush=True,
) )
status, msg = _handle_pull_request(payload) status, msg = _handle_pull_request(payload)
tests/pilot/test_ai_review.py
+439 -54
View File
@@ -11,15 +11,21 @@ sys.path.insert(0, os.path.join(ROOT, "pilot"))
import ai_review # noqa: E402 import ai_review # noqa: E402
from ai_review import ( # noqa: E402 from ai_review import ( # noqa: E402
_CONFIDENCE_BADGE,
_SEVERITY_EMOJI,
_balanced_json_substring, _balanced_json_substring,
_extract_first_json_object, _extract_first_json_object,
_last_balanced_json, _last_balanced_json,
_normalize_finding,
_render_collapsible_usage, _render_collapsible_usage,
_severity_badge,
build_user_prompt, build_user_prompt,
compute_attribution, compute_attribution,
findings_table, findings_table,
fmt_tokens,
format_review_body, format_review_body,
inline_comment_body, inline_comment_body,
merge_confidence,
parse_diff_anchors, parse_diff_anchors,
parse_findings, parse_findings,
parse_repo_config, parse_repo_config,
@@ -27,6 +33,9 @@ from ai_review import ( # noqa: E402
parse_text_blocks, parse_text_blocks,
prior_review_bodies, prior_review_bodies,
reviewed_shas, reviewed_shas,
REVIEW_HEADER,
SEVERITIES,
SEVERITY_RANK,
split_findings, split_findings,
summary_bullets, summary_bullets,
truncate_diff, truncate_diff,
@@ -316,7 +325,7 @@ def test_inline_comment_body_severity_emoji_mapping():
("medium", "🟡 [MEDIUM]"), ("medium", "🟡 [MEDIUM]"),
("low", "🔵 [LOW]"), ("low", "🔵 [LOW]"),
("info", "⚪ [INFO]"), ("info", "⚪ [INFO]"),
("nit", "⚪ [INFO]"), # "nit" maps to the INFO label ("nit", "⚪ [NIT]"), # legacy alias — renders with its own name
("bogus", "⚪ [INFO]"), # unknown severity falls back to INFO ("bogus", "⚪ [INFO]"), # unknown severity falls back to INFO
] ]
for sev, badge in cases: for sev, badge in cases:
@@ -334,7 +343,7 @@ def test_inline_comment_body_with_token_attribution():
"fix": "f", "suggestion": "", "reference": "", "fix": "f", "suggestion": "", "reference": "",
"_tok_attrib": 1234, "_tok_pct": 0.30} "_tok_attrib": 1234, "_tok_pct": 0.30}
body = inline_comment_body(f) body = inline_comment_body(f)
assert "🪙 ~1234 tok" in body assert "🪙 ~1,234 (1.2K) tok" in body
assert "30%" in body assert "30%" in body
assert "attributed output" in body assert "attributed output" in body
@@ -394,11 +403,11 @@ def test_parse_repo_config_full():
def test_parse_repo_config_partial_and_bad(): def test_parse_repo_config_partial_and_bad():
assert parse_repo_config('{"focus":"not-a-list"}') == {} assert parse_repo_config('{"focus":"not-a-list"}') == {"enabled": False}
assert parse_repo_config('{"focus":["ok"]}') == {"focus": ["ok"]} assert parse_repo_config('{"focus":["ok"]}') == {"focus": ["ok"], "enabled": False}
assert parse_repo_config("") == {} assert parse_repo_config("") == {}
assert parse_repo_config("not json") == {} assert parse_repo_config("not json") == {}
assert parse_repo_config('{"instructions":" "}') == {} assert parse_repo_config('{"instructions":" "}') == {"enabled": False}
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -470,9 +479,9 @@ def test_parse_review_output_bare_findings_no_summary():
def test_parse_review_output_empty_and_bogus(): def test_parse_review_output_empty_and_bogus():
assert parse_review_output("") == ("", [], [], []) assert parse_review_output("") == ("", [], [], [], [], "", "")
assert parse_review_output("no json here") == ("", [], [], []) assert parse_review_output("no json here") == ("", [], [], [], [], "", "")
assert parse_review_output('{"findings":[]}') == ("", [], [], []) assert parse_review_output('{"findings":[]}') == ("", [], [], [], [], "", "")
def test_parse_review_output_uses_last_json_block(): def test_parse_review_output_uses_last_json_block():
@@ -556,6 +565,44 @@ def test_parse_review_output_bare_array_at_tail():
assert len(fs) == 1 assert len(fs) == 1
def test_parse_review_output_extracts_walkthrough_risk_tests():
# The 7-tuple shape carries three new top-level fields:
# walkthrough (list[str]), risk_verdict (str), test_coverage (str).
txt = (
"```json\n"
"{\n"
' "summary": "x",\n'
' "summary_changes": [],\n'
' "risks": [],\n'
' "walkthrough": ["a.py: adds X", "b.py: refactors Y"],\n'
' "risk_verdict": "Low risk.",\n'
' "test_coverage": "No tests for behavioral change in a.py.",\n'
' "findings": []\n'
"}\n"
"```"
)
summary, findings, _changes, _risks, walkthrough, risk_verdict, test_coverage = (
parse_review_output(txt)
)
assert summary == "x"
assert findings == []
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():
# Backward-compatible: the 4-tuple shape still parses fine; the new
# fields default to empty list / empty string.
out = parse_review_output('{"summary":"x","findings":[]}')
summary, findings, _changes, _risks, walkthrough, risk_verdict, test_coverage = out
assert summary == "x"
assert findings == []
assert walkthrough == []
assert risk_verdict == ""
assert test_coverage == ""
def test_scan_balanced_handles_braces_in_strings(): def test_scan_balanced_handles_braces_in_strings():
# The JSON scanner must not be fooled by `{` or `}` inside string literals. # The JSON scanner must not be fooled by `{` or `}` inside string literals.
s = '{"a":"contains { and }","b":1}' s = '{"a":"contains { and }","b":1}'
@@ -616,35 +663,6 @@ def test_reference_non_url_renders_as_plain_text():
assert "](CVE-" not in body assert "](CVE-" not in body
def test_pr_has_label_reads_the_live_labels(monkeypatch):
# The AI-USAGE opt-in is read at render time, not from the trigger
# payload: labelling AI-REVIEW then AI-USAGE is two events, the review
# claims on the first, and the second is dropped by the in-flight dedupe.
seen = {}
def _get(api, repo, path, token, accept="application/json"):
seen["path"] = path
return 200, b'[{"name": "AI-REVIEW"}, {"name": "AI-USAGE"}]'
monkeypatch.setattr(ai_review, "gitea_get", _get)
assert ai_review.pr_has_label("http://api", "o/r", "9", "t", "AI-USAGE") is True
assert seen["path"] == "issues/9/labels"
assert ai_review.pr_has_label("http://api", "o/r", "9", "t", "NOPE") is False
def test_pr_has_label_survives_a_broken_api(monkeypatch):
def _boom(*a, **k):
raise RuntimeError("gitea down")
monkeypatch.setattr(ai_review, "gitea_get", _boom)
assert ai_review.pr_has_label("http://api", "o/r", "9", "t", "AI-USAGE") is False
monkeypatch.setattr(ai_review, "gitea_get", lambda *a, **k: (404, b"nope"))
assert ai_review.pr_has_label("http://api", "o/r", "9", "t", "AI-USAGE") is False
monkeypatch.setattr(ai_review, "gitea_get", lambda *a, **k: (200, b'{"not": "a list"}'))
assert ai_review.pr_has_label("http://api", "o/r", "9", "t", "AI-USAGE") is False
def test_int_env_falls_back_on_garbage(monkeypatch, capsys): def test_int_env_falls_back_on_garbage(monkeypatch, capsys):
monkeypatch.setenv("PRAGENT_DIFF_CONTEXT", "two") monkeypatch.setenv("PRAGENT_DIFF_CONTEXT", "two")
assert ai_review._int_env("PRAGENT_DIFF_CONTEXT", 1) == 1 assert ai_review._int_env("PRAGENT_DIFF_CONTEXT", 1) == 1
@@ -756,7 +774,7 @@ def test_render_collapsible_usage_renders_totals():
assert "`glm-5.2:cloud`" in sec assert "`glm-5.2:cloud`" in sec
assert "7 steps" in sec assert "7 steps" in sec
assert "142.0s" in sec assert "142.0s" in sec
assert "18420 in / 612 out" in sec and "19032 total" in sec assert "18,420 (18.4K) in / 612 out" in sec and "19,032 (19.0K) total" in sec
assert "$0.00" in sec assert "$0.00" in sec
assert "Whole-repo checkout" in sec assert "Whole-repo checkout" in sec
assert "attributed" in sec assert "attributed" in sec
@@ -812,6 +830,43 @@ def test_format_review_body_with_summary_changes_and_risks():
assert "`a.py:1`" in body assert "`a.py:1`" in body
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="",
)
assert body # non-empty
def test_render_collapsible_usage_contains_details(): def test_render_collapsible_usage_contains_details():
usage = { usage = {
"model": "glm-5.2:cloud", "input": 1000, "output": 200, "reasoning": 0, "model": "glm-5.2:cloud", "input": 1000, "output": 200, "reasoning": 0,
@@ -823,7 +878,7 @@ def test_render_collapsible_usage_contains_details():
assert "<summary>🔋 AI Usage & Run Details</summary>" in block assert "<summary>🔋 AI Usage & Run Details</summary>" in block
assert "</details>" in block assert "</details>" in block
assert "glm-5.2:cloud" in block assert "glm-5.2:cloud" in block
assert "1000 in / 200 out" in block assert "1,000 (1.0K) in / 200 out" in block
def test_render_collapsible_usage_empty_when_no_usage(): def test_render_collapsible_usage_empty_when_no_usage():
@@ -889,7 +944,10 @@ def test_parse_repo_config_still_accepts_normal_config():
cfg = parse_repo_config(json.dumps({ cfg = parse_repo_config(json.dumps({
"focus": ["security"], "languages": ["go"], "instructions": "No bare throw.", "focus": ["security"], "languages": ["go"], "instructions": "No bare throw.",
})) }))
assert cfg == {"focus": ["security"], "languages": ["go"], "instructions": "No bare throw."} assert cfg == {
"focus": ["security"], "languages": ["go"], "instructions": "No bare throw.",
"enabled": False,
}
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -911,7 +969,7 @@ def test_fetch_repo_config_uses_given_base_ref(monkeypatch):
monkeypatch.setattr(ai_review, "gitea_get", fake_get) monkeypatch.setattr(ai_review, "gitea_get", fake_get)
cfg = ai_review.fetch_repo_config("http://g", "o/r", "tok", ref="main") cfg = ai_review.fetch_repo_config("http://g", "o/r", "tok", ref="main")
assert cfg == {"focus": ["security"]} assert cfg == {"focus": ["security"], "enabled": False}
assert seen["path"] == "contents/.pr-review.json?ref=main" assert seen["path"] == "contents/.pr-review.json?ref=main"
@@ -1078,19 +1136,22 @@ def test_usage_block_shows_equivalent_provider_cost():
"cache_read": 0, "cache_write": 0, "total": 204000, "cache_read": 0, "cache_write": 0, "total": 204000,
"cost": 0.0, "steps": 6, "duration_s": 100.0} "cost": 0.0, "steps": 6, "duration_s": 100.0}
sec = ai_review._render_collapsible_usage(usage, "glm-5.2:cloud", config=None) sec = ai_review._render_collapsible_usage(usage, "glm-5.2:cloud", config=None)
# Two cost lines now: an equivalent (default Sonnet 5) AND the $0 actual. # New layout: equivalent-cost table instead of a single "Est. cost on …"
# line. The default compare_against is sonnet-5, gpt-5, gemini-2.5-pro,
# grok-4.5; cost_target defaults to sonnet-5 (bolded).
assert "🔋 AI Usage & Run Details" in sec assert "🔋 AI Usage & Run Details" in sec
assert "**Est. cost on Claude Sonnet 5**" in sec
assert "**Actual**: $0.00" in sec assert "**Actual**: $0.00" in sec
assert "free tier" in sec assert "free tier" in sec
# Equivalent should be > 0 for non-trivial token counts.
assert "$0.00" in sec # the actual line assert "$0.00" in sec # the actual line
# And a non-zero one for the equivalent. # Multi-provider table header present, default roster rendered, default
import re # cost_target (Sonnet 5) is the bolded row.
cost_lines = [ln for ln in sec.splitlines() if "cost on" in ln] assert "| Provider | Cost |" in sec
assert len(cost_lines) == 1 assert "**Claude Sonnet 5**" in sec
assert re.search(r"\$\d", cost_lines[0]) is not None assert "GPT-5" in sec
assert "$0.00" not in cost_lines[0] assert "Gemini 2.5 Pro" in sec
assert "Grok 4.5" in sec
# 200k * $2/MTok + 4k * $10/MTok → $0.44
assert "$0.44" in sec
def test_usage_block_honors_cost_target(monkeypatch): def test_usage_block_honors_cost_target(monkeypatch):
@@ -1117,17 +1178,22 @@ def test_usage_block_respects_repo_config_cost_target(monkeypatch):
assert "$0.0075" in sec assert "$0.0075" in sec
def test_usage_block_reports_unknown_price_target(): def test_usage_block_reports_unknown_price_target(capsys):
usage = {"input": 100, "output": 100, "reasoning": 0, usage = {"input": 100, "output": 100, "reasoning": 0,
"cache_read": 0, "cache_write": 0, "total": 200, "cache_read": 0, "cache_write": 0, "total": 200,
"cost": 0.0, "steps": 1, "duration_s": 1.0} "cost": 0.0, "steps": 1, "duration_s": 1.0}
sec = ai_review._render_collapsible_usage( sec = ai_review._render_collapsible_usage(
usage, "glm-5.2:cloud", config={"cost_target": "bogus-model"} usage, "glm-5.2:cloud", config={"cost_target": "bogus-model"}
) )
# Falls back to default + surfaces the error in the line. # Falls back to default. The error now goes to stderr (otherwise it would
# land mid-table and look like a model error in the posted summary).
assert "Claude Sonnet 5" in sec assert "Claude Sonnet 5" in sec
assert "unknown price target" in sec assert "**Claude Sonnet 5**" in sec # bolded as the resolved cost_target
assert "bogus-model" in sec assert "bogus-model" not in sec
assert "unknown price target" not in sec
err = capsys.readouterr().err
assert "unknown price target" in err
assert "bogus-model" in err
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -1629,3 +1695,322 @@ def test_render_collapsible_usage_omits_lenses_when_single_primary():
out = _render_collapsible_usage(usage, "headroom/glm-5.2:cloud", None) out = _render_collapsible_usage(usage, "headroom/glm-5.2:cloud", None)
assert "Lenses" not in out assert "Lenses" not in out
# ---------------------------------------------------------------------------
# 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) == "?"
# ---------------------------------------------------------------------------
# fmt_tokens — applied in usage + inline comment bodies (Task 3)
# ---------------------------------------------------------------------------
def test_collapsible_usage_renders_humanized_tokens():
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
# ---------------------------------------------------------------------------
# Multi-provider equivalent-cost table — Task 10
# ---------------------------------------------------------------------------
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"})
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
def test_inline_comment_body_humanized_tokens():
# Value chosen > 1000 so fmt_tokens actually adds the comma + short suffix;
# the plan's 362 would render identically with or without fmt_tokens.
f = {"severity": "medium", "path": "x.py", "line": 1,
"problem": "p", "fix": "", "suggestion": "", "reference": "",
"_tok_attrib": 17303, "_tok_pct": 0.11}
body = inline_comment_body(f)
assert "🪙 ~17,303 (17.3K) tok" in body
# ---------------------------------------------------------------------------
# Severity levels — Task 4 (add trivial + info)
# ---------------------------------------------------------------------------
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+ threshold:
# medium (rank 2) → kept
# low (rank 1) → DROPPED
# trivial (rank 0) → DROPPED
# info (rank -1) → DROPPED
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 = ai_review.apply_repo_config(findings, cfg, changed_paths=["x.py"])
sev_kept = [f["severity"] for f in kept]
sev_dropped = [f["severity"] for f in dropped]
assert "info" in sev_dropped
assert "trivial" in sev_dropped
assert "low" in sev_dropped
assert "medium" in sev_kept
# and nothing else
assert len(kept) == 1
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"
def test_emoji_for_trivial_and_info_is_neutral():
# The plan's emoji table maps trivial/info to ⚪
assert _SEVERITY_EMOJI["trivial"] == "⚪"
assert _SEVERITY_EMOJI["info"] == "⚪"
def test_severity_badge_labels_each_known_severity():
# Trivial and info (and legacy nit) should render with their own name,
# not fall back to "INFO".
for sev in ("critical", "high", "medium", "low", "trivial", "info", "nit"):
badge = _severity_badge(sev)
assert f"[{sev.upper()}]" in badge, (sev, badge)
# ---------------------------------------------------------------------------
# parse_repo_config — `enabled` (kill-switch) + `compare_against` (cost roster)
# ---------------------------------------------------------------------------
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():
# absent in returned cfg; defaults applied in render, not parse_repo_config
cfg = parse_repo_config('{}')
assert "compare_against" not in cfg
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(monkeypatch):
"""13+ valid keys must be truncated to the first 12; invalid keys are
dropped and do not count. Inject a 13th PRICES entry via monkeypatch so
the [:12] cap actually fires (cost_model.PRICES has exactly 12 keys
today, which would otherwise make the cap a no-op)."""
import cost_model as cm
monkeypatch.setitem(
cm.PRICES, "fake-model-13", cm.Price("Fake", 1.00, 2.00, 1.00, 0.10))
valid = list(cm.PRICES) # 13 unique keys (12 real + 1 test-only)
raw = valid + ["bogus-extra"] # 13 valid + 1 invalid
cfg = parse_repo_config(json.dumps({"compare_against": raw}))
assert len(cfg["compare_against"]) == 12
assert cfg["compare_against"] == valid[:12]
assert "fake-model-13" not in cfg["compare_against"]
assert "bogus-extra" not in cfg["compare_against"]
# ---------------------------------------------------------------------------
# Task 6 — merge_confidence + REVIEW_HEADER confidence badge
# ---------------------------------------------------------------------------
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():
# The flag has moved to a kwarg; passing `_multi_lens` on the dict is no
# longer enough — the kwarg is the only path that drops the score.
f = {"severity": "low"}
assert merge_confidence([f], multi_lens_observed=True) == 4
def test_merge_confidence_multi_lens_survives_normalization():
"""Real flow: `_multi_lens` is set on the raw finding, but stripped by
`_normalize_finding`. `merge_confidence(...)` with only the kwarg sees a
normalized finding; the dedup must be triggered by `multi_lens_observed=`
being true, not by reading `_multi_lens` off the dict."""
raw = {"_multi_lens": True, "severity": "low", "path": "x", "line": 1,
"problem": "p", "fix": "", "suggestion": "", "reference": ""}
normalized = _normalize_finding(raw)
assert "_multi_lens" not in normalized # confirms the strip
# Now call merge_confidence the way review_pr will:
assert merge_confidence([normalized], multi_lens_observed=True) == 4
# And without the kwarg, the flag-on-dict path is gone:
assert merge_confidence([normalized]) == 5
def test_merge_confidence_clamped():
# Three critical findings must NOT take the score below 1.
f = {"severity": "critical"}
assert merge_confidence([f, f, f]) == 1
def test_review_header_includes_confidence():
# REVIEW_HEADER gains a {confidence} placeholder; verify the format works.
h = REVIEW_HEADER.format(model="glm-5.2:cloud", sha="abc1234567", confidence="3/5 🟡")
assert "Merge confidence: 3/5 🟡" in h
def test_confidence_badge_table_complete():
# Sanity-check the badge table the render layer reads from.
assert _CONFIDENCE_BADGE == {5: "🟢", 4: "🟢", 3: "🟡", 2: "🟠", 1: "🔴"}
def test_format_review_body_default_confidence_is_green():
# Default confidence kwarg should produce a green 5/5 badge in the header,
# matching the pre-existing "clean PR" semantics.
body = format_review_body("- [high] x:1 — bug", "glm-5.2:cloud", "abcdef1234567890")
assert "Merge confidence: 5/5 🟢" in body
def test_format_review_body_low_confidence_shows_red_badge():
body = format_review_body(
"- [critical] x:1 — bug", "glm-5.2:cloud", "abcdef1234567890",
confidence=1,
)
assert "Merge confidence: 1/5 🔴" in body
def test_format_review_body_confidence_clamps_out_of_range():
# Out-of-range confidence is clamped to [1, 5] in the badge string.
body_hi = format_review_body("- x", "glm-5.2:cloud", "abcdef1234567890", confidence=99)
assert "Merge confidence: 5/5 🟢" in body_hi
body_lo = format_review_body("- x", "glm-5.2:cloud", "abcdef1234567890", confidence=0)
assert "Merge confidence: 1/5 🔴" in body_lo
tests/pilot/test_cost_model.py
+36
View File
@@ -244,3 +244,39 @@ def test_model_is_within_an_order_of_magnitude_of_the_measurement():
predicted = cm.tier_usage(modelled, FACTORY, caching=False).total_input predicted = cm.tier_usage(modelled, FACTORY, caching=False).total_input
measured = run["input"] measured = run["input"]
assert 0.4 < predicted / measured < 2.5, (predicted, measured) assert 0.4 < predicted / measured < 2.5, (predicted, measured)
# ---------------------------------------------------------------------------
# PRICES — the multi-provider table (GPT / Gemini / Grok)
# ---------------------------------------------------------------------------
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
tests/pilot/test_opencode_review.py
+94 -2
View File
@@ -844,7 +844,9 @@ def test_no_surface_response_parses_as_an_empty_review():
import ai_review import ai_review
text, usage = oc._no_surface_response("o/r", "9", "abc12345", 3) text, usage = oc._no_surface_response("o/r", "9", "abc12345", 3)
assert usage is None assert usage is None
summary, findings, _changes, _risks = ai_review.parse_review_output(text) summary, findings, _changes, _risks, _walkthrough, _risk_verdict, _test_coverage = (
ai_review.parse_review_output(text)
)
assert findings == [] assert findings == []
assert summary # non-empty, so ai_review does NOT take the salvage branch assert summary # non-empty, so ai_review does NOT take the salvage branch
assert "no review surface" in summary.lower() assert "no review surface" in summary.lower()
@@ -854,6 +856,96 @@ def test_no_surface_response_parses_as_an_empty_review():
def test_no_surface_response_zero_lenses_wording(): def test_no_surface_response_zero_lenses_wording():
import ai_review import ai_review
text, _ = oc._no_surface_response("o/r", "9", "abc12345", 0) text, _ = oc._no_surface_response("o/r", "9", "abc12345", 0)
summary, findings, _c, _r = ai_review.parse_review_output(text) summary, findings, _c, _r, _w, _rv, _tc = ai_review.parse_review_output(text)
assert findings == [] assert findings == []
assert "after path filtering" in summary assert "after path filtering" in summary
# ---------------------------------------------------------------------------
# _synthesize_summary_fields — Task 8: real Python fallback implementation
# ---------------------------------------------------------------------------
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, _, _ = oc._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, _, _ = oc._synthesize_summary_fields(
[],
"diff --git a/x.py b/x.py\n@@ -1 +1 @@\n-old\n+new\n+++ b/x.py\n",
)
assert any("x.py" in line for line in w)
def test_synthesize_risk_verdict_critical():
findings = [{"severity": "critical"}]
_, rv, _ = oc._synthesize_summary_fields(findings, "")
assert "Critical risk" in rv
def test_synthesize_risk_verdict_clean():
_, rv, _ = oc._synthesize_summary_fields([], "")
assert "Low risk" in rv
def test_synthesize_test_coverage_with_test_path():
_, _, tc = oc._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 = oc._synthesize_summary_fields(
[], "+diff\n", changed_paths=["pilot/foo.py"])
assert "No tests for behavioral change" in tc
def test_synthesize_walkthrough_picks_peak_severity_per_path():
# Three findings on the same path, with mixed severities. The walkthrough
# headline should use the PEAK severity's emoji (critical = 🔴), not the
# lexicographic-first severity (low).
findings = [
{"path": "x.py", "line": 1, "severity": "low",
"problem": "minor nit"},
{"path": "x.py", "line": 5, "severity": "critical",
"problem": "sql injection"},
{"path": "x.py", "line": 9, "severity": "high",
"problem": "auth bypass"},
]
w, _, _ = oc._synthesize_summary_fields(findings, "")
assert len(w) == 1
line = w[0]
assert "`x.py`" in line
assert "🔴" in line # critical = 🔴
assert "🟡" not in line
assert "🔵" not in line
assert "sql injection" in line # critical finding's problem, not low's
def test_synthesize_summary_fields_none_findings_safe():
# Old code crashed in risk_verdict with `for f in findings:` on None.
# After the `findings = findings or []` guard, None behaves like [].
w, rv, tc = oc._synthesize_summary_fields(None, "")
assert isinstance(w, list)
assert rv.startswith("Low risk")
# walkthrough should fall through to the diff-derived path list — empty
# diff produces no lines, but no crash is the point.
assert tc == ""
def test_synthesize_walkthrough_empty_problem_does_not_crash():
# An empty `problem` should render as "`a.py` — emoji" with a trailing
# space, not raise. Regression guard for splitlines()[0][:80].strip().
findings = [{"path": "a.py", "line": 1,
"severity": "low", "problem": ""}]
w, _, _ = oc._synthesize_summary_fields(findings, "")
assert len(w) == 1
assert "`a.py`" in w[0]
assert "🔵" in w[0] # low severity emoji
tests/pilot/test_webhook_server.py
+60 -14
View File
@@ -1,4 +1,5 @@
"""Unit tests for the webhook receiver's gating, dedupe and limits. No network.""" """Unit tests for the webhook receiver's gating, dedupe and limits. No network."""
import base64
import os import os
import sys import sys
import threading import threading
@@ -15,7 +16,6 @@ def _payload(**over):
"number": 7, "number": 7,
"title": "t", "title": "t",
"body": "b", "body": "b",
"labels": [{"name": "AI-REVIEW"}],
"head": {"sha": "a" * 40}, "head": {"sha": "a" * 40},
"base": {"ref": "main"}, "base": {"ref": "main"},
} }
@@ -25,16 +25,11 @@ def _payload(**over):
return p return p
# --------------------------------------------------------------------------- def _enable_repo(monkeypatch, enabled: bool = True):
# label gating """Patch `is_repo_enabled` to the given bool for handler tests."""
# --------------------------------------------------------------------------- monkeypatch.setattr(
"webhook_server.is_repo_enabled", lambda *a, **kw: enabled
)
def test_labels_have_matches_dicts_and_strings():
assert ws._labels_have([{"name": "AI-REVIEW"}], "AI-REVIEW")
assert ws._labels_have(["AI-REVIEW"], "AI-REVIEW")
assert not ws._labels_have([{"name": "other"}], "AI-REVIEW")
assert not ws._labels_have(None, "AI-REVIEW")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -75,6 +70,7 @@ def test_claim_is_thread_safe():
def test_duplicate_delivery_for_same_sha_is_not_reviewed_twice(monkeypatch): def test_duplicate_delivery_for_same_sha_is_not_reviewed_twice(monkeypatch):
started = [] started = []
monkeypatch.setattr(ws, "BOT_TOKEN", "tok") monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
_enable_repo(monkeypatch)
class FakeThread: class FakeThread:
def __init__(self, target, args, daemon): def __init__(self, target, args, daemon):
@@ -97,6 +93,7 @@ def test_duplicate_delivery_for_same_sha_is_not_reviewed_twice(monkeypatch):
def test_base_ref_is_passed_to_the_review_thread(monkeypatch): def test_base_ref_is_passed_to_the_review_thread(monkeypatch):
started = [] started = []
monkeypatch.setattr(ws, "BOT_TOKEN", "tok") monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
_enable_repo(monkeypatch)
class FakeThread: class FakeThread:
def __init__(self, target, args, daemon): def __init__(self, target, args, daemon):
@@ -114,16 +111,33 @@ def test_base_ref_is_passed_to_the_review_thread(monkeypatch):
def test_closed_action_is_ignored(monkeypatch): def test_closed_action_is_ignored(monkeypatch):
monkeypatch.setattr(ws, "BOT_TOKEN", "tok") monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
_enable_repo(monkeypatch)
status, msg = ws._handle_pull_request(_payload(action="closed")) status, msg = ws._handle_pull_request(_payload(action="closed"))
assert status == 200 assert status == 200
assert "ignore" in msg assert "ignore" in msg
def test_missing_label_is_ignored(monkeypatch): def test_handle_pull_request_skips_when_repo_not_enabled(monkeypatch):
_enable_repo(monkeypatch, enabled=False)
started = []
monkeypatch.setattr(ws, "BOT_TOKEN", "tok") monkeypatch.setattr(ws, "BOT_TOKEN", "tok")
status, msg = ws._handle_pull_request(_payload(pr={"labels": [{"name": "wip"}]}))
class FakeThread:
def __init__(self, target, args, daemon):
self.args = args
def start(self):
started.append(self.args)
monkeypatch.setattr(ws.threading, "Thread", FakeThread)
ws._release(("o/r", "7", "a" * 40))
status, msg = ws._handle_pull_request(_payload())
assert status == 200 assert status == 200
assert "AI-REVIEW" in msg assert "skip" in msg and "repo not opted in" in msg
assert "opened" in msg
assert started == []
ws._release(("o/r", "7", "a" * 40))
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -134,3 +148,35 @@ def test_missing_label_is_ignored(monkeypatch):
def test_review_slots_bound_matches_config(): def test_review_slots_bound_matches_config():
assert ws.MAX_CONCURRENT >= 1 assert ws.MAX_CONCURRENT >= 1
assert ws._review_slots._value <= ws.MAX_CONCURRENT assert ws._review_slots._value <= ws.MAX_CONCURRENT
# ---------------------------------------------------------------------------
# is_repo_enabled — reads `.pr-review.json` from the PR base ref and parses
# its `enabled` flag. False on any failure (404, parse error, missing field).
# ---------------------------------------------------------------------------
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 ws.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 ws.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 ws.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 ws.is_repo_enabled("api", "owner/repo", "main", "tok") is False