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