Marcos
087834565d
Add per-review + per-comment token accounting, surfaced only when a PR carries the new AI-USAGE label (on top of the existing AI-REVIEW trigger). opencode_review: - run_opencode now uses `--format json`; parse_opencode_events reconstructs the assistant text from `text` events and sums tokens/cost/steps from every `step_finish` event (tolerant of noise / missing fields). - run() measures duration_s around the opencode call and returns (text, usage). - changed_files(diff) extracts the `+++ b/` paths; the brief now lists them under a "Changed files" focus block so the agent grounds findings in the diff's neighbourhood instead of unbounded whole-repo walks. ai_review: - format_usage_section renders a `## AI usage` block: measured totals (in/out/reasoning/cache/cost/steps/duration), the whole-repo scope note, and an attributed per-finding table. Per-comment counts are output tokens split by each finding's body weight — labelled "attributed" since one model pass produces all findings. - inline_comment_body appends `🪙 ~N tok (X% · attributed output)` when attribution is present. - review_pr gains report_usage; compute_attribution stashes _tok_attrib/_tok_pct. - format_review_body inserts the usage section between summary and findings. webhook_server: - Fire on every pull_request action except `closed` (denylist, was an allowlist) — the AI-REVIEW gate + sha dedupe keep this safe. - AI-USAGE label detection + PRAGENT_USAGE_ALWAYS env drive report_usage. .opencode factory + review-methodology skill: new "Ground findings in context" step — read callers/imports/sibling functions per changed file (1-3 files per finding), no unbounded walks. Tests: parse_opencode_events (text+usage sum, malformed tolerance, none-usage), changed_files, compute_attribution math, inline 🪙 line, format_usage_section totals/table/cost, format_review_body ordering. 68 passing. Co-Authored-By: Claude <noreply@anthropic.com>
2.8 KiB
name, description
| name | description |
|---|---|
| review-methodology | pragent review methodology — severity rubric, what to report vs skip, anchoring rules, and how to honor repo focus. Load this before reviewing a PR. |
pragent review methodology
Severity rubric
- critical — exploitable security bug, data loss/corruption, or a crash on a normal input path. Must fix before merge.
- high — correctness bug on a real input path, broken contract, or a missing test for security/error behavior. Should fix before merge.
- medium — likely bug on an edge case, missing test for changed logic, or a risky pattern that isn't broken yet. Worth fixing.
- low — minor risk, stale expectation, or a defensive improvement. Nice to have.
Report vs skip
Report: correctness bugs, security problems, risky changes, missing tests for changed behavior, breaking API/contract changes, N+1/O(n²) in hot paths.
Skip: praise, nitpicks, pure formatting/style, personal preference,
speculative "what if" without a concrete trigger, anything already covered in
prior_reviews.
Cap at ~15 findings, highest severity first. Quality over quantity — an empty findings list for a clean diff is a correct result.
Anchoring (for inline comments)
Each finding's line MUST be a line that exists in the POST-CHANGE version of
path:
- a context line (unchanged, shown with a leading space in the diff), or
- an added line (shown with a leading
+).
Never anchor on a removed (-) line — it has no post-change line number.
If you're unsure of the exact line, use the closest context line you CAN see in
the diff. A misanchored finding becomes a summary bullet instead of an inline
comment, so correct anchoring is what gets a finding shown inline with its
suggested-fix code block (language-highlighted) rather than demoted to a bullet.
Ground each finding in context
Don't flag a hunk in isolation. For each changed file, read its callers, imports, sibling functions, and type definitions (the repo is checked out at the head sha), and make sure the finding holds against how the change is actually used. Keep it bounded — 1–3 related files per finding, no unbounded whole-repo walks.
Honoring repo config
If .pr-review.json is present, honor it:
focus— weight these areas higher, but never ignore a critical issue outside them.exclude_paths— skip findings in these paths.languages— hint to the primary languages; pick matching linters.instructions— house conventions / compliance language; treat as binding reviewer rules.
Linters are a signal, not the verdict
Run the repo's own typecheck/lint on changed files, but translate their output
into human findings — a raw TS2322 is not a review comment. Correlate
diagnostics with the diff; ignore diagnostics in files the PR didn't touch.