From 90cea84f6f2a7eaf632820b551d664785c4ed780 Mon Sep 17 00:00:00 2001 From: Marcos Date: Mon, 17 Aug 2026 19:59:36 +0000 Subject: [PATCH] pilot: dedupe + repo config + inline comments w/ suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Dedupe: Gitea-as-state. Scan existing reviews for a hidden marker matching the head sha; skip if present (kills duplicate reviews on label-toggle / re-fire). Prior review bodies fed back as 'already said' context (light framework §6.1). - Repo-local focus: optional .pr-review.json at repo root ({focus,exclude_paths,languages,instructions}), fetched at head ref. - Inline comments + apply-able suggestions: model emits JSON findings {severity,path,line,problem,fix,suggestion}; diff hunks parsed into valid (path,new_line) RIGHT-side anchors; anchored findings become positional review comments with a ```suggestion fence (Gitea apply-button); unanchored findings fold into the summary body. - Tests: parse_diff_anchors, parse_findings (tolerant JSON), split_findings, inline_comment_body, summary_bullets, parse_repo_config, reviewed_shas, prior_review_bodies, sha-marker. 35 pass. - Bump OLLAMA_MAX_TOKENS default 6000 -> 8000 (suggestions add length). Co-Authored-By: Claude --- pilot/README-webhook.md | 61 ++++- pilot/ai_review.py | 493 +++++++++++++++++++++++++++++++--- pilot/webhook_server.py | 13 +- tests/pilot/test_ai_review.py | 212 ++++++++++++++- 4 files changed, 734 insertions(+), 45 deletions(-) diff --git a/pilot/README-webhook.md b/pilot/README-webhook.md index 0acea40..1239067 100644 --- a/pilot/README-webhook.md +++ b/pilot/README-webhook.md @@ -17,13 +17,25 @@ Service pragent-webhook.pragent.svc.cluster.local (ClusterIP, ns pragent) │ AND pull_request.labels ∋ AI-REVIEW ▼ ai_review.review_pr() (same core the CI-step uses) - 1. fetch PR diff → GET gitea-http.gitea.svc:3000/api/v1/repos/{o}/{r}/pulls/{i}.diff - 2. review prompt → POST http://100.74.17.70:8789/v1/messages (glm-5.2:cloud) - 3. post review → POST .../pulls/{i}/reviews (event: COMMENT) as pragent-bot + 1. fetch existing reviews → dedupe: skip if a review already carries + (no duplicate on label-toggle / re-fire) + 2. fetch PR diff → GET .../pulls/{i}.diff + 3. fetch .pr-review.json @ head ref (optional repo-local focus/config) + 4. prior review bodies → fed as "already said" context (light §6.1) + 5. review prompt → POST http://100.74.17.70:8789/v1/messages (glm-5.2:cloud) + model emits JSON: {findings:[{severity,path,line,problem,fix,suggestion}]} + 6. parse diff hunks → valid (path, new_line) anchors (RIGHT side) + 7. post review → POST .../pulls/{i}/reviews (event: COMMENT) as pragent-bot + - anchored findings → inline line comments, body wraps `suggestion` in a + ```suggestion fence (Gitea renders an apply-button) + - unanchored findings → summary-body bullets + - summary body carries the marker for dedupe ``` -Fail-open, comment-only, re-posts on every qualifying trigger (no prior-comment -synthesis yet — framework §6.1). Reviews are tagged with the head SHA. +Fail-open. No duplicate per commit (dedupe). Inline comments + apply-able +suggestions where the line anchors cleanly. Repo-local focus via +`.pr-review.json`. Prior reviews fed as context so re-pushes synthesize instead +of repeating (light version of framework §6.1). ## What "onboarding a repo" means now @@ -37,6 +49,31 @@ 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.) +## Repo-local focus: `.pr-review.json` (optional) + +Drop a `.pr-review.json` at the repo root (committed on the PR's branch, or on +the default branch) to steer the review for that repo. All fields optional; +absent file = defaults. JSON (stdlib, no YAML dependency). + +```json +{ + "focus": ["security", "supply-chain", "sql-injection"], + "exclude_paths": ["vendor/**", "**/*.generated.ts"], + "languages": ["typescript", "go"], + "instructions": "We use Result for error handling. Flag any bare throw. Flag eval()/exec() on user input as critical." +} +``` + +- `focus` — weight these review areas higher (does not blind the reviewer to + critical issues outside them). +- `exclude_paths` — tell the model to ignore these paths. +- `languages` — hint the primary languages. +- `instructions` — free-form house conventions / compliance language. + +Fetched at review time from the PR head ref +(`GET /repos/{o}/{r}/contents/.pr-review.json?ref=`). Bad/missing file +fails open to defaults. The bot's `read:repository` scope reads it. + ## One-time per-owner setup: register a user-level webhook Gitea **system webhooks** (one webhook for the whole instance — the ideal) are @@ -129,11 +166,17 @@ that file once the owner has a user-level webhook — otherwise a labeled PR get reviewed twice. `gitea_admin/pragent`'s own self-CI workflow was retired when the webhook service went live. -## Known limitations (pilot, same as CI-step) +## Known limitations (pilot) -- Re-posts on every qualifying trigger; no prior-comment synthesis (framework §6.1). -- No inline line comments, no status checks, no fail-close. -- `glm-5.2:cloud` only; no tiering, no analyzer fan-out. +- One model (`glm-5.2:cloud`); no tiering, no analyzer fan-out, no shared-prefix + caching. Those are framework features. +- No status checks, no fail-close (review never blocks a PR). +- Dedupe is per-commit: a re-push (new SHA) always re-reviews (by design — the + diff changed). Prior-review context is fed to the model so it doesn't repeat, + but the bot does not delete or resolve its own old reviews. +- Inline comments only anchor to post-change lines present in the diff (context + + added). A finding whose `line` the model places on a removed line or outside + the diff is folded into the summary as a bullet instead of misplaced. - Gitea 1.26.1: system webhooks broken (see above) → user-level webhooks instead; hook delivery-history API (`.../hooks/{id}/tasks`) returns 404, so delivery is observed via the pragent-webhook pod logs (`kubectl -n pragent logs -f deploy/pragent-webhook`). diff --git a/pilot/ai_review.py b/pilot/ai_review.py index 7784693..bddcd11 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -1,51 +1,102 @@ #!/usr/bin/env python3 """pragent pilot — minimal AI PR reviewer. -Runs as a Gitea Actions step. Fetches a PR diff, asks glm-5.2:cloud (via the -on-network headroom proxy, Anthropic /v1/messages format) to review it, and -posts the findings back as a PR review authored by pragent-bot. +Runs as a Gitea Actions step OR is called by the central webhook server +(`webhook_server.py`). Fetches a PR diff, asks glm-5.2:cloud (via the on-network +headroom proxy, Anthropic /v1/messages format) to review it, and posts the +findings back as `pragent-bot` — as a **review summary** plus **inline line +comments** with apply-able ```suggestion blocks where the model could produce +them and the line anchors cleanly to the post-change file. + +Features (pilot v2): + - **Dedupe / persistence:** Gitea itself is the source of truth. Before + reviewing, fetch the PR's existing reviews and look for a hidden + `` marker matching this commit. If present, skip + (no duplicate review on label-toggle / re-fire). Prior review bodies are + fed back to the model as "already said" context so a re-push synthesizes + instead of repeating (light version of design §6.1). + - **Repo-local focus:** if the repo has a `.pr-review.json` at the PR's head + ref, its `focus` / `exclude_paths` / `instructions` / `languages` steer the + review. Optional — defaults apply when absent. + - **Inline comments + suggestions:** the model emits structured JSON + findings with `path`/`line`. We parse the diff hunks to learn which + `(path, new_line)` pairs are valid post-change anchors and post each + anchored finding as a positional review comment; the `suggestion` field, if + non-empty, is wrapped in a ```suggestion fence so Gitea renders an + apply-button. Findings that don't anchor (bad line, unchanged file, etc.) + are folded into the summary body as plain bullets. Fail-open by design: any error becomes a short "review failed" review comment, -and the process always exits 0 so it can never block CI. +and review_pr never raises. Stdlib only — no pip install. -Stdlib only — no pip install, fast cold start in CI. - -Env: - GITEA_API base URL of the in-cluster Gitea, e.g. http://gitea-http.gitea.svc.cluster.local:3000 +Env (CI run() path): + GITEA_API base URL of the in-cluster Gitea GITEA_REPOSITORY "owner/repo" of the PR (github.repository) PR_INDEX PR number (github.event.pull_request.number) PR_TITLE PR title PR_BODY PR body (optional) PRAGENT_BOT_TOKEN bot access token (repo secret) - PRAGENT_SHA head SHA to tag the review (github.event.pull_request.head.sha) + PRAGENT_SHA head SHA to tag the review OLLAMA_URL headroom proxy URL, e.g. http://100.74.17.70:8789 OLLAMA_MODEL model id, e.g. glm-5.2:cloud - OLLAMA_MAX_TOKENS (optional) output cap, default 6000 + OLLAMA_MAX_TOKENS (optional) output cap, default 8000 DIFF_MAX_CHARS (optional) diff truncation cap, default 150000 """ +import base64 import json import os +import re import sys import urllib.error import urllib.request REVIEW_HEADER = "🤖 **AI Review** · pragent pilot · {model} · `{sha}`" +# 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) +# is correctly skipped. +SHA_MARKER = "" +_SHA_MARKER_RE = re.compile(r"") + +AI_REVIEW_LABEL = "AI-REVIEW" +SEVERITIES = ("critical", "high", "medium", "low") +REPO_CONFIG_FILE = ".pr-review.json" 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 changes, missing tests for changed behaviour, and breaking API/contract changes. +Honour any repo-specific focus / instructions given in the prompt; if focus is +given, weight those areas higher, but do not ignore a critical issue outside them. -For each issue, output exactly one line in this format: - - [SEVERITY] path:line — concise problem. suggested fix. -where SEVERITY is one of: critical, high, medium, low. +Output STRICT JSON only — no prose, no markdown fences. Shape: +{ + "findings": [ + { + "severity": "critical|high|medium|low", + "path": "file path exactly as it appears in the diff (`+++ b/` side)", + "line": , + "problem": "one line: what is wrong", + "fix": "one line: how to fix it", + "suggestion": "" + } + ] +} Rules: -- Skip nitpicks, pure formatting, and praise. -- If the diff is clean, output exactly: No issues found. -- Be concise. At most ~15 findings, highest severity first. -- Do not restate the diff. Do not include a summary header. Just the findings lines.""" +- `line` MUST be a line number that exists in the post-change version of `path` + (i.e. a context line or an added `+` line shown in the diff). Never a removed + line. If you are unsure of the exact line, set `line` to the closest context + line you CAN see in the diff. +- `suggestion` is the literal new code that should replace the flagged line(s). + 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. + a missing test, an architectural note). +- Skip nitpicks, pure formatting, and praise. At most ~15 findings, highest + severity first. +- If the diff is clean, output: {"findings": []} +- Do NOT repeat anything already covered in "PREVIOUS REVIEWS" — only surface + new or still-unresolved issues.""" # --------------------------------------------------------------------------- # Pure helpers (unit-tested, no network) @@ -80,16 +131,53 @@ def parse_text_blocks(content: list) -> str: def format_review_body(findings: str, model: str, sha: str) -> str: - """Format the posted review body. Findings empty -> "No issues found.".""" + """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.". 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() if not findings: findings = "No issues found." - return f"{header}\n\n{findings}" + marker = SHA_MARKER.format(sha=sha) if sha else "" + body = f"{header}\n\n{findings}" + if marker: + body += f"\n{marker}" + return body -def build_user_prompt(title: str, body: str, diff: str) -> str: - parts = [f"## PR\nTitle: {title or '(none)'}"] +def build_user_prompt( + title: str, + body: str, + diff: str, + config: dict | None = None, + prior_reviews: list[str] | None = None, +) -> str: + """Assemble the user prompt: repo config + prior reviews + PR meta + diff.""" + parts: list[str] = [] + + if config: + cfg_lines = [] + if config.get("focus"): + cfg_lines.append("Focus areas: " + ", ".join(config["focus"])) + if config.get("exclude_paths"): + cfg_lines.append("Ignore paths: " + ", ".join(config["exclude_paths"])) + if config.get("languages"): + cfg_lines.append("Languages: " + ", ".join(config["languages"])) + if config.get("instructions"): + cfg_lines.append("Instructions:\n" + str(config["instructions"]).strip()) + if cfg_lines: + parts.append("## Repo review config (.pr-review.json)\n" + "\n".join(cfg_lines)) + + if prior_reviews: + joined = "\n\n---\n\n".join(prior_reviews) + if len(joined) > 8000: + joined = joined[:8000] + "\n…[prior reviews truncated]" + parts.append("## PREVIOUS REVIEWS (already posted — do NOT repeat these points)\n" + joined) + + parts.append(f"## PR\nTitle: {title or '(none)'}") if body and body.strip(): b = body.strip() if len(b) > 4000: @@ -99,6 +187,256 @@ def build_user_prompt(title: str, body: str, diff: str) -> str: return "\n\n".join(parts) +# --------------------------------------------------------------------------- +# Diff parsing — find valid post-change (RIGHT-side) line anchors per file +# --------------------------------------------------------------------------- + + +def parse_diff_anchors(diff: str) -> dict[str, set[int]]: + """Parse a unified diff into {path: {new_line, ...}} for lines that exist in + the post-change version (context + added lines). Removed lines are NOT + anchors (they have no RIGHT-side line). Used to validate inline comments. + + Robust to: + - `diff --git a/x b/x` and `+++ b/x` path headers (uses the `b/` side) + - hunk headers `@@ -a,b +c,d @@` (new line counter starts at c) + - No-newline-at-eof markers, binary files, missing hunks. + """ + anchors: dict[str, set[int]] = {} + current_path: str | None = None + new_line = 0 + for raw in (diff or "").splitlines(): + # File path: prefer the `+++ b/` line (handles renames); fall back to + # `diff --git a/x b/x`'s second path. + if raw.startswith("+++ "): + p = raw[4:].strip() + if p == "/dev/null": + current_path = None + else: + current_path = _strip_path_prefix(p) + anchors.setdefault(current_path, set()) + continue + if raw.startswith("diff --git "): + # `diff --git a/foo b/foo` — take the second path as a fallback in + # case the `+++` line is missing (binary). Split on " b/". + m = re.search(r" b/(.+)$", raw) + if m: + current_path = m.group(1).strip() + anchors.setdefault(current_path, set()) + continue + if raw.startswith("@@"): + m = re.search(r"\+(\d+)(?:,\d+)?\s@@", raw) + new_line = int(m.group(1)) if m else 0 + continue + if current_path is None: + continue + if raw.startswith("\\ No newline"): + continue + if raw.startswith("-"): + # removed line — no RIGHT-side anchor + continue + if raw.startswith("+"): + anchors[current_path].add(new_line) + new_line += 1 + continue + # context line (" " or anything else within a hunk) + if raw.startswith(" "): + anchors[current_path].add(new_line) + new_line += 1 + return anchors + + +def _strip_path_prefix(p: str) -> str: + """`b/foo` or `foo` -> `foo`.""" + if p.startswith("b/"): + return p[2:] + return p + + +# --------------------------------------------------------------------------- +# Model output parsing — tolerant JSON findings extraction +# --------------------------------------------------------------------------- + + +def parse_findings(text: str) -> list[dict]: + """Parse the model's JSON response into a list of finding dicts. + + Tolerant: strips ```json fences, and if the model wrapped JSON in prose, + scans for the first balanced `{...}` and extracts its `findings` array. + Drops findings missing path/line or with an unknown severity (normalised). + Never raises — returns [] on any parse failure. + """ + if not text: + return [] + s = text.strip() + # Strip a single wrapping code fence if present. + if s.startswith("```"): + s = re.sub(r"^```[a-zA-Z]*\n?", "", s) + s = re.sub(r"\n?```$", "", s).strip() + data = None + try: + data = json.loads(s) + except json.JSONDecodeError: + obj = _extract_first_json_object(s) + if obj is not None: + try: + data = json.loads(obj) + except json.JSONDecodeError: + data = None + if not isinstance(data, dict): + return [] + findings = data.get("findings") + if not isinstance(findings, list): + return [] + out = [] + for f in findings: + if not isinstance(f, dict): + continue + path = f.get("path") + line = f.get("line") + if not isinstance(path, str) or not path.strip(): + continue + if not isinstance(line, int) or line < 1: + continue + sev = str(f.get("severity", "medium")).strip().lower() + if sev not in SEVERITIES: + sev = "medium" + out.append({ + "severity": sev, + "path": path.strip(), + "line": line, + "problem": str(f.get("problem", "")).strip(), + "fix": str(f.get("fix", "")).strip(), + "suggestion": str(f.get("suggestion", "") or "").strip(), + }) + return out + + +def _extract_first_json_object(s: str) -> str | None: + """Return the substring of the first balanced top-level `{ ... }` in s.""" + start = s.find("{") + if start < 0: + return None + depth = 0 + in_str = False + esc = False + for i in range(start, len(s)): + c = s[i] + if in_str: + if esc: + esc = False + elif c == "\\": + esc = True + elif c == '"': + in_str = False + continue + if c == '"': + in_str = True + elif c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return s[start:i + 1] + return None + + +def split_findings(findings: list[dict], anchors: dict[str, set[int]]) -> tuple[list[dict], list[dict]]: + """Split findings into (anchored, unanchored). + + A finding is anchored if its path is known AND its line is a valid post-change + line for that path. Lines just outside the diff (model off-by-one) are NOT + anchored — safer to keep them as summary bullets than to drop or misplace. + """ + anchored, unanchored = [], [] + for f in findings: + valid = anchors.get(f["path"]) + if valid and f["line"] in valid: + anchored.append(f) + else: + unanchored.append(f) + return anchored, unanchored + + +def inline_comment_body(f: dict) -> str: + """Render one finding as a positional review-comment body. + + Includes a ```suggestion fence only if the model produced non-empty + replacement code. Gitea renders that as an apply-able suggestion. + """ + sev = f["severity"].upper() + body = f"**[{sev}]** {f['problem']}" + if f["fix"]: + body += f"\n\nFix: {f['fix']}" + if f["suggestion"]: + body += f"\n\n```suggestion\n{f['suggestion']}\n```" + return body + + +def summary_bullets(findings: list[dict]) -> str: + """Render unanchored findings as summary-body bullets (no line anchor).""" + lines = [] + for f in findings: + loc = f"{f['path']}:{f['line']}" if f["line"] else f["path"] + fix = f" — fix: {f['fix']}" if f["fix"] else "" + lines.append(f"- **[{f['severity'].upper()}]** `{loc}` — {f['problem']}{fix}") + return "\n".join(lines) + + +# --------------------------------------------------------------------------- +# Repo config + existing-review helpers +# --------------------------------------------------------------------------- + + +def parse_repo_config(raw: str) -> dict: + """Parse a .pr-review.json blob tolerantly. Returns {} on any failure.""" + if not raw: + return {} + try: + data = json.loads(raw) + except json.JSONDecodeError: + return {} + if not isinstance(data, dict): + return {} + out = {} + for k in ("focus", "exclude_paths", "languages"): + v = data.get(k) + if isinstance(v, list) and all(isinstance(x, str) for x in v): + out[k] = v + instr = data.get("instructions") + if isinstance(instr, str) and instr.strip(): + out["instructions"] = instr.strip() + return out + + +def reviewed_shas(reviews: list[dict]) -> set[str]: + """Pull every `` marker out of a PR's reviews.""" + shas: set[str] = set() + for r in reviews or []: + body = r.get("body") or "" + for m in _SHA_MARKER_RE.finditer(body): + shas.add(m.group(1)) + return shas + + +def prior_review_bodies(reviews: list[dict], current_sha: str, limit: int = 6) -> list[str]: + """Bodies of prior bot reviews (older shas), newest-first, bounded.""" + out = [] + for r in reviews or []: + body = (r.get("body") or "").strip() + if not body: + continue + shas = _SHA_MARKER_RE.findall(body) + # Skip the current sha (that would be a self-reference) and non-bot + # noise; keep reviews that carry our marker. + if not shas: + continue + if current_sha and current_sha in shas: + continue + out.append(body) + return out[:limit] + + # --------------------------------------------------------------------------- # Network helpers # --------------------------------------------------------------------------- @@ -147,6 +485,35 @@ def fetch_pr_diff(api: str, repo: str, index: str, token: str, max_chars: int) - return truncate_diff("\n".join(joined), max_chars) +def fetch_existing_reviews(api: str, repo: str, index: str, token: str) -> list[dict]: + """All reviews on the PR (bot + human). Empty list on failure (fail-open).""" + status, raw = gitea_get(api, repo, f"pulls/{index}/reviews", token) + if status != 200: + return [] + try: + data = json.loads(raw) + except json.JSONDecodeError: + return [] + return data if isinstance(data, list) else [] + + +def fetch_repo_config(api: str, repo: str, sha: str, token: str) -> dict: + """Fetch .pr-review.json from the PR's head ref. {} if absent/unreadable.""" + if not sha: + return {} + status, raw = gitea_get(api, repo, f"contents/{REPO_CONFIG_FILE}?ref={sha}", token) + if status != 200: + return {} + try: + data = json.loads(raw) + content_b64 = data.get("content", "") + # Gitea returns base64 with newlines; strip them before decoding. + decoded = base64.b64decode(content_b64.replace("\n", "")).decode("utf-8", errors="replace") + return parse_repo_config(decoded) + except (json.JSONDecodeError, ValueError): + return {} + + def call_model(ollama_url: str, model: str, system: str, user: str, max_tokens: int) -> str: payload = { "model": model, @@ -167,6 +534,7 @@ def call_model(ollama_url: str, model: str, system: str, user: str, max_tokens: def post_review(api: str, repo: str, index: str, token: str, body: str) -> None: + """Post a body-only review (summary / failure note). No inline comments.""" status, raw = gitea_post(api, repo, f"pulls/{index}/reviews", token, {"event": "COMMENT", "body": body}) if status not in (200, 201): # Fallback to a plain issue comment if reviews endpoint refuses. @@ -175,6 +543,33 @@ def post_review(api: str, repo: str, index: str, token: str, body: str) -> None: raise RuntimeError(f"post review failed: reviews={status}, comments={status2}") +def post_inline_review( + api: str, repo: str, index: str, token: str, summary: str, anchored: list[dict] +) -> None: + """Post a review with a summary body AND positional inline comments. + + Each anchored finding becomes one entry in `comments`: + {path, side:"RIGHT", line, body}. The body carries the ```suggestion + fence when the model produced replacement code. + """ + comments = [ + { + "path": f["path"], + "side": "RIGHT", + "line": f["line"], + "body": inline_comment_body(f), + } + for f in anchored + ] + payload = {"event": "COMMENT", "body": summary, "comments": comments} + status, raw = gitea_post(api, repo, f"pulls/{index}/reviews", token, payload) + if status in (200, 201): + return + # If the inline post failed (e.g. a bad line slipped through), retry as a + # body-only review so the findings still land somewhere. + post_review(api, repo, index, token, summary) + + # --------------------------------------------------------------------------- # Main # --------------------------------------------------------------------------- @@ -197,23 +592,61 @@ def review_pr( token: str, ollama_url: str, model: str, - max_tokens: int = 6000, + max_tokens: int = 8000, max_chars: int = 150000, ) -> bool: - """Run one review and post it as a PR comment. + """Run one review and post it as `pragent-bot`. - Returns True on success, False on failure (failure note is posted when - possible). Never raises — fail-open by design. Both the CI `run()` entry - point and the central webhook server call this. + Dedupe: if a prior review already carries this commit's sha marker, skip + (no duplicate). Otherwise: fetch repo config + prior-review context, call + the model, parse JSON findings, anchor what we can to diff lines, post a + review with inline comments + suggestions (unanchored findings → summary + bullets). + + 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. """ try: + reviews = fetch_existing_reviews(api, repo, index, token) + # Dedupe: already reviewed this exact commit -> nothing to do. + if sha and sha in reviewed_shas(reviews): + print(f"pragent: {repo}#{index} sha={sha[:8]} already reviewed, skipping", flush=True) + return True + diff, _truncated, _orig = fetch_pr_diff(api, repo, index, token, max_chars) if not diff.strip(): post_review(api, repo, index, token, format_review_body("No diff content to review.", model, sha)) return True - user_prompt = build_user_prompt(title, body, diff) - findings = call_model(ollama_url, model, SYSTEM_PROMPT, user_prompt, max_tokens) - post_review(api, repo, index, token, format_review_body(findings, model, sha)) + + config = fetch_repo_config(api, repo, sha, token) + prior = prior_review_bodies(reviews, sha) + 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) + + anchors = parse_diff_anchors(diff) + anchored, unanchored = split_findings(findings, anchors) + + # Summary body: the unanchored bullets (or "No issues found."), plus a + # one-line note when inline comments were posted so the summary isn't + # empty-looking. + bullets = summary_bullets(unanchored) + summary_parts = [] + if anchored: + summary_parts.append(f"_{len(anchored)} inline comment(s) posted below._") + if bullets: + 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) + + post_inline_review(api, repo, index, token, summary_body, anchored) + print( + f"pragent: reviewed {repo}#{index} sha={sha[:8]} " + f"findings={len(findings)} inline={len(anchored)}", + flush=True, + ) return True except Exception as e: # fail-open try: @@ -235,7 +668,7 @@ def run() -> int: token=_need("PRAGENT_BOT_TOKEN"), ollama_url=_need("OLLAMA_URL"), model=_need("OLLAMA_MODEL"), - max_tokens=int(os.environ.get("OLLAMA_MAX_TOKENS", "6000")), + max_tokens=int(os.environ.get("OLLAMA_MAX_TOKENS", "8000")), max_chars=int(os.environ.get("DIFF_MAX_CHARS", "150000")), ) return 0 diff --git a/pilot/webhook_server.py b/pilot/webhook_server.py index e622af4..148673c 100644 --- a/pilot/webhook_server.py +++ b/pilot/webhook_server.py @@ -1,12 +1,14 @@ #!/usr/bin/env python3 """pragent pilot — central webhook receiver. -A stdlib-only HTTP server that Gitea posts system-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 CI-step pilot uses, posting findings back as `pragent-bot`. -Zero per-repo setup: one Gitea **system webhook** fires for every repo on the -instance; this service filters to labeled PRs. Onboarding a repo = label a PR. +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 +webhooks are broken — see pilot/README-webhook.md.) Onboarding a repo = add the +bot as a Write collaborator + create the label + label a PR. Stdlib only — no pip install, runs on python:3-slim with the scripts mounted. @@ -17,7 +19,8 @@ Endpoints: Env: WEBHOOK_SECRET shared secret used to register the Gitea webhook (HMAC) GITEA_API in-cluster Gitea base URL - PRAGENT_BOT_TOKEN pragent-bot access token (admin so it can read any repo) + PRAGENT_BOT_TOKEN pragent-bot access token (non-admin; must be a Write + collaborator on each reviewed repo) OLLAMA_URL headroom proxy URL, e.g. http://100.74.17.70:8789 OLLAMA_MODEL model id, e.g. glm-5.2:cloud OLLAMA_MAX_TOKENS (optional) output cap, default 6000 @@ -46,7 +49,7 @@ GITEA_API = os.environ.get("GITEA_API", "http://gitea-http.gitea.svc.cluster.loc BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "") OLLAMA_URL = os.environ.get("OLLAMA_URL", "http://100.74.17.70:8789") OLLAMA_MODEL = os.environ.get("OLLAMA_MODEL", "glm-5.2:cloud") -OLLAMA_MAX_TOKENS = int(os.environ.get("OLLAMA_MAX_TOKENS", "6000")) +OLLAMA_MAX_TOKENS = int(os.environ.get("OLLAMA_MAX_TOKENS", "8000")) DIFF_MAX_CHARS = int(os.environ.get("DIFF_MAX_CHARS", "150000")) WEBHOOK_SECRET = os.environ.get("WEBHOOK_SECRET", "").encode() PORT = int(os.environ.get("WEBHOOK_PORT", "8080")) diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index a633824..0161e27 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -10,7 +10,15 @@ sys.path.insert(0, os.path.join(ROOT, "pilot")) from ai_review import ( # noqa: E402 build_user_prompt, format_review_body, + inline_comment_body, + parse_diff_anchors, + parse_findings, + parse_repo_config, parse_text_blocks, + prior_review_bodies, + reviewed_shas, + split_findings, + summary_bullets, truncate_diff, ) @@ -130,4 +138,206 @@ def test_build_user_prompt_truncates_long_body(): def test_build_user_prompt_no_body(): p = build_user_prompt("t", "", "d") - assert "Description:" not in p \ No newline at end of file + assert "Description:" not in p + + +def test_build_user_prompt_with_config_and_prior(): + cfg = {"focus": ["security"], "instructions": "Use Result."} + prior = ["🤖 **AI Review** …\n- [high] x:1 — old."] + p = build_user_prompt("t", "b", "diff --git a/x b/x", config=cfg, prior_reviews=prior) + assert "## Repo review config" in p + assert "security" in p + assert "Result" in p + assert "## PREVIOUS REVIEWS" in p + assert "old." in p + + +# --------------------------------------------------------------------------- +# parse_diff_anchors +# --------------------------------------------------------------------------- + + +_DIFF = """\ +diff --git a/src/a.py b/src/a.py +index 1..2 100644 +--- a/src/a.py ++++ b/src/a.py +@@ -1,4 +1,5 @@ + context +-removed ++added + context2 +@@ -10,3 +10,4 @@ + keep ++new + last +diff --git a/binary.bin b/binary.bin +new file mode 100644 +index 0..1 +Binary files differ +""" + + +def test_parse_diff_anchors_context_and_added(): + a = parse_diff_anchors(_DIFF) + # context(1), +added(2), context2(3) | keep(10), +new(11), last(12) + assert a["src/a.py"] == {1, 2, 3, 10, 11, 12} + # removed line (-removed, old line 2) has no new-line anchor + assert 2 in a["src/a.py"] # 2 here is the +added line, not the removed one + + +def test_parse_diff_anchors_binary_file_present_no_lines(): + a = parse_diff_anchors(_DIFF) + assert "binary.bin" in a + assert a["binary.bin"] == set() + + +def test_parse_diff_anchors_empty(): + assert parse_diff_anchors("") == {} + assert parse_diff_anchors(None) == {} # type: ignore[arg-type] + + +def test_parse_diff_anchors_new_file(): + diff = "diff --git a/new.ts b/new.ts\nnew file mode 100644\n--- /dev/null\n+++ b/new.ts\n@@ -0,0 +1,3 @@\n+a\n+b\n+c\n" + a = parse_diff_anchors(diff) + assert a["new.ts"] == {1, 2, 3} + + +# --------------------------------------------------------------------------- +# parse_findings +# --------------------------------------------------------------------------- + + +def test_parse_findings_clean_json(): + txt = '{"findings":[{"severity":"high","path":"a.py","line":3,"problem":"x","fix":"y","suggestion":"z"}]}' + fs = parse_findings(txt) + assert len(fs) == 1 + assert fs[0]["severity"] == "high" + assert fs[0]["path"] == "a.py" + assert fs[0]["line"] == 3 + + +def test_parse_findings_fenced_json(): + txt = '```json\n{"findings":[{"severity":"low","path":"b.go","line":1,"problem":"p","fix":"","suggestion":""}]}\n```' + fs = parse_findings(txt) + assert len(fs) == 1 + assert fs[0]["path"] == "b.go" + + +def test_parse_findings_json_in_prose(): + txt = 'Here is my review: {"findings":[{"severity":"critical","path":"c","line":9,"problem":"q"}]} thanks!' + fs = parse_findings(txt) + assert len(fs) == 1 + assert fs[0]["severity"] == "critical" + + +def test_parse_findings_empty(): + assert parse_findings('{"findings":[]}') == [] + assert parse_findings("") == [] + assert parse_findings("not json at all") == [] + + +def test_parse_findings_drops_bad_entries(): + # missing path, bad line, unknown severity (normalised) + txt = '{"findings":[{"line":1},{"path":"x","line":-1},{"path":"x","line":2,"severity":"bogus","problem":"p"}]}' + fs = parse_findings(txt) + assert len(fs) == 1 + assert fs[0]["severity"] == "medium" + + +# --------------------------------------------------------------------------- +# split_findings + inline_comment_body + summary_bullets +# --------------------------------------------------------------------------- + + +def test_split_findings_by_anchor(): + anchors = {"a.py": {1, 3, 4}} + fs = [ + {"severity": "high", "path": "a.py", "line": 3, "problem": "p", "fix": "f", "suggestion": ""}, + {"severity": "low", "path": "a.py", "line": 99, "problem": "off", "fix": "", "suggestion": ""}, + {"severity": "medium", "path": "other.go", "line": 1, "problem": "x", "fix": "", "suggestion": ""}, + ] + anchored, unanchored = split_findings(fs, anchors) + assert [f["line"] for f in anchored] == [3] + assert len(unanchored) == 2 + + +def test_inline_comment_body_with_suggestion(): + f = {"severity": "high", "path": "a", "line": 1, "problem": "bad", "fix": "swap", "suggestion": "good()"} + body = inline_comment_body(f) + assert "**[HIGH]**" in body + assert "bad" in body + assert "```suggestion\n" in body + assert "good()" in body + + +def test_inline_comment_body_no_suggestion(): + f = {"severity": "low", "path": "a", "line": 1, "problem": "p", "fix": "f", "suggestion": ""} + body = inline_comment_body(f) + assert "```suggestion" not in body + assert "Fix: f" in body + + +def test_summary_bullets_format(): + fs = [{"severity": "high", "path": "a.py", "line": 7, "problem": "p", "fix": "f", "suggestion": ""}] + b = summary_bullets(fs) + assert "- **[HIGH]**" in b + assert "`a.py:7`" in b + + +# --------------------------------------------------------------------------- +# repo config parsing +# --------------------------------------------------------------------------- + + +def test_parse_repo_config_full(): + raw = '{"focus":["security","perf"],"exclude_paths":["vendor/**"],"languages":["go"],"instructions":"be strict"}' + c = parse_repo_config(raw) + assert c["focus"] == ["security", "perf"] + assert c["exclude_paths"] == ["vendor/**"] + assert c["instructions"] == "be strict" + + +def test_parse_repo_config_partial_and_bad(): + assert parse_repo_config('{"focus":"not-a-list"}') == {} + assert parse_repo_config('{"focus":["ok"]}') == {"focus": ["ok"]} + assert parse_repo_config("") == {} + assert parse_repo_config("not json") == {} + assert parse_repo_config('{"instructions":" "}') == {} + + +# --------------------------------------------------------------------------- +# dedupe / prior-context parsing +# --------------------------------------------------------------------------- + + +def test_reviewed_shas_extracts_marker(): + reviews = [ + {"body": "🤖 AI Review · glm · `abcdef12`\n\n"}, + {"body": "human comment, no marker"}, + {"body": ""}, + ] + shas = reviewed_shas(reviews) + assert "abcdef1234567890" in shas + assert "0987654" in shas + + +def test_reviewed_shas_empty(): + assert reviewed_shas([]) == set() + assert reviewed_shas([{"body": "no marker"}]) == set() + + +def test_prior_review_bodies_skips_current_sha(): + reviews = [ + {"body": "r1\n"}, + {"body": "r2\n"}, + {"body": "no marker here"}, + ] + prior = prior_review_bodies(reviews, current_sha="2222222") + assert len(prior) == 1 + assert "r1" in prior[0] + + +def test_format_review_body_has_sha_marker(): + body = format_review_body("- [high] x:1 — b", "glm-5.2:cloud", "abcdef1234567890") + assert "" in body \ No newline at end of file