Files
pragent/tests/pilot/test_diff_compress.py
T
Marcos 2b1cf750b7 fix(review): correct diff-compression line numbers, prior-review dedupe, triage skip
Four defects, all found reviewing PR #9 (two of them by pragent-bot's own
review of that PR, which the anchoring bug then misplaced):

* compress_diff dropped context lines but copied the original `@@` hunk
  header verbatim, so the header no longer described the lines beneath it.
  parse_diff_anchors then walked stale headers and produced anchor sets
  shifted by the number of elided lines, misplacing inline comments or
  demoting them to bullets. Each surviving run of lines is now re-emitted as
  its own hunk with a recomputed `@@ -a,b +c,d @@`, so the output stays a
  valid unified diff whose numbers describe the real post-change file. The
  pseudo-marker `@@ … N context line(s) omitted … @@` is gone; it parsed as
  a hunk header and reset the anchor counter to 0. Anchoring additionally
  runs on the raw diff now, so the prompt window can never shrink the
  anchorable set.

* compress_diff's `_FILE_HEADER` regex matched diff *body* lines: a removed
  YAML `---` separator or an added `++` line was read as a file header,
  truncating the hunk and dropping its `@@` header with it. Body detection
  is now prefix-based, with a full-shape hunk-header regex.

* extract_finding_bullets could not match the bullets pragent itself posts:
  summary_bullets renders an emoji severity badge between the `-` and the
  `[SEV]` tag, which the regex rejected, so compact_prior_reviews always
  returned [] and every re-review repeated its previous findings.

* triage returning `{"lenses":[]}` — documented in .opencode/agents/triage.md
  as "no lens has surface, skip the fan-out" — ran every lens instead, since
  _intersect_with_triage mapped an empty selection to "all" and the call site
  had a second `or reviewers` fallback. `[]` and None are now distinct
  outcomes: `[]` skips, None fails open. A roster naming only unknown lens
  ids now fails open rather than silencing the review. The skip path returns
  a well-formed empty-findings response instead of "", which had landed in
  ai_review's unparseable-output branch and posted "AI review produced no
  parseable output" — a malfunction message for a normal verdict.

Also: non-URL references (a CVE id, a doc title) rendered as
`[CVE-2024-1234](CVE-2024-1234)`, a broken relative link in Gitea — now
plain text. PRAGENT_DIFF_CONTEXT and friends parse through _int_env, so a
typo logs and falls back instead of killing a review mid-flight. Removed
format_usage_section, dead since the collapsible usage block replaced it and
carrying a duplicate copy of the price-target logic.

Tests: 290 -> 301. New coverage for hunk-header fidelity before/after
compression, header-shaped content lines, the bullet round-trip against the
real renderer, and triage's three outcomes (previously untested).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B11e8TZZxJyzHW7jj7KWUN
2026-08-20 23:05:03 +00:00

339 lines
12 KiB
Python

"""Unit tests for pragent pilot diff_compress. No network."""
import os
import re
import sys
HERE = os.path.dirname(os.path.abspath(__file__))
ROOT = os.path.abspath(os.path.join(HERE, "..", ".."))
sys.path.insert(0, os.path.join(ROOT, "pilot"))
import diff_compress # noqa: E402
from diff_compress import compress_diff, extract_finding_bullets # noqa: E402
# ---------------------------------------------------------------------------
# compress_diff
# ---------------------------------------------------------------------------
_DIFF = """\
diff --git a/src/a.py b/src/a.py
index 1..2 100644
--- a/src/a.py
+++ b/src/a.py
@@ -1,20 +1,21 @@
ctx1
-removed
+added
ctx2
ctx3
ctx4
ctx5
ctx6
ctx7
ctx8
ctx9
ctx10
ctx11
ctx12
ctx13
ctx14
ctx15
ctx16
+extra
ctx17
@@ -20,3 +21,4 @@
tail1
tail2
+tail3
tail4
diff --git a/binary.bin b/binary.bin
new file mode 100644
index 0..1
Binary files differ
"""
def test_compress_diff_default_context_two():
text, orig, kept = compress_diff(_DIFF, context=2)
# +/- lines preserved
assert "+added" in text
assert "-removed" in text
assert "+extra" in text
assert "+tail3" in text
# 2 context lines around +/- kept, the rest collapsed
assert "ctx2" in text and "ctx3" in text
assert "ctx4" not in text # outside the +/- window
# Binary files pass through
assert "Binary files differ" in text
# File headers preserved
assert "diff --git a/src/a.py b/src/a.py" in text
assert orig > kept
def test_compress_diff_context_zero_strips_context():
text, orig, kept = compress_diff(_DIFF, context=0)
assert "+added" in text and "-removed" in text and "+extra" in text
# Context lines dropped (only +/- survive)
assert " ctx1" not in text
assert "ctx2" not in text
assert orig > kept
def test_compress_diff_negative_disables_compression():
text, orig, kept = compress_diff(_DIFF, context=-1)
assert text == _DIFF
assert orig == kept
def test_compress_diff_collapsed_gap_splits_into_two_hunks():
# Two +/- lines separated by 14 context lines, context=2. The dropped
# middle is expressed by SPLITTING the hunk in two, each with a recomputed
# `@@` header — not by a pseudo-marker line. `parse_diff_anchors` reads
# `@@` headers to reset its line counter, so anything that looks like a
# header but isn't one silently misanchors every following comment.
middle = "\n".join(f" m{i}" for i in range(14)) + "\n" # trailing \n!
diff = (
"diff --git a/x.py b/x.py\n"
"--- a/x.py\n"
"+++ b/x.py\n"
"@@ -1,21 +1,23 @@\n"
+ " c1\n c2\n" # ctx near +a (kept with context=2)
+ "+a\n"
+ middle
+ "+b\n"
+ " c1\n c2\n" # ctx near +b (kept with context=2)
)
text, _, _ = compress_diff(diff, context=2)
assert "+a" in text and "+b" in text
for m in ("m2", "m3", "m4", "m5", "m6", "m7", "m8", "m9", "m10", "m11"):
assert f" {m}\n" not in text # the gap itself is gone
# Two hunks, and every emitted header is a real unified-diff header.
headers = [ln for ln in text.splitlines() if ln.startswith("@@")]
assert len(headers) == 2
assert all(re.match(r"^@@ -\d+,\d+ \+\d+,\d+ @@", h) for h in headers)
def test_compress_diff_strips_no_newline_marker():
diff = (
"diff --git a/x.py b/x.py\n"
"--- a/x.py\n"
"+++ b/x.py\n"
"@@ -1,2 +1,2 @@\n"
" a\n"
"-b\n"
"\\ No newline at end of file\n"
"+c\n"
"\\ No newline at end of file\n"
)
text, _, _ = compress_diff(diff, context=2)
assert "\\ No newline" not in text
assert "-b" in text and "+c" in text
def test_compress_diff_empty_and_none():
text, orig, kept = compress_diff("", context=2)
assert text == ""
assert orig == 0 and kept == 0
text, orig, kept = compress_diff(None, context=2) # type: ignore[arg-context]
assert text == ""
assert orig == 0 and kept == 0
def test_compress_diff_pure_context_hunk_drops_body():
# A hunk that's *only* context lines (rare but legal — `git diff` emits
# these when the post-image differs only in whitespace outside the visible
# hunk) collapses entirely: file headers stay, the empty hunk header
# itself drops. The reviewer doesn't need to re-read unchanged code.
diff = (
"diff --git a/x.py b/x.py\n"
"--- a/x.py\n"
"+++ b/x.py\n"
"@@ -1,3 +1,3 @@\n"
" a\n"
" b\n"
" c\n"
)
text, _, _ = compress_diff(diff, context=2)
assert text == "diff --git a/x.py b/x.py\n--- a/x.py\n+++ b/x.py\n"
assert "@@ -1,3" not in text # empty hunk header dropped
def test_compress_diff_wide_window_keeps_more_context():
narrow, _, _ = compress_diff(_DIFF, context=0)
wide, _, wide_kept = compress_diff(_DIFF, context=10)
assert wide_kept > len(narrow)
# ---------------------------------------------------------------------------
# extract_finding_bullets
# ---------------------------------------------------------------------------
_BODY = """\
🤖 **AI Review** · pragent pilot · glm-5.2:cloud · `abcdef12`
Adds the salvavoid void-death item-rescue module. Risk is moderate on the
PlayerDeathEvent item/inventory path. New findings (not in prior review):
orphaned chest left in world on rescue failure, missing module-enabled check.
- **[HIGH]** `src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java:162` — drop duplication race. fix: use ItemMeta to write inventory once. (ref: https://example.com)
- **[MEDIUM]** `src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java:67` — O(n^2) spiral. fix: cap radius. (https://example.com/spiral)
- **[LOW]** `src/main/java/dev/marcospaulo/canalhandia/VoidProtection.java:3` — package-info javadoc missing.
_4 inline comment(s) posted below._
<!-- pragent:sha=abcdef1234567890 -->
"""
def test_extract_finding_bullets_basic():
bs = extract_finding_bullets(_BODY)
assert len(bs) == 3
assert any("HIGH" in b and "VoidProtection.java:162" in b for b in bs)
assert any("MEDIUM" in b for b in bs)
assert any("LOW" in b for b in bs)
def test_extract_finding_bullets_drops_prose():
bs = extract_finding_bullets(_BODY)
joined = "\n".join(bs)
# The summary prose is dropped.
assert "Adds the salvavoid" not in joined
assert "PlayerDeathEvent item/inventory path" not in joined
# The inline-comment footer is dropped.
assert "inline comment(s) posted below" not in joined
# The sha marker is dropped.
assert "pragent:sha=" not in joined
def test_extract_finding_bullets_accepts_lowercase_summary_bullets():
# `summary_bullets` renders `- **[HIGH]**` (bold); older reviews used
# `- [high]` (plain). Both should match.
text = (
"- [critical] `a.py:1` — bug. fix: fix it.\n"
"- **[HIGH]** `b.go:9` — race.\n"
)
bs = extract_finding_bullets(text)
assert len(bs) == 2
assert "CRITICAL" in bs[0].upper() or "critical" in bs[0]
assert "HIGH" in bs[1]
def test_extract_finding_bullets_empty_and_prose_only():
assert extract_finding_bullets("") == []
assert extract_finding_bullets(" \n \n") == []
assert extract_finding_bullets("Just some prose, no bullets here.") == []
assert extract_finding_bullets("- This is a regular bullet, not a finding.") == []
def test_extract_finding_bullets_keeps_indented_subbullets():
# A finding may carry continuation lines below it (rare in pragent output
# but legal). We only pull the matching line itself — sub-bullets stay
# with their parent as prose.
text = (
"- **[HIGH]** `a.py:1` — bug.\n"
" sub-bullet continuation that the reviewer wrote\n"
"- **[LOW]** `b.go:2` — nit.\n"
)
bs = extract_finding_bullets(text)
assert len(bs) == 2
assert all("sub-bullet continuation" not in b for b in bs)
def test_compress_diff_preserves_anchors_for_post_change_lines():
# Sanity: a finding anchored on a context line that compress_diff keeps
# must still be a valid anchor after compression. We re-run the parser the
# ai_review core uses, so a regression here surfaces as misanchored
# inline comments in production.
import ai_review
diff = (
"diff --git a/x.py b/x.py\n"
"--- a/x.py\n"
"+++ b/x.py\n"
"@@ -10,4 +10,5 @@\n"
" ctx_a\n"
" ctx_b\n"
"+new\n"
" ctx_c\n"
" ctx_d\n"
)
text, _, _ = compress_diff(diff, context=1)
anchors = ai_review.parse_diff_anchors(text)
assert 12 in anchors["x.py"] # +new
# ctx_a is within 1 line of +new at line 12, so kept.
assert 11 in anchors["x.py"]
def test_compress_diff_keeps_post_change_line_numbers_exact():
# The regression that motivated the hunk-header rewrite: dropping context
# lines without renumbering shifted every anchor. Here `+new` really is
# line 10 of the post-change file; compression must not move it.
raw = (
"diff --git a/x.py b/x.py\n"
"--- a/x.py\n"
"+++ b/x.py\n"
"@@ -1,12 +1,12 @@\n"
+ "".join(f" l{i}\n" for i in range(1, 10))
+ "-old\n"
+ "+new\n"
+ " l11\n"
)
import ai_review
raw_anchors = ai_review.parse_diff_anchors(raw)["x.py"]
assert 10 in raw_anchors # +new
text, _, _ = compress_diff(raw, context=1)
comp_anchors = ai_review.parse_diff_anchors(text)["x.py"]
# Compression only ever drops anchors; it never invents or moves one.
assert comp_anchors <= raw_anchors
assert 10 in comp_anchors # +new still anchors to its real line
def test_compress_diff_content_line_starting_with_dashes_is_not_a_header():
# A removed YAML document separator renders as `----`; an added one as
# `+++new`. Treating those as file headers truncated the hunk body and
# dropped the `@@` header with it.
diff = (
"diff --git a/x.yml b/x.yml\n"
"--- a/x.yml\n"
"+++ b/x.yml\n"
"@@ -1,4 +1,4 @@\n"
" a: 1\n"
" b: 2\n"
"----\n"
"+++new\n"
" c: 3\n"
)
text, _, _ = compress_diff(diff, context=1)
assert "----" in text and "+++new" in text
# The hunk header survives, so the body is still anchorable.
headers = [ln for ln in text.splitlines() if _is_hunk_header(ln)]
assert len(headers) == 1
import ai_review
assert ai_review.parse_diff_anchors(text)["x.yml"] == {2, 3, 4}
def _is_hunk_header(line: str) -> bool:
return bool(re.match(r"^@@ -\d+,\d+ \+\d+,\d+ @@", line))
def test_extract_finding_bullets_matches_current_renderer_output():
# The prior-review dedupe is only worth anything if it can read the
# bullets pragent itself posts. `summary_bullets` renders an emoji badge
# between the `-` and the `[SEV]` tag, which the original regex rejected.
import ai_review
findings = [
{"path": "a.py", "line": 10, "severity": "high",
"problem": "boom", "fix": "guard it", "suggestion": "", "reference": ""},
{"path": "b.go", "line": 0, "severity": "low",
"problem": "nit", "fix": "", "suggestion": "", "reference": ""},
]
body = ai_review.format_review_body(
ai_review.summary_bullets(findings), "m", "abc123",
findings_for_table=findings,
)
bullets = extract_finding_bullets(body)
assert len(bullets) == 2
assert any("a.py:10" in b and "boom" in b for b in bullets)
# `**Fix:**` continuation lines are prose, not findings.
assert all("**Fix:**" not in b for b in bullets)
assert ai_review.compact_prior_reviews([body]) != []