diff --git a/pilot/ai_review.py b/pilot/ai_review.py index 9dfee33..b16a568 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -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/` 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) diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index 591bb5e..39acb91 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -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 "( — 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.