Claude
7a510a926d
Group review, feedback, evaluation, observability, and entrypoint code into packages. Keep thin top-level compatibility shims for existing scripts and imports, and mirror the structure in the tests.
339 lines
12 KiB
Python
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]) != []
|