feat(input): add diff_compress module + prior-review compaction helpers #9
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: gitea_admin/pragent#9
Reference in New Issue
Block a user
Delete Branch "feat/cost-display-compress-config"
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?
Two pure stdlib helpers that shrink what lands in the model prompt: * compress_diff(diff, *, context=2) — re-renders a unified diff so each hunk keeps only unchanged lines on either side of its +/- lines. File headers + hunk headers + +/- lines preserved verbatim. Pure-context hunks dropped (rare but legal — git emits them on whitespace-only diffs). Collapsed gaps of >=5 lines emit a single '@@ … N context line(s) omitted … @@' marker so the reviewer knows code was elided. Smaller gaps stay silent — the marker would be longer than the elision. * extract_finding_bullets(review_body) — pulls the lines of a prior review that look like a pragent finding (- **[SEVERITY]** path:line — …) and drops everything else. The model already has the diff; repeating the prose is just token burn. No I/O, no network. Tolerant of malformed input — never raises. 14 unit tests cover both helpers, including an anchor-preservation check against parse_diff_anchors to guarantee compress-then-anchor still works. Wiring in ai_review/opencode_review lives in the next commit.The canalhandia PR review lost all findings because the agent ran out of context before emitting the closing json fence. Three failure modes hit the old regex \{.*?\}: * nested objects inside the fence truncated at the first } * bare arrays (no {summary, findings} wrapper) returned [] * unfenced JSON in the prose tail was never reached (first not last) Replace the regex with a balanced-brace scanner: * _last_json_block walks the fence contents with a depth counter so nested objects survive * _last_balanced_json + _balanced_json_substring handle bare arrays and prose-tail JSON when no fence is present * _parse_json_tolerant returns list as well as dict; parse_findings and parse_review_output accept a bare array as the outer value Agent prompt tightened: reserve the final step for emitting the JSON block so the analysis isn't lost when context runs out. 10 new tests in tests/pilot/test_ai_review.py cover the new shapes. Co-Authored-By: Claude <noreply@anthropic.com>PR-level comment layout (per operator's format guide): * Summary of Changes — 2-4 bullets, sourced from the agent's new `summary_changes` JSON field. Falls back to splitting the prose `summary` if the list is missing. * Key Risks & Concerns — bullets from the new `risks` JSON field. * Findings Overview — Markdown table covering every finding (severity emoji / location / one-line problem). Both anchored and unanchored findings appear here so the table is the single scan point. * Unanchored Notes — bullets with severity + fix + Markdown-linked ref, for findings with no post-change line to anchor. * AI Usage & Run Details — wrapped in a <details>/<summary> collapsible so the body stays scannable. Cost line stays inside it. Inline comment shape: * Severity badge: 🔴 [HIGH] / 🟡 [MEDIUM] / 🔵 [LOW] / ⚪ [INFO]. Unknown severities fall back to � [INFO]. * 1-2 short paragraphs of problem; **Fix:** label for the fix line. * Standard ```suggestion fence for replacement code (Gitea/Forgejo apply-on-click). Language-tagged fences are no longer used for single-file diffs. * Reference as a Markdown hyperlink, visible label truncated to <=60 chars; the underlying URL is preserved verbatim. * NO per-comment 🪙 token attribution. All telemetry stays in the collapsible block on the PR-level comment. Agent prompt updated to emit `summary_changes` and `risks` in the JSON output (backward-compatible — older outputs missing them still parse; they fall back to splitting the prose `summary`). Tests: 15 new (severity emoji mapping, reference truncation, findings table escaping, collapsible usage rendering, summary_changes+risks layout). Existing tests updated for the new structure. Co-Authored-By: Claude <noreply@anthropic.com>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 <noreply@anthropic.com>On by default, opt-out via "reviewers": []. 290 tests pass. - pilot/opencode_review.py: ReviewerSpec dataclass, default_reviewers(), parse_reviewers_config(), parse_triage_config(), resolve_reviewers(), _normalize_lens_finding(), posthash() (matches feedback.py scheme), _agreement_hash() (severity-free for cross-lens promotion), _tone_strip(), synthesize() 7-stage (severity_floor → tone-strip → length cap → per-lens max → per-file cap → dedup by _posthash → cross-lens severity promote → per-PR cap), run_lenses() (ThreadPoolExecutor pool=4), triage(), _intersect_with_triage(), _filter_by_skip_if(), run_lenses_review(). run() routes to fan-out when config.reviewers[] present or PRAGENT_REVIEWERS=1. - pilot/ai_review.py: parse_repo_config learns reviewers[] and triage objects (id regex /^[a-z0-9][a-z0-9-]{0,31}$/, 8-entry cap, agent_file/model/ severity_floor/max_findings/activation/skip_if_all_changed_paths/hotpath_globs). review_pr branches to opencode_review.run_lenses_review when configured. _render_collapsible_usage shows lenses: ... line when present. - .opencode/agents/{docs,code-quality,triage}.md: 3 new lens subagents. - .opencode/skills/lens-orchestration/SKILL.md: strict-JSON contract every lens subagent MUST honor. - .opencode/agents/pragent.md: slim to coordinator; no more hardcoded @security/@tests/@perf delegation; loads lens-orchestration skill. - .opencode/README.md: rewrite 'Add a review lens' recipe for multi-lens. - pilot/README-webhook.md: new 'Multi-lens pipeline' section (diagram + default roster + config schema + env vars + cross-lens dedup contract). - tests: 36 new tests (test_ai_review.py +12 reviewers/triage/usage, test_opencode_review.py +24 orchestration). posthash golden-vector matches feedback.py exactly across 5 severity × 2 line cases.🤖 AI Review · pragent pilot · glm-5.2:cloud ·
78bcf6a9Summary of Changes
Key Risks & Concerns
Findings Overview
1 inline comment(s); 2 total.
pilot/ai_review.py:1993compress_diff(pilot/diff_compress.py) drops context lines but copies the original@@ -a,b +c,d @@header verbatim, so the header's line numbers no longer match the kept lines.parse_diff_anchors(diff)then walks those stale headers and produces anchor sets that are wrong relative to the real file. Verified: a 5-line hunk@@ -1,5 +1,5 @@with context=1 yields anchors{1,2,3}instead of{1,2,3,4,5}. The agent reads the same stale header, reports line numbers consistent with the compressed view, and those numbers validate against the wrong anchor set — so inline comments are posted to Gitea at real file lines that don't correspond to the code the agent meant, or get demoted to summary bullets.pilot/opencode_review.py:3083{"lenses":[]}meaning 'no lenses needed — skip the fan-out', but_intersect_with_triagereturnslist(reviewers)(all lenses) whenselected_idsis empty, andrun_lenses_reviewonly skips filtering whenselected is None. So a triage verdict of 'no lenses' silently runs every lens instead of short-circuiting — wasting N subprocess runs and producing findings on a PR triage said to have no surface.Unanchored Notes
pilot/opencode_review.py:3083— Triage's empty-list contract is not honored. The triage agent is documented to emit{"lenses":[]}meaning 'no lenses needed — skip the fan-out', but_intersect_with_triagereturnslist(reviewers)(all lenses) whenselected_idsis empty, andrun_lenses_reviewonly skips filtering whenselected is None. So a triage verdict of 'no lenses' silently runs every lens instead of short-circuiting — wasting N subprocess runs and producing findings on a PR triage said to have no surface.[]from triage-returned-None: let[]mean 'no lenses' (return early with empty findings) andNonemean 'triage failed, fail open to all'.@@ -969,3 +1991,3 @@usage_section = format_usage_section(usage, findings, model)usage_section = _render_collapsible_usage(usage, model, config=config) if report_usage else ""anchors = parse_diff_anchors(diff)🔴 [HIGH] Inline comments are anchored against the COMPRESSED diff.
compress_diff(pilot/diff_compress.py) drops context lines but copies the original@@ -a,b +c,d @@header verbatim, so the header's line numbers no longer match the kept lines.parse_diff_anchors(diff)then walks those stale headers and produces anchor sets that are wrong relative to the real file.Verified: a 5-line hunk
@@ -1,5 +1,5 @@with context=1 yields anchors{1,2,3}instead of{1,2,3,4,5}. The agent reads the same stale header, reports line numbers consistent with the compressed view, and those numbers validate against the wrong anchor set — so inline comments are posted to Gitea at real file lines that don't correspond to the code the agent meant, or get demoted to summary bullets.Fix: Recompute the hunk header line counts in compress_diff after trimming (track kept context/+ lines and rewrite
@@ -x,y +x,z @@), or run parse_diff_anchors on the raw (uncompressed) diff instead.🤖 AI Review · pragent pilot · glm-5.2:cloud ·
2b1cf750Summary of Changes
Key Risks & Concerns
Findings Overview
1 inline comment(s); 1 total.
pilot/ai_review.py:1922apply_repo_configis called withchanged_pathsbuilt from the flagged findings'pathfields (lines 1915-1919:sorted({f.get('path') for f in findings})), not the actual diff's changed files. Therequire_testsbranch in apply_repo_config (line 1414) checksnon_test and not any_testagainst this list. When the agent produces no findings on a clean PR,changed_pathsis empty, so the synthetic 'no test file changed' finding is never appended — the feature is disabled for exactly the case it is meant to catch. The docstring at line 1397 says 'caller passes changed_paths from the brief', but the caller passes findings' paths instead. The correct source ischanged_files(diff)(orchanged_files(raw_diff)), which already exists in pilot/opencode_review.py and is used by the multi-lens path.🔋 AI Usage & Run Details
glm-5.2:cloud· opencode · 32 steps · 242.8s@@ -967,0 +1919,4 @@})except Exception:changed_paths = []kept, _dropped = apply_repo_config(findings, config, changed_paths=changed_paths)🔴 [HIGH] require_tests silently no-ops on clean PRs.
apply_repo_configis called withchanged_pathsbuilt from the flagged findings'pathfields (lines 1915-1919:sorted({f.get('path') for f in findings})), not the actual diff's changed files. Therequire_testsbranch in apply_repo_config (line 1414) checksnon_test and not any_testagainst this list. When the agent produces no findings on a clean PR,changed_pathsis empty, so the synthetic 'no test file changed' finding is never appended — the feature is disabled for exactly the case it is meant to catch. The docstring at line 1397 says 'caller passes changed_paths from the brief', but the caller passes findings' paths instead. The correct source ischanged_files(diff)(orchanged_files(raw_diff)), which already exists in pilot/opencode_review.py and is used by the multi-lens path.Fix: Derive changed_paths from the diff via changed_files(raw_diff) (import or inline the helper) before calling apply_repo_config, not from the findings' path fields.
🪙 ~9304 tok (100% · attributed output)