diff --git a/.opencode/agents/pragent.md b/.opencode/agents/pragent.md index ee1ec1d..7f0c1e0 100644 --- a/.opencode/agents/pragent.md +++ b/.opencode/agents/pragent.md @@ -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 (1–3 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 ` if configured. - Python: `ruff check ` or `python -m pyright ` / `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 diff --git a/.opencode/skills/review-methodology/SKILL.md b/.opencode/skills/review-methodology/SKILL.md index 655a34e..f3290af 100644 --- a/.opencode/skills/review-methodology/SKILL.md +++ b/.opencode/skills/review-methodology/SKILL.md @@ -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 — 1–3 related files per finding, no unbounded +whole-repo walks. + ## Honoring repo config If `.pr-review.json` is present, honor it: diff --git a/pilot/README-webhook.md b/pilot/README-webhook.md index b900a83..cfebbd1 100644 --- a/pilot/README-webhook.md +++ b/pilot/README-webhook.md @@ -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 - --model headroom/glm-5.2:cloud` headlessly; returns the agent's stdout. +4. `run_opencode` — `opencode run --pure --format json --agent pragent + --dir --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 diff --git a/pilot/ai_review.py b/pilot/ai_review.py index 81d2de1..c100f4f 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -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)) + 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 = ( + "(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( diff --git a/pilot/opencode_review.py b/pilot/opencode_review.py index 988fcf4..66abf4e 100644 --- a/pilot/opencode_review.py +++ b/pilot/opencode_review.py @@ -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*$") + + +def changed_files(diff: str) -> list[str]: + """Extract the sorted list of changed file paths from a unified diff. + + Pulled from `+++ b/` 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 (1–3 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) + 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) + 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) \ No newline at end of file diff --git a/pilot/webhook_server.py b/pilot/webhook_server.py index 148673c..58f7b9d 100644 --- a/pilot/webhook_server.py +++ b/pilot/webhook_server.py @@ -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"} 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: 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) diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index 47304c1..8e01283 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -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, @@ -436,4 +438,114 @@ def test_format_review_body_with_summary_section(): assert "- [high] x:1" in body assert "" in body # summary appears before the findings bullets - assert body.index("risky helper") < body.index("[high]") \ No newline at end of file + 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("