From 998f793ec28a550c762df462b149beb439b63bbd Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 20 Aug 2026 17:35:06 +0000 Subject: [PATCH] feat(agent): tighten prompt to bound beyond-diff reads + de-generalize cost-model labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes from operator feedback: 1. Per-comment � attribution restored on inline comments (operator wants it back — the PR-level collapsible is collapsed by default, so the attribution is the visible signal of per-finding cost share). Hidden only when no _tok_attrib was computed (legacy callers / ollama path without usage metering). 2. Agent prompt now bounds reads beyond the diff — the single biggest driver of input-token bloat on long agent loops: * ≤ 5 file reads beyond the diff for the entire review * ≤ 80 lines per read (use --offset + --limit) * ≤ 3 grep calls beyond the diff (prefer rtk grep) * no re-reads of files already seen * no directory walks (ls -R, find .) * honor .pr-review.json:exclude_paths 3. De-generalize cost_model calibration labels. The OBSERVED_RUNS list referred to `gitea_admin/pragent#7` — a real internal repo path that blocks commercialization. Replaced with `internal/hardening-PR (16 files, 1020 insertions / 91 deletions)`. The numbers (input/output tokens, steps, duration) are unchanged — only the labels are generic. Tests: * test_inline_comment_body_with_attribution_line — asserts 🪙 line shows when _tok_attrib is set * test_inline_comment_body_no_attribution_no_coin_line — still verifies the line is hidden when no attribution data * test_observed_report_prices_every_model — asserts no internal repo name appears in the rendered report Co-Authored-By: Claude --- .opencode/agents/pragent.md | 26 ++++++++++++++++++-------- pilot/ai_review.py | 10 ++++++++-- pilot/cost_model.py | 4 ++-- tests/pilot/test_ai_review.py | 29 +++++++++++++++++------------ tests/pilot/test_cost_model.py | 4 +++- 5 files changed, 48 insertions(+), 25 deletions(-) diff --git a/.opencode/agents/pragent.md b/.opencode/agents/pragent.md index a013f7c..c530cb7 100644 --- a/.opencode/agents/pragent.md +++ b/.opencode/agents/pragent.md @@ -85,14 +85,24 @@ read the full file around a flagged line, not just the diff hunk. lists the changed files explicitly under "Changed files" — use that as your focus list. -3. **Ground findings in context.** For each changed file, before finalizing any - finding, `read`/`grep` its **callers, imports, sibling functions, and type - definitions** so your findings reflect how the change is actually used, not - the hunk in isolation. The repo is checked out at the head sha, so the - surrounding code is on disk — use it. Keep it bounded: stop exploring a file - once the finding is grounded (1–3 related files per finding); do NOT do - unbounded whole-repo walks (token cost, and the focus is the diff's - neighbourhood). +3. **Ground findings in context — but stay bounded.** For each changed file, + before finalizing any finding, `read`/`grep` its **callers, imports, sibling + functions, and type definitions** so your findings reflect how the change + is actually used, not the hunk in isolation. The repo is checked out at the + head sha, so the surrounding code is on disk — use it. + + HARD budget on reads beyond the diff (this is the single biggest driver of + token cost on long agent loops): + * ≤ 5 file reads BEYOND the diff for the entire review. Count them. + * ≤ 80 lines per `read` call — use `read --offset N --limit 80` to slice + large files; never `cat` a whole 1000-line file. + * ≤ 3 grep calls beyond the diff (use `rtk grep` if available; `grep -n` + with a precise pattern otherwise). + * Do NOT re-read a file you've already seen. The diff is the source of + truth — re-reads only confirm what you already know. + * Do NOT walk directories (`ls -R`, `find .`) — list explicitly. + * Honour `.pr-review.json:exclude_paths` — those files do not exist for + you; do not read them even if they appear in the diff. 4. **Run the repo's own checks via bash.** Detect tooling and run it on the CHANGED files only (keep it fast, keep tokens low): diff --git a/pilot/ai_review.py b/pilot/ai_review.py index b00bd1f..c80eb7a 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -1002,8 +1002,10 @@ def inline_comment_body(f: dict) -> str: produced replacement code. Language-tagged fences are reserved for cross-file patterns the suggestion block can't carry. * Reference as a Markdown hyperlink (``[label](url)``) — never a raw URL. - * No per-comment token attribution: the PR-level collapsible carries - all telemetry; inline comments stay focused on the code. + * Per-comment attributed output tokens (`🪙 ~N tok (P% · attributed)`) + when the caller passed `compute_attribution` data. Hidden when the + finding has no attributed tokens (e.g. legacy callers / ollama path + without usage metering). """ badge = _severity_badge(f.get("severity", "medium")) body = f"{badge} {f.get('problem', '').strip()}" @@ -1019,6 +1021,10 @@ def inline_comment_body(f: dict) -> str: ref_md = _format_reference(f.get("reference", "")) if ref_md: body += f"\n\n🔗 **Reference:** {ref_md}" + tok = f.get("_tok_attrib") + if tok is not None: + pct = (f.get("_tok_pct", 0.0) or 0.0) * 100 + body += f"\n\n🪙 ~{tok} tok ({pct:.0f}% · attributed output)" return body diff --git a/pilot/cost_model.py b/pilot/cost_model.py index 372d002..e0df5c3 100644 --- a/pilot/cost_model.py +++ b/pilot/cost_model.py @@ -176,7 +176,7 @@ DEFAULT_TIERS = [ # from a guess, and the first entry corrected the tier assumptions by ~15x. OBSERVED_RUNS: list[dict] = [ { - "label": "gitea_admin/pragent#7 (the hardening PR)", + "label": "internal/hardening-PR (16 files, 1020 insertions / 91 deletions)", "date": "2026-08-18", "tier": "full", "diff_tokens": 17_600, # 16 files, 1020 insertions / 91 deletions @@ -189,7 +189,7 @@ OBSERVED_RUNS: list[dict] = [ "subagents": 0, }, { - "label": "gitea_admin/pragent#7 (+ cost-model calibration + salvage fix)", + "label": "internal/hardening-PR (same PR, two commits later)", "date": "2026-08-18", "tier": "full", "diff_tokens": 21_000, # same PR, two commits later diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index 80664e7..98bc76a 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -326,15 +326,18 @@ def test_inline_comment_body_severity_emoji_mapping(): assert badge in inline_comment_body(f), f"{sev} → {badge}" -def test_inline_comment_body_no_token_attribution(): - # Per spec: no per-comment 🪙 token attribution line. +def test_inline_comment_body_with_token_attribution(): + # Operator wants per-comment attribution back: every inline comment shows + # the attributed output tokens + share of total. Hidden only when no + # attribution data was computed (legacy callers / ollama path without + # usage metering). f = {"severity": "high", "path": "a", "line": 1, "problem": "p", "fix": "f", "suggestion": "", "reference": "", - "_tok_attrib": 1234, "_tok_pct": 0.3} + "_tok_attrib": 1234, "_tok_pct": 0.30} body = inline_comment_body(f) - assert "🪙" not in body - assert "tok" not in body.lower().split("fix")[0] # only in fix is OK - assert "attributed" not in body + assert "🪙 ~1234 tok" in body + assert "30%" in body + assert "attributed output" in body def test_summary_bullets_format(): @@ -673,15 +676,17 @@ def test_compute_attribution_noop_on_empty_or_zero_budget(): assert "_tok_attrib" not in fs[0] -def test_inline_comment_body_no_attribution_line(): - # Per spec: NO per-comment token attribution — that telemetry lives in the - # collapsible block on the PR-level comment. +def test_inline_comment_body_with_attribution_line(): + # Operator wants per-comment attribution back: every inline comment shows + # the attributed output tokens + share of total. Hidden only when no + # attribution data was computed (legacy callers / ollama path without + # usage metering). f = {"severity": "high", "path": "a", "line": 1, "problem": "bad", "fix": "swap", "suggestion": "", "_tok_attrib": 180, "_tok_pct": 0.29} body = inline_comment_body(f) - assert "🪙" not in body - assert "tok" not in body - assert "attributed" not in body + assert "🪙 ~180 tok" in body + assert "29%" in body + assert "attributed output" in body def test_inline_comment_body_no_attribution_no_coin_line(): diff --git a/tests/pilot/test_cost_model.py b/tests/pilot/test_cost_model.py index 973286d..841b54a 100644 --- a/tests/pilot/test_cost_model.py +++ b/tests/pilot/test_cost_model.py @@ -226,7 +226,9 @@ def test_observed_report_prices_every_model(): text = cm.observed_report(["claude-opus-5", "gpt-5.6-luna"]) assert "Claude Opus 5" in text assert "GPT-5.6 Luna" in text - assert "pragent#7" in text + # Labels are generic (no internal repo names) for commercialization. + assert "gitea_admin" not in text + assert "internal/hardening-PR" in text def test_model_is_within_an_order_of_magnitude_of_the_measurement():