feat: token-usage reporting (AI-USAGE label) #6
Reference in New Issue
Block a user
Delete Branch "feat/token-usage"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
E2E + deliverable PR. Adds per-review + per-comment token accounting, gated by the new AI-USAGE label. See commit. Label both AI-REVIEW + AI-USAGE to see the usage section + per-comment 🪙 lines.
🤖 AI Review · pragent pilot · glm-5.2:cloud ·
08783456Adds token-usage reporting (per-review totals + per-comment attributed 🪙 lines) gated by a new AI-USAGE label, plus a changed-files focus list in the brief and a broadened webhook action gate (allowlist→denylist). Diff is ~340 lines of Python across the review core, opencode engine, webhook server, and tests; not security-sensitive. All 68 tests pass and files parse cleanly. Three issues: a parser contract violation that can abort a review, a hardcoded provider cost note, and missing tests for the changed webhook gate logic.
🔋 AI usage
glm-5.2:cloud· engine: opencode · agent steps: 23 · duration: 268.4spilot/webhook_server.py:48pilot/opencode_review.py:324pilot/ai_review.py:2133 inline comment(s) posted below.
@@ -161,0 +210,4 @@dur_s = f"{dur}s" if dur is not None else "?"cost = usage.get("cost") or 0.0cost_s = f"${cost:.4f}" if cost else "$0.00"cost_note = ([LOW] cost_note hardcodes "(on-network glm-5.2:cloud via headroom — no per-token charge)" keyed off
not costrather than the actual provider/model, so it mislabels any non-headroom model, and also mislabels a billed provider whose run cost $0.0000 (e.g. cached tokens) as "no per-token charge".Fix: Drop the provider-specific note, or derive it from the model/provider config instead of the cost value; at minimum key it on the model name passed in, not on
not cost.🪙 ~5330 tok (35% · attributed output)
@@ -236,0 +321,4 @@if isinstance(tok, dict):saw_step = Trueusage["steps"] += 1usage["input"] += int(tok.get("input") or 0)[MEDIUM] int(tok.get("input") or 0) raises ValueError when a token field is a non-int string (e.g. "90.5"), violating the docstring's "Never raises" promise; run_opencode trusts that contract and a single bad event line aborts the whole parse, losing the assistant text.
Fix: Wrap each token coercion in a try/except (or use a safe _to_int helper that falls back to 0 on TypeError/ValueError) so malformed values are skipped, not fatal.
🪙 ~5043 tok (33% · attributed output)
@@ -46,0 +45,4 @@# 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`.SKIP_ACTIONS = {"closed"}[HIGH] No tests cover the changed webhook gate logic: the _labels_have refactor, SKIP_ACTIONS denylist, AI_USAGE_LABEL detection, and report_usage plumbing are all new/changed with zero test coverage (no webhook tests exist in the repo). Gate logic is exactly what needs tests.
Fix: Add tests/pilot/test_webhook_server.py covering _labels_have (dict + bare-string labels), the SKIP_ACTIONS gate (closed skipped, other actions pass), AI-USAGE label detection, and report_usage env-override + label detection.
🪙 ~4727 tok (31% · attributed output)
🤖 AI Review · pragent pilot · glm-5.2:cloud ·
513de0e3Adds opt-in token-usage reporting (AI-USAGE label): measured review totals + attributed per-comment 🪙 lines, via opencode --format json NDJSON parsing. Also broadens the webhook from an action allowlist to a
closed-only denylist. Tests pass (68) and ruff is clean of new diagnostics. Risk is low-medium: the denylist broadening is the riskiest behavioral change and has no test coverage; attribution rounding can make the per-finding table not sum to the stated output total.🔋 AI usage
glm-5.2:cloud· engine: opencode · agent steps: 15 · duration: 210.8spilot/webhook_server.py:100pilot/ai_review.py:193pilot/ai_review.py:9122 inline comment(s) posted below.
pilot/ai_review.py:912— The CIrun()entry point callsreview_prwithoutreport_usage, so AI-USAGE reporting only works via the webhook path — but the README implies the feature is generally available and the CI path is described as reusing the same review core. — fix: Either passreport_usage=bool(os.environ.get("PRAGENT_USAGE_ALWAYS"))inrun()so CI can opt in, or note in the README that AI-USAGE is webhook-only.@@ -161,0 +190,4 @@f["_tok_pct"] = 1.0 / len(findings)returnfor f, w in zip(findings, weights):f["_tok_attrib"] = int(round(output_tokens * w / total_w))[LOW]
int(round(output_tokens * w / total_w))per finding can make the per-finding≈out toktable column sum to a value ≠ the stated measured output total (off by 1–N), so the table is internally inconsistent with the headline output figure.Fix: Compute all but the last finding's attribution by rounding, then set the last finding's to
output_tokens - sum(others)so the column always sums to the measured total.🪙 ~2603 tok (45% · attributed output)
@@ -88,3 +98,3 @@repo = repo_obj.get("full_name") or ""if action not in REVIEW_ACTIONS:if action in SKIP_ACTIONS:[MEDIUM] The allowlist→denylist broadening (fire on every action except
closed) is the riskiest behavioral change in the PR but has zero test coverage — a regression here could fire on merged PRs or cause review storms, and the safety argument (sha-dedupe + label gate) is never verified by a test.Fix: Add tests for
_handle_pull_request:closedis skipped, a non-closedaction with AI-REVIEW label proceeds, and anunlabeledpayload whose labels no longer contain AI-REVIEW is skipped.🪙 ~1733 tok (30% · attributed output)
🤖 AI Review · pragent pilot · glm-5.2:cloud ·
d3dcd9c8Adds opt-in token-usage reporting (AI-USAGE label): measured review totals + attributed per-comment 🪙 lines, via opencode --format json NDJSON parsing, plus a changed-files focus list in the brief and a webhook gate broadened from an action allowlist to a closed-only denylist. ~340 lines of Python; 68 tests pass and files compile cleanly. Risk is low — the two prior reviews already raised the main issues (untested webhook denylist, hardcoded cost-note model name, CI run() not passing report_usage, attribution rounding). Two new minor findings: a regex false-positive on diff body lines starting with
+++ b/, and a misleading0 totalwhen the opencode event omits the total field.2 inline comment(s) posted below.
@@ -145,1 +147,4 @@# Matches unified-diff new-file path headers: `+++ b/path` (and `+++ /dev/null`# for deletions, which we skip). Captures the path after the `b/` prefix._NEW_FILE_HEADER_RE = re.compile(r"^\+\+\+ b/(.+?)\s*$")[LOW] changed_files() scans every line of the raw diff for
+++ b/without distinguishing file headers from hunk body lines, so an added diff line whose content begins with+++ b/(e.g. a patch that itself contains diff text) is falsely extracted as a changed file and injected into the brief's focus list.Fix: Only treat a
+++ b/line as a header when it is a real file header — i.e. not preceded by a hunk+content marker. Track whether a hunk is open and skip matches inside hunk bodies, or require the line to follow adiff --githeader.@@ -236,0 +328,4 @@if isinstance(cache, dict):usage["cache_read"] += int(cache.get("read") or 0)usage["cache_write"] += int(cache.get("write") or 0)usage["total"] += int(tok.get("total") or 0)[LOW] usage["total"] is only summed from an explicit
totalfield in the event; if opencode ever emits a step_finish with input/output but nototal, the usage section reports0 totalalongside non-zero in/out figures, which is misleading.Fix: Fall back to input+output+reasoning when
totalis absent, or derive total as the sum of the components when the field is missing.