fix(ai_review): show actual model in cost line (was hardcoded glm-5.2:cloud)
The webhook pod now routes through headroom's MiniMax-M2.7 endpoint, but the
cost line in the AI-usage collapsible still read 'headroom glm-5.2:cloud'.
Resolve a single display_model at the top of review_pr (OPENCODE_MODEL env
wins, else headroom/{OLLAMA_MODEL}) and pass it to:
* the opencode subprocess (was already doing this on the same line, now
sharing the value)
* format_review_body so REVIEW_HEADER also reflects the actual run
* _render_collapsible_usage so the parenthetical reads
'({display_model} — free tier)' or '({display_model} — billed)'.
Test additions in tests/pilot/test_ai_review.py cover:
* the parenthetical picks up the passed-in model verbatim
* the full provider prefix survives (headroom/<id>) for the opencode path
* nonzero cost flips the inner clause from 'free tier' to 'billed'
* the existing nonzero-cost assertion flips to assert 'billed' instead
of dropping the parenthetical entirely
This commit is contained in:
+22
-12
@@ -1067,7 +1067,11 @@ def _render_collapsible_usage(usage: dict | None, model: str, config: dict | Non
|
||||
|
||||
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.
|
||||
and its parenthetical clause reflects the *actually-routed* model —
|
||||
typically `OPENCODE_MODEL` if set, otherwise `headroom/{OLLAMA_MODEL}` —
|
||||
not a stale literal. `model` here is the resolved display name (the caller
|
||||
computes it once and passes it everywhere: this helper, `format_review_body`,
|
||||
and the opencode subprocess). Cost == 0 → "free tier"; nonzero → "billed".
|
||||
"""
|
||||
if not usage:
|
||||
return ""
|
||||
@@ -1075,7 +1079,7 @@ def _render_collapsible_usage(usage: dict | None, model: str, config: dict | Non
|
||||
dur_s = f"{dur}s" if dur is not None else "?"
|
||||
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 ""
|
||||
actual_note = f" ({model} — {'free tier' if not actual else 'billed'})"
|
||||
price_key, price_err = _resolve_price_target(config)
|
||||
from cost_model import PRICES
|
||||
eq = equivalent_cost(usage, price_key)
|
||||
@@ -1839,6 +1843,13 @@ def review_pr(
|
||||
Both the CI `run()` entry point and the central webhook server call this.
|
||||
"""
|
||||
try:
|
||||
# Resolve the model display name once and pass it to every consumer
|
||||
# (opencode subprocess, REVIEW_HEADER, cost-line parenthetical). Env
|
||||
# override wins; otherwise we prefix the OLLAMA_MODEL bare id with
|
||||
# the headroom provider so the line reads `headroom/<model>` instead
|
||||
# of a stale hardcoded literal.
|
||||
display_model = os.environ.get("OPENCODE_MODEL") or f"headroom/{model}"
|
||||
|
||||
reviews = fetch_existing_reviews(api, repo, index, token)
|
||||
# Dedupe: already reviewed this exact commit -> nothing to do.
|
||||
if sha and sha in reviewed_shas(reviews):
|
||||
@@ -1847,7 +1858,7 @@ def review_pr(
|
||||
|
||||
raw_diff, _truncated, _orig = fetch_pr_diff(api, repo, index, token, max_chars)
|
||||
if not raw_diff.strip():
|
||||
post_review(api, repo, index, token, format_review_body("No diff content to review.", model, sha))
|
||||
post_review(api, repo, index, token, format_review_body("No diff content to review.", display_model, sha))
|
||||
return True
|
||||
|
||||
config = fetch_repo_config(api, repo, token, ref=base_ref)
|
||||
@@ -1886,10 +1897,9 @@ def review_pr(
|
||||
# the brief, and the pragent agent factory; returns stdout with a
|
||||
# summary + findings JSON. We parse + anchor + post here.
|
||||
import opencode_review # local import keeps the ollama path dep-free
|
||||
# opencode wants a provider-prefixed model ref (headroom/glm-5.2:cloud);
|
||||
# `model` here is the bare id (OLLAMA_MODEL). OPENCODE_MODEL overrides
|
||||
# with the full ref; otherwise we prefix the configured provider.
|
||||
oc_model = os.environ.get("OPENCODE_MODEL") or f"headroom/{model}"
|
||||
# Reuse the display_model resolved above for the subprocess — same
|
||||
# provider-prefixed ref goes to the engine and into the review body.
|
||||
oc_model = display_model
|
||||
# Multi-lens fan-out: when the repo declared `reviewers[]` (or the
|
||||
# operator pinned PRAGENT_REVIEWERS=1), spawn one opencode subprocess
|
||||
# per lens in parallel and synthesize. Falls through to the legacy
|
||||
@@ -1932,10 +1942,10 @@ def review_pr(
|
||||
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, display_model, config=config) if report_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))
|
||||
display_model, sha, usage_section=usage_section))
|
||||
return True
|
||||
else:
|
||||
user_prompt = build_user_prompt(title, body + compression_note, diff, config, prior, additional_context)
|
||||
@@ -1979,7 +1989,7 @@ def review_pr(
|
||||
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, display_model, config=config) if report_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
|
||||
@@ -1998,7 +2008,7 @@ def review_pr(
|
||||
if bullets:
|
||||
summary_parts.append("### Unanchored Notes\n\n" + bullets)
|
||||
summary_body = format_review_body(
|
||||
"\n\n".join(summary_parts), model, sha,
|
||||
"\n\n".join(summary_parts), display_model, sha,
|
||||
summary=review_summary,
|
||||
usage_section=usage_section,
|
||||
summary_changes=summary_changes,
|
||||
@@ -2016,7 +2026,7 @@ def review_pr(
|
||||
return True
|
||||
except Exception as e: # fail-open
|
||||
try:
|
||||
post_review(api, repo, index, token, format_review_body(f"⚠️ AI review failed: {e}", model, sha))
|
||||
post_review(api, repo, index, token, format_review_body(f"⚠️ AI review failed: {e}", display_model, sha))
|
||||
except Exception as e2:
|
||||
print(f"pragent: could not post failure note: {e2}", file=sys.stderr)
|
||||
print(f"pragent: review failed: {e}", file=sys.stderr)
|
||||
|
||||
@@ -771,6 +771,39 @@ def test_render_collapsible_usage_cost_nonzero_drops_free_tier_note():
|
||||
"cache_write": 0, "total": 10, "cost": 0.0123, "steps": 1, "duration_s": 1.0}
|
||||
sec = _render_collapsible_usage(usage, "m", config=None)
|
||||
assert "$0.0123" in sec
|
||||
# Was hardcoded "free tier" previously; now says "billed" since cost > 0.
|
||||
assert "billed" in sec
|
||||
assert "free tier" not in sec
|
||||
|
||||
|
||||
def test_render_collapsible_usage_uses_passed_model_for_free_tier_clause():
|
||||
# Regression: the cost parenthetical must reflect the actually-routed model,
|
||||
# not a stale hardcoded `headroom glm-5.2:cloud` literal that predates the
|
||||
# MiniMax / Anthropic switch.
|
||||
usage = {"input": 10, "output": 0, "reasoning": 0, "cache_read": 0,
|
||||
"cache_write": 0, "total": 10, "cost": 0.0, "steps": 1, "duration_s": 1.0}
|
||||
sec = _render_collapsible_usage(usage, "MiniMax-M2.7", config=None)
|
||||
# The parenthetical clause is "(<model> — free tier)" — a model name MUST
|
||||
# sit immediately before "— free tier".
|
||||
assert "(MiniMax-M2.7 — free tier)" in sec
|
||||
# And the stale hardcoded model name must no longer appear anywhere.
|
||||
assert "glm-5.2:cloud" not in sec
|
||||
|
||||
|
||||
def test_render_collapsible_usage_full_provider_prefix_in_display():
|
||||
# When the caller has resolved a provider-prefixed model ref (the opencode
|
||||
# subprocess path), the parenthetical should mirror that verbatim.
|
||||
usage = {"input": 10, "output": 0, "reasoning": 0, "cache_read": 0,
|
||||
"cache_write": 0, "total": 10, "cost": 0.0, "steps": 1, "duration_s": 1.0}
|
||||
sec = _render_collapsible_usage(usage, "headroom/MiniMax-M2.7", config=None)
|
||||
assert "(headroom/MiniMax-M2.7 — free tier)" in sec
|
||||
|
||||
|
||||
def test_render_collapsible_usage_nonzero_cost_says_billed():
|
||||
usage = {"input": 10, "output": 0, "reasoning": 0, "cache_read": 0,
|
||||
"cache_write": 0, "total": 10, "cost": 0.123, "steps": 1, "duration_s": 1.0}
|
||||
sec = _render_collapsible_usage(usage, "MiniMax-M2.7", config=None)
|
||||
assert "(MiniMax-M2.7 — billed)" in sec
|
||||
assert "free tier" not in sec
|
||||
|
||||
|
||||
@@ -1082,7 +1115,10 @@ def test_usage_block_shows_equivalent_provider_cost():
|
||||
assert "🔋 AI Usage & Run Details" in sec
|
||||
assert "**Est. cost on Claude Sonnet 5**" in sec
|
||||
assert "**Actual**: $0.00" in sec
|
||||
# The "free tier" clause must mention the routed model verbatim, not the
|
||||
# stale hardcoded `headroom glm-5.2:cloud` literal.
|
||||
assert "free tier" in sec
|
||||
assert "glm-5.2:cloud" 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.
|
||||
|
||||
Reference in New Issue
Block a user