feat: token-usage reporting (AI-USAGE label) #6

Merged
gitea_admin merged 3 commits from feat/token-usage into main 2026-08-18 04:32:19 +00:00
8 changed files with 577 additions and 53 deletions
+20 -9
View File
@@ -51,9 +51,19 @@ read the full file around a flagged line, not just the diff hunk.
2. **Map the change.** Skim the diff. Note the changed paths, the languages, and
whether the change touches security-sensitive areas (auth, crypto, SQL, file
I/O, deserialization, CI/supply-chain, secrets).
I/O, deserialization, CI/supply-chain, secrets). The brief lists the changed
files explicitly under "Changed files" — use that as your focus list.
3. **Run the repo's own checks via bash.** Detect tooling and run it on the
3. **Ground findings in context.** For each changed file, before finalizing any
finding, `read`/`grep` its **callers, imports, sibling functions, and type
definitions** so your findings reflect how the change is actually used, not
the hunk in isolation. The repo is checked out at the head sha, so the
surrounding code is on disk — use it. Keep it bounded: stop exploring a file
once the finding is grounded (13 related files per finding); do NOT do
unbounded whole-repo walks (token cost, and the focus is the diff's
neighbourhood).
4. **Run the repo's own checks via bash.** Detect tooling and run it on the
CHANGED files only (keep it fast, keep tokens low):
- TS/JS: `npx --no-install tsc --noEmit` if `tsconfig.json` exists; `npx --no-install eslint <changed>` if configured.
- Python: `ruff check <changed>` or `python -m pyright <changed>` / `mypy` if configured.
@@ -62,18 +72,19 @@ read the full file around a flagged line, not just the diff hunk.
- Never run install/build steps (`npm install`, `go mod download`, etc.) — too slow / too much output. If a check needs deps that aren't installed, skip it and note that.
- Capture only diagnostics (errors/warnings), not success prose.
4. **Find real issues.** Combine: the diff, the surrounding code you read, and the
linter/typecheck diagnostics. Report ONLY real, actionable issues — correctness
bugs, security problems, risky changes, missing tests for changed behavior,
breaking API/contract changes. Skip praise, nitpicks, pure formatting.
5. **Find real issues.** Combine: the diff, the surrounding context you read in
step 3, and the linter/typecheck diagnostics. Report ONLY real, actionable
issues — correctness bugs, security problems, risky changes, missing tests
for changed behavior, breaking API/contract changes. Skip praise, nitpicks,
pure formatting.
5. **References.** When a finding involves a specific library API, known
6. **References.** When a finding involves a specific library API, known
vulnerability, or footgun, use `webfetch` to confirm it (e.g. a CVE page, the
library docs) and put the URL in the finding's `reference` field. Leave
`reference` empty when there's nothing authoritative to link. Don't fetch for
the sake of it — keep it lean.
6. **Delegate on heavy diffs.** If the diff is large (>~400 changed lines) OR
7. **Delegate on heavy diffs.** If the diff is large (>~400 changed lines) OR
touches auth/crypto/SQL/deserialization/CI, delegate that lens to a subagent
via the Task tool:
- `@security` — injection, auth, secrets, supply-chain, unsafe deserialization.
@@ -83,7 +94,7 @@ read the full file around a flagged line, not just the diff hunk.
keep highest severity). For small/medium diffs, do all lenses inline yourself —
do NOT spawn subagents. Cost must scale with PR size.
7. **Anchor every finding.** Each finding's `line` MUST be a line that exists in
8. **Anchor every finding.** Each finding's `line` MUST be a line that exists in
the POST-CHANGE version of `path` — a context line or an added `+` line shown
in the diff. Never a removed line. If unsure, use the closest context line you
can see in the diff. A finding with a bad line gets folded into the summary as
@@ -41,6 +41,14 @@ the diff. A misanchored finding becomes a summary bullet instead of an inline
comment, so correct anchoring is what gets a finding shown inline with its
suggested-fix code block (language-highlighted) rather than demoted to a bullet.
## Ground each finding in context
Don't flag a hunk in isolation. For each changed file, read its callers,
imports, sibling functions, and type definitions (the repo is checked out at
the head sha), and make sure the finding holds against how the change is
actually used. Keep it bounded — 13 related files per finding, no unbounded
whole-repo walks.
## Honoring repo config
If `.pr-review.json` is present, honor it:
+55 -7
View File
@@ -8,13 +8,13 @@ service, which gates on the `AI-REVIEW` label and runs the same review core.
## Architecture
```
PR opened/pushed/labeled "AI-REVIEW" (any repo under a covered owner)
PR opened/pushed/labeled/edited/… (any repo under a covered owner)
│ Gitea user-level webhook (events: pull_request)
Service pragent-webhook.pragent.svc.cluster.local (ClusterIP, ns pragent)
│ HMAC-verify (X-Gitea-Signature) → gate: action in {opened,
│ reopened, synchronize/synchronized, labeled/label_updated}
│ HMAC-verify (X-Gitea-Signature) → gate: action ≠ closed
│ AND pull_request.labels ∋ AI-REVIEW
│ (report_usage ← pull_request.labels ∋ AI-USAGE, optional)
ai_review.review_pr() (same core the CI-step uses)
1. fetch existing reviews → dedupe: skip if a review already carries
@@ -60,6 +60,50 @@ No workflow file, no repo secret, no act-runner needed. (The owner must already
be covered by a user-level webhook — see below. If not, do the one-time
per-owner setup first.)
## AI-USAGE label — token-usage reporting (optional, opt-in)
A review always fires on `AI-REVIEW`. Adding a second label **`AI-USAGE`** on
the same PR opts the review into appending a token-usage report:
- a `## 🔋 AI usage` section on the review summary body with the **measured**
review total — input / output / reasoning / cache read+write / total tokens,
agent step count, wall-clock duration, estimated cost, the model, and a scope
note (the agent reviews a whole-repo checkout at the head sha, so input
tokens include files read beyond the diff);
- a per-finding attribution table (severity · location · ≈out tok · %);
- a `🪙 ~N tok (X% · attributed output)` line at the foot of each inline
comment.
**Attributed, not measured.** One opencode agent pass produces *all* findings,
so there is no native per-finding token metering. The per-comment / per-row
counts are the review's measured **output** tokens split by each finding's
rendered-body weight (`len(problem)+len(fix)+len(suggestion)`) — an honest
attribution, labelled as such. The totals are real measurements summed from
opencode's `step_finish` events.
`PRAGENT_USAGE_ALWAYS=1` on the Deployment forces usage reporting on for every
review (testing / a future default-on) regardless of the label.
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.
## Webhook fires on any PR update (except `closed`)
The receiver uses a **denylist**, not an allowlist: it reviews on every
`pull_request` action **except `closed`**`opened`, `reopened`,
`synchronize`/`synchronized`, `labeled`/`label_updated`, `edited` (title/body),
`ready_for_review` (draft→ready), `assigned`, `review_requested`, `milestone`,
… . This is safe because of two downstream gates:
- the **AI-REVIEW label gate** — payload `labels` reflect current state, so an
`unlabeled` that *removed* AI-REVIEW fails the gate (no review); an
`unlabeled` of another label still passes;
- 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
actions are ones that change the head sha (`synchronize`, already covered) or
move a draft to ready (`ready_for_review`) on an un-reviewed sha.
## Repo-local focus: `.pr-review.json` (optional)
Drop a `.pr-review.json` at the repo root (committed on the PR's branch, or on
@@ -160,13 +204,17 @@ cramped model call. `pilot/opencode_review.py` is the glue:
`.pr-review.json`, prior reviews, sha, anchor hint).
3. `drop_factory` — copies `opencode.json` + `.opencode/` (agents/skills/commands)
into the workdir as the project config.
4. `run_opencode` — `opencode run --pure --agent pragent --dir <workdir>
--model headroom/glm-5.2:cloud` headlessly; returns the agent's stdout.
4. `run_opencode` — `opencode run --pure --format json --agent pragent
--dir <workdir> --model headroom/glm-5.2:cloud` headlessly. `--format json`
emits NDJSON events: `parse_opencode_events` reconstructs the assistant text
from `text` events and sums tokens/cost/steps from every `step_finish` event.
Returns `(text, usage)`.
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
all v2 logic (dedupe marker, anchor validation, ```suggestion fencing, posting)
is reused and never depends on the model remembering it.
all v2 logic (dedupe marker, anchor validation, language-tagged suggestion
fencing, posting, optional AI-USAGE attribution) is reused and never depends on
the model remembering it.
The factory lives in the pragent repo root: `opencode.json` (provider/model/
permission) + `.opencode/` (agents, skills, commands). It is **both** the
+113 -5
View File
@@ -134,14 +134,16 @@ def parse_text_blocks(content: list) -> str:
return "\n".join(out).strip()
def format_review_body(findings: str, model: str, sha: str, summary: str = "") -> str:
def format_review_body(findings: str, model: str, sha: str, summary: str = "", usage_section: str = "") -> str:
"""Format the posted review summary body.
`findings` is the bullet text for findings that could NOT be anchored inline
(or, on the legacy/no-inline path, the whole review). Empty -> "No issues
found.". `summary` (optional, opencode engine) is rendered as a "Summary"
section right under the header. The hidden sha marker is always appended
for the dedupe pass.
section right under the header. `usage_section` (optional, shown only when
the PR carries the `AI-USAGE` label) is rendered between the summary and the
findings bullets. The hidden sha marker is always appended for the dedupe
pass.
"""
header = REVIEW_HEADER.format(model=model, sha=sha[:8] if sha else "unknown")
findings = (findings or "").strip()
@@ -151,6 +153,8 @@ def format_review_body(findings: str, model: str, sha: str, summary: str = "") -
parts = [header]
if summary:
parts.append(summary.strip())
if usage_section:
parts.append(usage_section.strip())
parts.append(findings)
body = "\n\n".join(parts)
if marker:
@@ -158,6 +162,88 @@ def format_review_body(findings: str, model: str, sha: str, summary: str = "") -
return body
def _finding_weight(f: dict) -> int:
"""Body-weight used to attribute output tokens to a finding (char length of
its rendered problem + fix + suggestion). One model pass produces all
findings, so per-finding tokens can't be measured directly — we split the
measured output total by this weight as an honest attribution."""
return (
len(f.get("problem") or "")
+ len(f.get("fix") or "")
+ len(f.get("suggestion") or "")
)
def compute_attribution(findings: list[dict], output_tokens: int) -> None:
"""Stash `_tok_attrib` (attributed output tokens) and `_tok_pct` (0..1) on
each finding, splitting `output_tokens` by each finding's body weight.
Mutates in place. No-op when there are no findings or no output budget."""
if not findings or not output_tokens:
return
weights = [_finding_weight(f) for f in findings]
total_w = sum(weights)
if total_w <= 0:
# All-zero weights (no prose): split evenly.
share = output_tokens / len(findings)
for f in findings:
f["_tok_attrib"] = int(round(share))
f["_tok_pct"] = 1.0 / len(findings)
return
for f, w in zip(findings, weights):
f["_tok_attrib"] = int(round(output_tokens * w / total_w))
Review

[LOW] int(round(output_tokens * w / total_w)) per finding can make the per-finding ≈out tok table column sum to a value ≠ the stated measured output total (off by 1–N), so the table is internally inconsistent with the headline output figure.

Fix: Compute all but the last finding's attribution by rounding, then set the last finding's to output_tokens - sum(others) so the column always sums to the measured total.

for f, w in zip(findings[:-1], weights[:-1]):
        f["_tok_attrib"] = int(round(output_tokens * w / total_w))
        f["_tok_pct"] = w / total_w
    last_w = weights[-1]
    findings[-1]["_tok_attrib"] = output_tokens - sum(f["_tok_attrib"] for f in findings[:-1])
    findings[-1]["_tok_pct"] = last_w / total_w

🪙 ~2603 tok (45% · attributed output)

**[LOW]** `int(round(output_tokens * w / total_w))` per finding can make the per-finding `≈out tok` table column sum to a value ≠ the stated measured output total (off by 1–N), so the table is internally inconsistent with the headline output figure. Fix: Compute all but the last finding's attribution by rounding, then set the last finding's to `output_tokens - sum(others)` so the column always sums to the measured total. ```python for f, w in zip(findings[:-1], weights[:-1]): f["_tok_attrib"] = int(round(output_tokens * w / total_w)) f["_tok_pct"] = w / total_w last_w = weights[-1] findings[-1]["_tok_attrib"] = output_tokens - sum(f["_tok_attrib"] for f in findings[:-1]) findings[-1]["_tok_pct"] = last_w / total_w ``` 🪙 ~2603 tok (45% · attributed output)
f["_tok_pct"] = w / total_w
def format_usage_section(usage: dict | None, findings: list[dict], model: str) -> str:
"""Render the `## 🔋 AI usage` block for the review body.
Only called when the PR carries the `AI-USAGE` label (and the opencode
engine produced a usage dict). Reports the MEASURED total
(input/output/reasoning/cache/cost/steps/duration) plus an ATTRIBUTED
per-finding table — one model pass generates all findings, so per-comment
counts are an estimate (output split by body weight), clearly labelled.
Returns "" if `usage` is None.
"""
if not usage:
return ""
dur = usage.get("duration_s")
dur_s = f"{dur}s" if dur is not None else "?"
cost = usage.get("cost") or 0.0
cost_s = f"${cost:.4f}" if cost else "$0.00"
cost_note = (
Review

[LOW] cost_note hardcodes "(on-network glm-5.2:cloud via headroom — no per-token charge)" keyed off not cost rather than the actual provider/model, so it mislabels any non-headroom model, and also mislabels a billed provider whose run cost $0.0000 (e.g. cached tokens) as "no per-token charge".

Fix: Drop the provider-specific note, or derive it from the model/provider config instead of the cost value; at minimum key it on the model name passed in, not on not cost.

cost_note = (
    f"(billed by provider: {model})" if cost else "(no per-token charge reported)"
)

🪙 ~5330 tok (35% · attributed output)

**[LOW]** cost_note hardcodes "(on-network glm-5.2:cloud via headroom — no per-token charge)" keyed off `not cost` rather than the actual provider/model, so it mislabels any non-headroom model, and also mislabels a billed provider whose run cost $0.0000 (e.g. cached tokens) as "no per-token charge". Fix: Drop the provider-specific note, or derive it from the model/provider config instead of the cost value; at minimum key it on the model name passed in, not on `not cost`. ```python cost_note = ( f"(billed by provider: {model})" if cost else "(no per-token charge reported)" ) ``` 🪙 ~5330 tok (35% · attributed output)
"(on-network glm-5.2:cloud via headroom — no per-token charge)"
if not cost else "(billed by provider)"
)
lines = [
"## 🔋 AI usage",
"",
f"- model: `{model}` · engine: opencode · agent steps: {usage.get('steps', 0)} · duration: {dur_s}",
(
f"- tokens: {usage.get('input', 0)} in · {usage.get('output', 0)} out · "
f"{usage.get('reasoning', 0)} reasoning · cache "
f"{usage.get('cache_read', 0)} read / {usage.get('cache_write', 0)} write "
f"{usage.get('total', 0)} total"
),
f"- est. cost: {cost_s} {cost_note}",
"- scope: whole-repo checkout at head sha (agent can read any file + run linters, not just the diff) — input tokens include files read beyond the diff",
"- per-comment tokens are *attributed* (one model pass produces all findings; output split by each finding's body weight)",
]
# Per-finding attribution table.
rows = [f for f in findings if f.get("_tok_attrib") is not None]
if rows:
lines.append("")
lines.append("| severity | location | ≈out tok | % |")
lines.append("|---|---|---:|---:|")
for f in rows:
loc = f"{f['path']}:{f['line']}" if f.get("line") else f.get("path", "?")
pct = f.get("_tok_pct", 0.0) * 100
lines.append(
f"| {f.get('severity', '').upper()} | `{loc}` | "
f"{f.get('_tok_attrib', 0)} | {pct:.0f}% |"
)
return "\n".join(lines)
def build_user_prompt(
title: str,
body: str,
@@ -483,6 +569,10 @@ def inline_comment_body(f: dict) -> str:
ref = f.get("reference", "")
if ref:
body += f"\n\n📎 ref: {ref}"
tok = f.get("_tok_attrib")
if tok is not None:
pct = (f.get("_tok_pct", 0.0) or 0.0) * 100
body += f"\n\n🪙 ~{tok} tok ({pct:.0f}% · attributed output)"
return body
@@ -713,6 +803,7 @@ def review_pr(
model: str,
max_tokens: int = 8000,
max_chars: int = 150000,
report_usage: bool = False,
) -> bool:
"""Run one review and post it as `pragent-bot`.
@@ -722,6 +813,11 @@ def review_pr(
review with inline comments + suggestions (unanchored findings → summary
bullets).
`report_usage`: when True (PR carries the `AI-USAGE` label), the opencode
engine's measured token/cost usage is rendered as a `## 🔋 AI usage` section
on the review body and an attributed `🪙 ~N tok` line on each inline
comment. No-op on the ollama fallback (no usage available).
Returns True on success (including a deliberate skip), False on failure
(failure note posted when possible). Never raises — fail-open by design.
Both the CI `run()` entry point and the central webhook server call this.
@@ -752,7 +848,7 @@ def review_pr(
# `model` here is the bare id (OLLAMA_MODEL). OPENCODE_MODEL overrides
# with the full ref; otherwise we prefix the configured provider.
oc_model = os.environ.get("OPENCODE_MODEL") or f"headroom/{model}"
stdout = opencode_review.run(
stdout, usage = opencode_review.run(
api=api, repo=repo, index=index, sha=sha, token=token,
title=title, body=body, diff=diff, config=config,
prior_reviews=prior, model=oc_model,
@@ -767,6 +863,15 @@ def review_pr(
user_prompt = build_user_prompt(title, body, diff, config, prior)
raw_findings = call_model(ollama_url, model, SYSTEM_PROMPT, user_prompt, max_tokens)
findings = parse_findings(raw_findings)
usage = None
# Attribute output tokens to each finding (mutates finding dicts) so
# inline comments + the usage table can show a per-comment estimate.
# Only meaningful when we have measured usage AND the PR asked for it.
usage_section = ""
if report_usage and usage and usage.get("output"):
compute_attribution(findings, usage["output"])
usage_section = format_usage_section(usage, findings, model)
anchors = parse_diff_anchors(diff)
anchored, unanchored = split_findings(findings, anchors)
@@ -782,7 +887,10 @@ def review_pr(
summary_parts.append(bullets)
if not summary_parts:
summary_parts.append("No issues found.")
summary_body = format_review_body("\n\n".join(summary_parts), model, sha, summary=review_summary)
summary_body = format_review_body(
"\n\n".join(summary_parts), model, sha,
summary=review_summary, usage_section=usage_section,
)
post_inline_review(api, repo, index, token, summary_body, anchored)
print(
+132 -15
View File
@@ -34,10 +34,12 @@ Env:
import io
import json
import os
import re
import shutil
import subprocess
import tarfile
import tempfile
import time
import urllib.error
import urllib.request
@@ -143,6 +145,34 @@ def _extract_tar_strip_one(blob: bytes, dest: str) -> None:
BRIEF_PATH = ".pragent/brief.md"
# Matches unified-diff new-file path headers: `+++ b/path` (and `+++ /dev/null`
# for deletions, which we skip). Captures the path after the `b/` prefix.
_NEW_FILE_HEADER_RE = re.compile(r"^\+\+\+ b/(.+?)\s*$")
Review

[LOW] changed_files() scans every line of the raw diff for +++ b/ without distinguishing file headers from hunk body lines, so an added diff line whose content begins with +++ b/ (e.g. a patch that itself contains diff text) is falsely extracted as a changed file and injected into the brief's focus list.

Fix: Only treat a +++ b/ line as a header when it is a real file header — i.e. not preceded by a hunk + content marker. Track whether a hunk is open and skip matches inside hunk bodies, or require the line to follow a diff --git header.

out = []
    seen = set()
    in_hunk = False
    for line in (diff or "").splitlines():
        if line.startswith("@@"):
            in_hunk = True
            continue
        if line.startswith("diff --git"):
            in_hunk = False
        if in_hunk:
            continue
        if not line.startswith("+++ b/"):
            continue
        m = _NEW_FILE_HEADER_RE.match(line)
        if not m:
            continue
        path = m.group(1).strip()
        if path and path not in seen:
            seen.add(path)
            out.append(path)
    return sorted(out)
**[LOW]** changed_files() scans every line of the raw diff for `+++ b/` without distinguishing file headers from hunk body lines, so an added diff line whose content begins with `+++ b/` (e.g. a patch that itself contains diff text) is falsely extracted as a changed file and injected into the brief's focus list. Fix: Only treat a `+++ b/` line as a header when it is a real file header — i.e. not preceded by a hunk `+` content marker. Track whether a hunk is open and skip matches inside hunk bodies, or require the line to follow a `diff --git` header. ```python out = [] seen = set() in_hunk = False for line in (diff or "").splitlines(): if line.startswith("@@"): in_hunk = True continue if line.startswith("diff --git"): in_hunk = False if in_hunk: continue if not line.startswith("+++ b/"): continue m = _NEW_FILE_HEADER_RE.match(line) if not m: continue path = m.group(1).strip() if path and path not in seen: seen.add(path) out.append(path) return sorted(out) ```
def changed_files(diff: str) -> list[str]:
"""Extract the sorted list of changed file paths from a unified diff.
Pulled from `+++ b/<path>` headers (the post-change side). Deletions
(`+++ /dev/null`) are excluded. Used to give the agent a clean focus list
for context research, so it reads callers/imports of the actually-changed
files instead of re-deriving them from the raw diff.
"""
out = []
seen = set()
for line in (diff or "").splitlines():
if not line.startswith("+++ b/"):
continue
m = _NEW_FILE_HEADER_RE.match(line)
if not m:
continue
path = m.group(1).strip()
if path and path not in seen:
seen.add(path)
out.append(path)
return sorted(out)
_BRIEF_TEMPLATE = """\
# pragent review brief
@@ -156,6 +186,14 @@ _BRIEF_TEMPLATE = """\
## Description
{description}
## Changed files (focus your context research here)
{changed_files}
For each changed file, read its callers, imports, sibling functions, and type
definitions so findings reflect how the change is actually used — don't flag a
hunk in isolation. Stop once a finding is grounded (13 related files per
finding; avoid runaway whole-repo walks).
## Repo review config (.pr-review.json)
{config}
@@ -198,12 +236,15 @@ def write_brief(
prior = "\n\n---\n\n".join(prior_reviews)
if len(prior) > 8000:
prior = prior[:8000] + "\n…[prior reviews truncated]"
files = changed_files(diff)
files_block = "\n".join(f"- `{p}`" for p in files) if files else "_(none)_"
content = _BRIEF_TEMPLATE.format(
repo=repo or "?",
index=index or "?",
sha=sha or "?",
title=title or "(none)",
description=description.strip() or "_(none)_",
changed_files=files_block,
config=cfg,
prior=prior,
diff=diff or "_(empty)_",
@@ -233,6 +274,66 @@ def drop_factory(workdir: str) -> None:
# opencode invocation
# ---------------------------------------------------------------------------
def _new_usage() -> dict:
return {
"input": 0, "output": 0, "reasoning": 0,
"cache_read": 0, "cache_write": 0, "total": 0,
"cost": 0.0, "steps": 0,
}
def parse_opencode_events(stdout: str) -> tuple[str, dict | None]:
"""Parse `opencode run --format json` NDJSON stdout into (text, usage).
- assistant text: concatenation of every `{"type":"text","part":{"text":…}}`
event, in order → the agent's full message (prose + the findings ```json
block). This is what `ai_review.parse_review_output` then extracts the
findings JSON from.
- usage: summed across every `{"type":"step_finish","part":{"tokens":…,
"cost":…}}` event (one per model turn). Returns a dict with input/output/
reasoning/cache_read/cache_write/total/cost/steps, or None if no
step_finish was seen (e.g. empty/failed run).
Tolerant: non-JSON lines, missing fields, or non-dict events are skipped
(warm-up / log noise / tool events we don't care about). Never raises.
"""
text_parts: list[str] = []
usage = _new_usage()
saw_step = False
for line in (stdout or "").splitlines():
line = line.strip()
if not line or not line.startswith("{"):
continue
try:
ev = json.loads(line)
except json.JSONDecodeError:
continue
if not isinstance(ev, dict):
continue
etype = ev.get("type")
part = ev.get("part") or {}
if etype == "text" and isinstance(part, dict):
t = part.get("text")
if isinstance(t, str):
text_parts.append(t)
elif etype == "step_finish" and isinstance(part, dict):
tok = part.get("tokens") or {}
if isinstance(tok, dict):
saw_step = True
usage["steps"] += 1
usage["input"] += int(tok.get("input") or 0)
Review

[MEDIUM] int(tok.get("input") or 0) raises ValueError when a token field is a non-int string (e.g. "90.5"), violating the docstring's "Never raises" promise; run_opencode trusts that contract and a single bad event line aborts the whole parse, losing the assistant text.

Fix: Wrap each token coercion in a try/except (or use a safe _to_int helper that falls back to 0 on TypeError/ValueError) so malformed values are skipped, not fatal.

def _to_int(v) -> int:
    try:
        return int(v)
    except (TypeError, ValueError):
        return 0

🪙 ~5043 tok (33% · attributed output)

**[MEDIUM]** int(tok.get("input") or 0) raises ValueError when a token field is a non-int string (e.g. "90.5"), violating the docstring's "Never raises" promise; run_opencode trusts that contract and a single bad event line aborts the whole parse, losing the assistant text. Fix: Wrap each token coercion in a try/except (or use a safe _to_int helper that falls back to 0 on TypeError/ValueError) so malformed values are skipped, not fatal. ```python def _to_int(v) -> int: try: return int(v) except (TypeError, ValueError): return 0 ``` 🪙 ~5043 tok (33% · attributed output)
usage["output"] += int(tok.get("output") or 0)
usage["reasoning"] += int(tok.get("reasoning") or 0)
cache = tok.get("cache") or {}
if isinstance(cache, dict):
usage["cache_read"] += int(cache.get("read") or 0)
usage["cache_write"] += int(cache.get("write") or 0)
usage["total"] += int(tok.get("total") or 0)
Review

[LOW] usage["total"] is only summed from an explicit total field in the event; if opencode ever emits a step_finish with input/output but no total, the usage section reports 0 total alongside non-zero in/out figures, which is misleading.

Fix: Fall back to input+output+reasoning when total is absent, or derive total as the sum of the components when the field is missing.

tot = tok.get("total")
                if tok.get("total") is None:
                    tok_total = usage["input"] + usage["output"] + usage["reasoning"]
                else:
                    tok_total = int(tok_total or 0)
                usage["total"] += tok_total
**[LOW]** usage["total"] is only summed from an explicit `total` field in the event; if opencode ever emits a step_finish with input/output but no `total`, the usage section reports `0 total` alongside non-zero in/out figures, which is misleading. Fix: Fall back to input+output+reasoning when `total` is absent, or derive total as the sum of the components when the field is missing. ```python tot = tok.get("total") if tok.get("total") is None: tok_total = usage["input"] + usage["output"] + usage["reasoning"] else: tok_total = int(tok_total or 0) usage["total"] += tok_total ```
cost = part.get("cost")
if isinstance(cost, (int, float)):
usage["cost"] += float(cost)
return "".join(text_parts), (usage if saw_step else None)
_PROMPT = (
"Read .pragent/brief.md and review this pull request as pragent. "
"Load the review-methodology and findings-schema skills, inspect the "
@@ -333,8 +434,12 @@ def _warm_opencode(home: str, model: str) -> None:
pass
def run_opencode(workdir: str, model: str, timeout: int | None = None) -> str:
"""Run the pragent agent headlessly in `workdir`. Returns the agent's stdout.
def run_opencode(workdir: str, model: str, timeout: int | None = None) -> tuple[str, dict | None]:
"""Run the pragent agent headlessly in `workdir`. Returns `(text, usage)`.
`text` is the reconstructed assistant message (prose + findings JSON) from
the `--format json` event stream; `usage` is the summed token/cost usage
across all model turns (or None if no step_finish event was seen).
Isolates from the host user's global opencode config by pointing HOME at a
shared temp dir (so ~/.config/opencode is not merged) and passing --pure
@@ -342,7 +447,11 @@ def run_opencode(workdir: str, model: str, timeout: int | None = None) -> str:
drop_factory) are the only project config discovered; the shared home's
global opencode.json supplies the provider/model/permission. PATH prepends
the rtk dir so the agent's bash tool can call `rtk`. Warms the HOME first
(cold runs produce no output) and retries once on empty stdout.
(cold runs produce no output) and retries once on empty text.
`--format json` makes opencode emit NDJSON events (text + step_finish with
token usage) instead of formatted stdout — `parse_opencode_events` turns
that into the assistant text + a usage dict.
stdin=DEVNULL is critical: opencode blocks on stdin (permission prompt /
interactive input) when run headlessly via subprocess, hanging until timeout.
@@ -356,6 +465,7 @@ def run_opencode(workdir: str, model: str, timeout: int | None = None) -> str:
bin_,
"run",
"--pure",
"--format", "json",
"--agent", "pragent",
"--dir", workdir,
"--model", model,
@@ -371,10 +481,13 @@ def run_opencode(workdir: str, model: str, timeout: int | None = None) -> str:
except subprocess.TimeoutExpired as e:
last_err = f"opencode timed out after {e.timeout}s"
continue
out = (proc.stdout or "").strip()
if out:
return proc.stdout
last_err = f"opencode empty stdout (rc={proc.returncode}); stderr: {(proc.stderr or '')[-1500:]}"
text, usage = parse_opencode_events(proc.stdout or "")
if text.strip():
return text, usage
last_err = (
f"opencode empty text (rc={proc.returncode}); "
f"stderr: {(proc.stderr or '')[-1500:]}"
)
raise RuntimeError(last_err or "opencode produced no output")
@@ -396,16 +509,18 @@ def run(
config: dict | None,
prior_reviews: list[str] | None,
model: str,
) -> str:
"""End-to-end: checkout archive → brief → drop factory → opencode → stdout.
) -> tuple[str, dict | None]:
"""End-to-end: checkout archive → brief → drop factory → opencode → (text, usage).
Returns the raw opencode stdout (summary + findings JSON). Raises on any
failure; the caller (review_pr) fails open. The workdir is removed unless
PRAGENT_KEEP_WORK is set.
Returns the reconstructed opencode assistant text (summary + findings JSON)
and a usage dict (token/cost totals + `duration_s`), or `(text, None)` when
no usage events were seen. Raises on any failure; the caller (`review_pr`)
fails open. The workdir is removed unless PRAGENT_KEEP_WORK is set.
"""
os.makedirs(WORK_ROOT, exist_ok=True)
workdir = tempfile.mkdtemp(prefix=f"{repo.replace('/', '_')}-{sha[:8]}-", dir=WORK_ROOT)
keep = bool(os.environ.get("PRAGENT_KEEP_WORK"))
t0 = time.monotonic()
try:
fetch_archive(api, repo, sha, token, workdir)
write_brief(
@@ -414,10 +529,12 @@ def run(
diff=diff, config=config, prior_reviews=prior_reviews,
)
drop_factory(workdir)
stdout = run_opencode(workdir, model)
if not stdout.strip():
text, usage = run_opencode(workdir, model)
if not text.strip():
raise RuntimeError("opencode produced no output")
return stdout
if usage is not None:
usage["duration_s"] = round(time.monotonic() - t0, 1)
return text, usage
finally:
if not keep:
shutil.rmtree(workdir, ignore_errors=True)
+34 -15
View File
@@ -37,13 +37,17 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from ai_review import review_pr
# Pull-request webhook `action` values worth reviewing on. Gitea emits
# GitHub-style payload `action` names (`labeled`, `synchronize`) even though the
# `X-Gitea-Event-Type` header uses `label_updated` / `synchronized` — accept both
# so the gate is robust to either. The label gate below means a non-AI-REVIEW
# label update is a no-op.
REVIEW_ACTIONS = {"opened", "reopened", "synchronize", "synchronized", "labeled", "label_updated"}
# 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
# gate + sha dedupe downstream make broadening safe: a same-sha re-fire (title
# edit, assignee, milestone, label toggle of another label…) is skipped by
# `review_pr`'s dedupe, and an `unlabeled` event that removed AI-REVIEW fails
# the label gate (payload `labels` reflect current state). Gitea emits
# GitHub-style `action` names (`labeled`, `synchronize`) even though the
# `X-Gitea-Event-Type` header uses `label_updated` / `synchronized`.
SKIP_ACTIONS = {"closed"}
Review

[HIGH] No tests cover the changed webhook gate logic: the _labels_have refactor, SKIP_ACTIONS denylist, AI_USAGE_LABEL detection, and report_usage plumbing are all new/changed with zero test coverage (no webhook tests exist in the repo). Gate logic is exactly what needs tests.

Fix: Add tests/pilot/test_webhook_server.py covering _labels_have (dict + bare-string labels), the SKIP_ACTIONS gate (closed skipped, other actions pass), AI-USAGE label detection, and report_usage env-override + label detection.

🪙 ~4727 tok (31% · attributed output)

**[HIGH]** No tests cover the changed webhook gate logic: the _labels_have refactor, SKIP_ACTIONS denylist, AI_USAGE_LABEL detection, and report_usage plumbing are all new/changed with zero test coverage (no webhook tests exist in the repo). Gate logic is exactly what needs tests. Fix: Add tests/pilot/test_webhook_server.py covering _labels_have (dict + bare-string labels), the SKIP_ACTIONS gate (closed skipped, other actions pass), AI-USAGE label detection, and report_usage env-override + label detection. 🪙 ~4727 tok (31% · attributed output)
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")
BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "")
@@ -55,17 +59,23 @@ WEBHOOK_SECRET = os.environ.get("WEBHOOK_SECRET", "").encode()
PORT = int(os.environ.get("WEBHOOK_PORT", "8080"))
def _labels_have_ai_review(labels) -> bool:
def _labels_have(labels, name: str) -> bool:
"""True if the Gitea PR `labels` list (dicts with `name`, or bare strings)
contains `name`."""
if not isinstance(labels, list):
return False
for lab in labels:
if isinstance(lab, dict) and lab.get("name") == AI_REVIEW_LABEL:
if isinstance(lab, dict) and lab.get("name") == name:
return True
if isinstance(lab, str) and lab == AI_REVIEW_LABEL:
if isinstance(lab, str) and lab == name:
return True
return False
def _labels_have_ai_review(labels) -> bool:
return _labels_have(labels, AI_REVIEW_LABEL)
def _verify_signature(raw_body: bytes, headers) -> bool:
if not WEBHOOK_SECRET:
return False # refuse to run without a configured secret
@@ -87,12 +97,13 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
repo_obj = payload.get("repository") or {}
repo = repo_obj.get("full_name") or ""
if action not in REVIEW_ACTIONS:
if action in SKIP_ACTIONS:
Review

[MEDIUM] The allowlist→denylist broadening (fire on every action except closed) is the riskiest behavioral change in the PR but has zero test coverage — a regression here could fire on merged PRs or cause review storms, and the safety argument (sha-dedupe + label gate) is never verified by a test.

Fix: Add tests for _handle_pull_request: closed is skipped, a non-closed action with AI-REVIEW label proceeds, and an unlabeled payload whose labels no longer contain AI-REVIEW is skipped.

🪙 ~1733 tok (30% · attributed output)

**[MEDIUM]** The allowlist→denylist broadening (fire on every action except `closed`) is the riskiest behavioral change in the PR but has zero test coverage — a regression here could fire on merged PRs or cause review storms, and the safety argument (sha-dedupe + label gate) is never verified by a test. Fix: Add tests for `_handle_pull_request`: `closed` is skipped, a non-`closed` action with AI-REVIEW label proceeds, and an `unlabeled` payload whose labels no longer contain AI-REVIEW is skipped. 🪙 ~1733 tok (30% · attributed output)
return 200, f"ignore action={action}"
if not repo:
return 400, "no repository.full_name"
if not _labels_have_ai_review(pr.get("labels")):
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")
@@ -106,15 +117,22 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
if not BOT_TOKEN:
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")
)
threading.Thread(
target=_run_review,
args=(repo, str(index), title, body, sha),
args=(repo, str(index), title, body, sha, report_usage),
daemon=True,
).start()
return 202, f"reviewing {repo}#{index} action={action} sha={sha[:8]}"
return 202, f"reviewing {repo}#{index} action={action} sha={sha[:8]} usage={report_usage}"
def _run_review(repo: str, index: str, title: str, body: str, sha: str) -> None:
def _run_review(repo: str, index: str, title: str, body: str, sha: str, report_usage: bool) -> None:
try:
ok = review_pr(
api=GITEA_API,
@@ -128,8 +146,9 @@ def _run_review(repo: str, index: str, title: str, body: str, sha: str) -> None:
model=OLLAMA_MODEL,
max_tokens=OLLAMA_MAX_TOKENS,
max_chars=DIFF_MAX_CHARS,
report_usage=report_usage,
)
print(f"pragent-webhook: reviewed {repo}#{index} sha={sha[:8]} ok={ok}", flush=True)
print(f"pragent-webhook: reviewed {repo}#{index} sha={sha[:8]} ok={ok} usage={report_usage}", flush=True)
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)
+112
View File
@@ -9,7 +9,9 @@ sys.path.insert(0, os.path.join(ROOT, "pilot"))
from ai_review import ( # noqa: E402
build_user_prompt,
compute_attribution,
format_review_body,
format_usage_section,
inline_comment_body,
parse_diff_anchors,
parse_findings,
@@ -437,3 +439,113 @@ def test_format_review_body_with_summary_section():
assert "<!-- pragent:sha=abcdef1234567890 -->" in body
# summary appears before the findings bullets
assert body.index("risky helper") < body.index("[high]")
# ---------------------------------------------------------------------------
# AI-USAGE: compute_attribution + format_usage_section + inline 🪙 line
# ---------------------------------------------------------------------------
def test_compute_attribution_weighted_split():
# weights 1 (problem="a") and 3 (problem="aaa"), output 100 → 25 / 75
fs = [
{"severity": "high", "path": "x", "line": 1, "problem": "a", "fix": "", "suggestion": ""},
{"severity": "low", "path": "x", "line": 2, "problem": "aaa", "fix": "", "suggestion": ""},
]
compute_attribution(fs, 100)
assert fs[0]["_tok_attrib"] == 25
assert fs[1]["_tok_attrib"] == 75
assert abs(fs[0]["_tok_pct"] - 0.25) < 1e-9
assert abs(fs[1]["_tok_pct"] - 0.75) < 1e-9
def test_compute_attribution_zero_weights_splits_evenly():
fs = [
{"severity": "high", "path": "x", "line": 1, "problem": "", "fix": "", "suggestion": ""},
{"severity": "low", "path": "x", "line": 2, "problem": "", "fix": "", "suggestion": ""},
]
compute_attribution(fs, 80)
assert fs[0]["_tok_attrib"] == 40
assert fs[1]["_tok_attrib"] == 40
assert abs(fs[0]["_tok_pct"] - 0.5) < 1e-9
def test_compute_attribution_noop_on_empty_or_zero_budget():
fs = [{"severity": "high", "path": "x", "line": 1, "problem": "a", "fix": "", "suggestion": ""}]
compute_attribution([], 100)
compute_attribution(fs, 0)
assert "_tok_attrib" not in fs[0]
def test_inline_comment_body_with_attribution_line():
f = {"severity": "high", "path": "a", "line": 1, "problem": "bad", "fix": "swap",
"suggestion": "", "_tok_attrib": 180, "_tok_pct": 0.29}
body = inline_comment_body(f)
assert "🪙 ~180 tok" in body
assert "29%" in body
assert "attributed output" in body
def test_inline_comment_body_no_attribution_no_coin_line():
f = {"severity": "high", "path": "a", "line": 1, "problem": "bad", "fix": "swap",
"suggestion": ""}
assert "🪙" not in inline_comment_body(f)
def test_format_usage_section_renders_totals_and_table():
fs = [
{"severity": "critical", "path": "src/Foo.java", "line": 98,
"problem": "p"*10, "fix": "f", "suggestion": "", "_tok_attrib": 180, "_tok_pct": 0.29},
]
usage = {"input": 18420, "output": 612, "reasoning": 0, "cache_read": 15210,
"cache_write": 0, "total": 19032, "cost": 0.0, "steps": 7, "duration_s": 142.0}
sec = format_usage_section(usage, fs, "glm-5.2:cloud")
assert "## 🔋 AI usage" in sec
assert "`glm-5.2:cloud`" in sec
assert "agent steps: 7" in sec
assert "duration: 142.0s" in sec
assert "18420 in" in sec and "612 out" in sec and "19032 total" in sec
assert "$0.00" in sec
assert "whole-repo checkout" in sec
assert "attributed" in sec
# table
assert "| severity | location | ≈out tok | % |" in sec
assert "CRITICAL" in sec
assert "`src/Foo.java:98`" in sec
assert "180" in sec and "29%" in sec
def test_format_usage_section_omits_table_when_no_attributed_rows():
usage = {"input": 10, "output": 0, "reasoning": 0, "cache_read": 0,
"cache_write": 0, "total": 10, "cost": 0.0, "steps": 1, "duration_s": 1.0}
sec = format_usage_section(usage, [], "glm-5.2:cloud")
assert "## 🔋 AI usage" in sec
assert "severity | location" not in sec # no rows → no table
def test_format_usage_section_none_returns_empty():
assert format_usage_section(None, [], "m") == ""
def test_format_usage_section_cost_nonzero():
usage = {"input": 10, "output": 0, "reasoning": 0, "cache_read": 0,
"cache_write": 0, "total": 10, "cost": 0.0123, "steps": 1, "duration_s": 1.0}
sec = format_usage_section(usage, [], "m")
assert "$0.0123" in sec
assert "billed by provider" in sec
def test_format_review_body_usage_section_between_summary_and_findings():
usage_sec = "## 🔋 AI usage\n\n- model: `m`"
body = format_review_body("- [high] x:1 — b", "glm-5.2:cloud", "abcdef1234567890",
summary="This PR is risky.", usage_section=usage_sec)
# order: header < summary < usage < findings < marker
assert body.index("risky.") < body.index("AI usage")
assert body.index("AI usage") < body.index("[high]")
assert body.index("[high]") < body.index("<!-- pragent:sha=")
assert "## 🔋 AI usage" in body
def test_format_review_body_no_usage_section_omitted():
body = format_review_body("- [high] x:1 — b", "glm-5.2:cloud", "abcdef1234567890")
assert "AI usage" not in body
+101
View File
@@ -124,3 +124,104 @@ def test_drop_factory_copies_config_and_agents(tmp_path):
assert os.path.isfile(tmp_path / "opencode.json")
assert os.path.isfile(tmp_path / ".opencode" / "agents" / "pragent.md")
assert os.path.isfile(tmp_path / ".opencode" / "skills" / "findings-schema" / "SKILL.md")
# ---------------------------------------------------------------------------
# changed_files — extract changed paths from a unified diff
# ---------------------------------------------------------------------------
def test_changed_files_extracts_new_side_paths():
diff = (
"diff --git a/src/a.py b/src/a.py\n+++ b/src/a.py\n@@ -1 +1 @@\n-x\n+y\n"
"diff --git a/README.md b/README.md\n+++ b/README.md\n@@ -1 +1 @@\n+z\n"
)
assert oc.changed_files(diff) == ["README.md", "src/a.py"]
def test_changed_files_skips_deletions_and_dedups():
diff = (
"diff --git a/gone.txt b/gone.txt\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\n"
"diff --git a/dup.go b/dup.go\n+++ b/dup.go\n@@ -1 +1 @@\n+a\n"
"diff --git a/dup.go b/dup.go\n+++ b/dup.go\n@@ -1 +1 @@\n+b\n"
)
assert oc.changed_files(diff) == ["dup.go"]
def test_changed_files_empty():
assert oc.changed_files("") == []
assert oc.changed_files("no diff headers here") == []
def test_write_brief_lists_changed_files(tmp_path):
brief = oc.write_brief(
str(tmp_path), repo="o/r", index="1", sha="abcdef1234567890",
title="t", description="d",
diff="diff --git a/src/x.ts b/src/x.ts\n+++ b/src/x.ts\n@@ -1 +1 @@\n+x",
config=None, prior_reviews=None,
)
text = open(brief, encoding="utf-8").read()
assert "Changed files (focus your context research here)" in text
assert "`src/x.ts`" in text
# ---------------------------------------------------------------------------
# parse_opencode_events — NDJSON → (text, usage)
# ---------------------------------------------------------------------------
def _ev(obj):
import json
return json.dumps(obj)
def test_parse_events_text_and_usage_summed():
stdout = "\n".join([
_ev({"type": "step_start", "part": {}}),
_ev({"type": "text", "part": {"text": "Hello "}}),
_ev({"type": "text", "part": {"text": "world"}}),
_ev({"type": "step_finish", "part": {
"tokens": {"total": 100, "input": 90, "output": 10,
"reasoning": 0, "cache": {"write": 0, "read": 5}},
"cost": 0.0}}),
_ev({"type": "text", "part": {"text": " more"}}),
_ev({"type": "step_finish", "part": {
"tokens": {"total": 50, "input": 40, "output": 10,
"reasoning": 2, "cache": {"write": 1, "read": 0}},
"cost": 0.01}}),
])
text, usage = oc.parse_opencode_events(stdout)
assert text == "Hello world more"
assert usage is not None
assert usage["steps"] == 2
assert usage["input"] == 130
assert usage["output"] == 20
assert usage["reasoning"] == 2
assert usage["cache_read"] == 5
assert usage["cache_write"] == 1
assert usage["total"] == 150
assert abs(usage["cost"] - 0.01) < 1e-9
def test_parse_events_no_step_finish_returns_none_usage():
stdout = _ev({"type": "text", "part": {"text": "only text"}})
text, usage = oc.parse_opencode_events(stdout)
assert text == "only text"
assert usage is None
def test_parse_events_tolerates_noise_and_malformed():
stdout = "\n".join([
"not json at all",
_ev({"type": "text", "part": {"text": "ok"}}),
"{ broken json",
_ev({"type": "step_finish", "part": {}}), # no tokens field -> counted, zero
_ev({"type": "tool_start", "part": {"text": "ignored"}}),
" ",
])
text, usage = oc.parse_opencode_events(stdout)
assert text == "ok"
# step_finish with no tokens still counts as a step; usage dict returned
assert usage is not None
assert usage["steps"] == 1
assert usage["input"] == 0 and usage["output"] == 0