pilot: dedupe + repo config + inline comments w/ suggestions
- Dedupe: Gitea-as-state. Scan existing reviews for a hidden
<!-- pragent:sha=... --> 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 <noreply@anthropic.com>
This commit is contained in:
Marcos
+52
-9
@@ -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
|
||||
<!-- pragent:sha=<this sha> --> (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 <!-- pragent:sha=... --> 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<T,E> 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=<head sha>`). 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`).
|
||||
|
||||
+463
-30
@@ -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
|
||||
`<!-- pragent:sha=... -->` 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 = "<!-- pragent:sha={sha} -->"
|
||||
_SHA_MARKER_RE = re.compile(r"<!-- pragent:sha=([0-9a-f]{7,40}) -->")
|
||||
|
||||
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": <int, the NEW-file line number the issue is on, within the diff>,
|
||||
"problem": "one line: what is wrong",
|
||||
"fix": "one line: how to fix it",
|
||||
"suggestion": "<exact replacement lines for that location, or empty string if you cannot produce safe replacement code>"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
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 `<!-- pragent:sha=... -->` 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
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -131,3 +139,205 @@ 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
|
||||
|
||||
|
||||
def test_build_user_prompt_with_config_and_prior():
|
||||
cfg = {"focus": ["security"], "instructions": "Use Result<T,E>."}
|
||||
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<T,E>" 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<!-- pragent:sha=abcdef1234567890 -->"},
|
||||
{"body": "human comment, no marker"},
|
||||
{"body": "<!-- pragent:sha=0987654 -->"},
|
||||
]
|
||||
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<!-- pragent:sha=1111111 -->"},
|
||||
{"body": "r2\n<!-- pragent:sha=2222222 -->"},
|
||||
{"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 "<!-- pragent:sha=abcdef1234567890 -->" in body
|
||||
Reference in New Issue
Block a user