refactor(review): always render usage; drop report_usage flag
This commit is contained in:
+6
-11
@@ -16,7 +16,6 @@ Service pragent-webhook.pragent.svc.cluster.local (ClusterIP, ns pragent)
|
|||||||
│ → gate: action ≠ closed AND pull_request.labels ∋ AI-REVIEW
|
│ → gate: action ≠ closed AND pull_request.labels ∋ AI-REVIEW
|
||||||
│ → claim (repo, index, sha) in-flight (closes the dedupe race)
|
│ → claim (repo, index, sha) in-flight (closes the dedupe race)
|
||||||
│ → bounded worker (PRAGENT_MAX_CONCURRENT_REVIEWS, default 2)
|
│ → 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)
|
ai_review.review_pr() (same core the CI-step uses)
|
||||||
1. fetch existing reviews → dedupe: skip if a review already carries
|
1. fetch existing reviews → dedupe: skip if a review already carries
|
||||||
@@ -67,10 +66,10 @@ 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
|
be covered by a user-level webhook — see below. If not, do the one-time
|
||||||
per-owner setup first.)
|
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
|
Every opencode review now appends a token-usage report — no label, no env var
|
||||||
the same PR opts the review into appending a token-usage report:
|
needed:
|
||||||
|
|
||||||
- a `## 🔋 AI usage` section on the review summary body with the **measured**
|
- a `## 🔋 AI usage` section on the review summary body with the **measured**
|
||||||
review total — input / output / reasoning / cache read+write / total tokens,
|
review total — input / output / reasoning / cache read+write / total tokens,
|
||||||
@@ -88,12 +87,8 @@ rendered-body weight (`len(problem)+len(fix)+len(suggestion)`) — an honest
|
|||||||
attribution, labelled as such. The totals are real measurements summed from
|
attribution, labelled as such. The totals are real measurements summed from
|
||||||
opencode's `step_finish` events.
|
opencode's `step_finish` events.
|
||||||
|
|
||||||
`PRAGENT_USAGE_ALWAYS=1` on the Deployment forces usage reporting on for every
|
No-op on the ollama fallback (no usage available). The usage section is part
|
||||||
review (testing / a future default-on) regardless of the label.
|
of the review body, so it's covered by the existing sha-marker dedupe.
|
||||||
|
|
||||||
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.
|
|
||||||
|
|
||||||
## Repo-provided static context (`ADDITIONAL_CONTEXT_URL`)
|
## Repo-provided static context (`ADDITIONAL_CONTEXT_URL`)
|
||||||
|
|
||||||
@@ -483,7 +478,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
|
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
|
`(summary, findings)`, validates findings against diff anchors, and posts. So
|
||||||
all v2 logic (dedupe marker, anchor validation, language-tagged suggestion
|
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 model remembering it.
|
||||||
|
|
||||||
The factory lives in the pragent repo root: `opencode.json` (provider/model/
|
The factory lives in the pragent repo root: `opencode.json` (provider/model/
|
||||||
|
|||||||
+12
-13
@@ -1998,7 +1998,6 @@ def review_pr(
|
|||||||
model: str,
|
model: str,
|
||||||
max_tokens: int = 8000,
|
max_tokens: int = 8000,
|
||||||
max_chars: int = 150000,
|
max_chars: int = 150000,
|
||||||
report_usage: bool = False,
|
|
||||||
base_ref: str = "",
|
base_ref: str = "",
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Run one review and post it as `pragent-bot`.
|
"""Run one review and post it as `pragent-bot`.
|
||||||
@@ -2013,10 +2012,11 @@ def review_pr(
|
|||||||
from the PR head) so a PR cannot ship its own reviewer instructions; empty
|
from the PR head) so a PR cannot ship its own reviewer instructions; empty
|
||||||
means "the repo's default branch".
|
means "the repo's default branch".
|
||||||
|
|
||||||
`report_usage`: when True (PR carries the `AI-USAGE` label), the opencode
|
The opencode engine's measured token/cost usage is always rendered as a
|
||||||
engine's measured token/cost usage is rendered as a `## 🔋 AI usage` section
|
`## 🔋 AI usage` section on the review body and an attributed `🪙 ~N tok`
|
||||||
on the review body and an attributed `🪙 ~N tok` line on each inline
|
line on each inline comment when usage data is available (i.e. when the
|
||||||
comment. No-op on the ollama fallback (no usage available).
|
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
|
Returns True on success (including a deliberate skip), False on failure
|
||||||
(failure note posted when possible). Never raises — fail-open by design.
|
(failure note posted when possible). Never raises — fail-open by design.
|
||||||
@@ -2100,16 +2100,16 @@ def review_pr(
|
|||||||
review_summary, findings, summary_changes, risks, _walkthrough, _risk_verdict, _test_coverage = 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:
|
if not findings and not review_summary:
|
||||||
# The findings JSON was missing or malformed. Don't discard the
|
# The findings JSON was missing or malformed. Don't discard the
|
||||||
# run: salvage the prose, keep the usage report (the label asked
|
# run: salvage the prose, keep the usage report (the tokens were
|
||||||
# for it, and the tokens were spent either way), and log enough
|
# spent either way), and log enough of the raw output to
|
||||||
# of the raw output to diagnose why the agent went off-format.
|
# diagnose why the agent went off-format.
|
||||||
print(
|
print(
|
||||||
f"pragent: {repo}#{index} sha={sha[:8]} unparseable output "
|
f"pragent: {repo}#{index} sha={sha[:8]} unparseable output "
|
||||||
f"({len(stdout)} chars); tail: {stdout[-600:]!r}",
|
f"({len(stdout)} chars); tail: {stdout[-600:]!r}",
|
||||||
file=sys.stderr, flush=True,
|
file=sys.stderr, flush=True,
|
||||||
)
|
)
|
||||||
salvaged = salvage_summary(stdout)
|
salvaged = salvage_summary(stdout)
|
||||||
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(
|
post_review(api, repo, index, token, format_review_body(
|
||||||
salvaged or "AI review produced no parseable output.",
|
salvaged or "AI review produced no parseable output.",
|
||||||
model, sha, usage_section=usage_section))
|
model, sha, usage_section=usage_section))
|
||||||
@@ -2154,11 +2154,10 @@ def review_pr(
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Compute attribution so inline comments + the table can show per-comment
|
# Compute attribution so inline comments + the table can show per-comment
|
||||||
# estimates. Only meaningful when we have measured usage AND the PR asked
|
# estimates. Only meaningful when we have measured usage.
|
||||||
# for it.
|
if usage and usage.get("output"):
|
||||||
if report_usage and usage and usage.get("output"):
|
|
||||||
compute_attribution(findings, usage["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
|
# Anchor against the RAW diff, never the compressed one. Compression
|
||||||
# drops context lines, so a finding on a line that survived in the file
|
# drops context lines, so a finding on a line that survived in the file
|
||||||
|
|||||||
+35
-13
@@ -43,6 +43,13 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
|||||||
|
|
||||||
from ai_review import review_pr
|
from ai_review import 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
|
# 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
|
# 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
|
# gate + sha dedupe downstream make broadening safe: a same-sha re-fire (title
|
||||||
@@ -53,7 +60,6 @@ from ai_review import review_pr
|
|||||||
# `X-Gitea-Event-Type` header uses `label_updated` / `synchronized`.
|
# `X-Gitea-Event-Type` header uses `label_updated` / `synchronized`.
|
||||||
SKIP_ACTIONS = {"closed"}
|
SKIP_ACTIONS = {"closed"}
|
||||||
AI_REVIEW_LABEL = "AI-REVIEW"
|
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")
|
GITEA_API = os.environ.get("GITEA_API", "http://gitea-http.gitea.svc.cluster.local:3000")
|
||||||
BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "")
|
BOT_TOKEN = os.environ.get("PRAGENT_BOT_TOKEN", "")
|
||||||
@@ -65,6 +71,9 @@ WEBHOOK_SECRET = os.environ.get("WEBHOOK_SECRET", "").encode()
|
|||||||
PORT = int(os.environ.get("WEBHOOK_PORT", "8080"))
|
PORT = int(os.environ.get("WEBHOOK_PORT", "8080"))
|
||||||
MAX_CONCURRENT = max(1, int(os.environ.get("PRAGENT_MAX_CONCURRENT_REVIEWS", "2")))
|
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)))
|
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
|
# 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
|
# untars a repo, reads files and shells out to linters, so an unbounded thread
|
||||||
@@ -143,23 +152,16 @@ def _handle_pull_request(payload: dict) -> tuple[int, str]:
|
|||||||
if not BOT_TOKEN:
|
if not BOT_TOKEN:
|
||||||
return 500, "PRAGENT_BOT_TOKEN not set"
|
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)
|
key = (repo, str(index), sha)
|
||||||
if not _claim(key):
|
if not _claim(key):
|
||||||
return 200, f"ignore (already in flight) {repo}#{index} sha={sha[:8]}"
|
return 200, f"ignore (already in flight) {repo}#{index} sha={sha[:8]}"
|
||||||
|
|
||||||
threading.Thread(
|
threading.Thread(
|
||||||
target=_run_review,
|
target=_run_review,
|
||||||
args=(key, title, body, report_usage, base_ref),
|
args=(key, title, body, base_ref),
|
||||||
daemon=True,
|
daemon=True,
|
||||||
).start()
|
).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:
|
def _claim(key: tuple[str, str, str]) -> bool:
|
||||||
@@ -177,9 +179,30 @@ def _release(key: tuple[str, str, str]) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def _run_review(
|
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:
|
) -> None:
|
||||||
repo, index, sha = key
|
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:
|
try:
|
||||||
with _review_slots:
|
with _review_slots:
|
||||||
ok = review_pr(
|
ok = review_pr(
|
||||||
@@ -194,10 +217,9 @@ def _run_review(
|
|||||||
model=OLLAMA_MODEL,
|
model=OLLAMA_MODEL,
|
||||||
max_tokens=OLLAMA_MAX_TOKENS,
|
max_tokens=OLLAMA_MAX_TOKENS,
|
||||||
max_chars=DIFF_MAX_CHARS,
|
max_chars=DIFF_MAX_CHARS,
|
||||||
report_usage=report_usage,
|
|
||||||
base_ref=base_ref,
|
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
|
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)
|
print(f"pragent-webhook: thread crashed for {repo}#{index}: {e}", flush=True)
|
||||||
finally:
|
finally:
|
||||||
|
|||||||
Reference in New Issue
Block a user