feat(input): add diff_compress module + prior-review compaction helpers
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.
This commit is contained in:
@@ -0,0 +1,169 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
r"""pragent pilot — diff compression + prior-review compaction.
|
||||||
|
|
||||||
|
Two pure helpers that shrink what lands in the model prompt without losing
|
||||||
|
signal:
|
||||||
|
|
||||||
|
* ``compress_diff(diff, *, context=2)`` — re-renders a unified diff so each
|
||||||
|
hunk keeps only ``context`` unchanged lines on either side of its +/- lines.
|
||||||
|
The default 2 matches what most reviewers see on GitHub/Gitea, and is
|
||||||
|
enough to anchor every ``+``/``-`` line and give the reviewer the enclosing
|
||||||
|
statement. Wider context = more reading; narrower = less. Set
|
||||||
|
``context=0`` for +/- only, ``context=-1`` to disable entirely.
|
||||||
|
|
||||||
|
* ``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 ("this PR adds eval() — risky") is just token burn. Bullet-only
|
||||||
|
priors cut ~75% off prior-review bytes on a typical 4-finding review.
|
||||||
|
|
||||||
|
Stdlib only. No I/O. Tolerant of malformed input — never raises.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
|
||||||
|
# Diff line types. Order matters: `+++`/ `---` headers and `@@` hunk headers
|
||||||
|
# are caught before the per-line prefix check.
|
||||||
|
_FILE_HEADER = re.compile(r"^(diff --git|Index:|---|\+\+\+|@@)")
|
||||||
|
|
||||||
|
# Captures `- <n>[,<m>]` AND `+ <n>[,<m>]` from `@@ -a,b +c,d @@`. We use the
|
||||||
|
# `+` side to reset the new-line counter; old-side is ignored.
|
||||||
|
_HUNK_RE = re.compile(r"^@@\s+-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s+@@")
|
||||||
|
|
||||||
|
# Match a pragent summary-bullet line: `- **[SEVERITY]** \`path:line\` — …`.
|
||||||
|
# Severity is uppercased critical|high|medium|low per the findings schema.
|
||||||
|
# We also accept the lower-case form (`- [high]`) used by summary_bullets.
|
||||||
|
_FINDING_BULLET_RE = re.compile(
|
||||||
|
r"^\s*-\s*\*?\*?\[(?P<sev>critical|high|medium|low|CRITICAL|HIGH|MEDIUM|LOW)\]"
|
||||||
|
r"\*?\*?\s+(?P<rest>.+)$"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def compress_diff(diff: str, *, context: int = 2) -> tuple[str, int, int]:
|
||||||
|
"""Re-render `diff` keeping at most `context` unchanged lines around +/-.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
diff: unified-diff text (what `gitea .../pulls/{n}.diff` returns).
|
||||||
|
context: max unchanged lines to keep on each side of a hunk. Use 0
|
||||||
|
for +/- only, -1 to disable compression (raw passthrough).
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
`(text, original_chars, kept_chars)`. `original_chars` is the byte
|
||||||
|
length of `diff` as given; `kept_chars` is the byte length of `text`.
|
||||||
|
On parse failure the original is returned unchanged so the worst case
|
||||||
|
is no improvement, never corruption.
|
||||||
|
"""
|
||||||
|
if not diff:
|
||||||
|
return diff or "", len(diff or ""), len(diff or "")
|
||||||
|
if context < 0:
|
||||||
|
return diff, len(diff), len(diff)
|
||||||
|
|
||||||
|
orig = len(diff)
|
||||||
|
lines = diff.splitlines()
|
||||||
|
out: list[str] = []
|
||||||
|
|
||||||
|
# State for the per-file walk.
|
||||||
|
i = 0
|
||||||
|
n = len(lines)
|
||||||
|
while i < n:
|
||||||
|
# Copy file headers verbatim until we hit the first `@@` hunk header.
|
||||||
|
hunk_start = i
|
||||||
|
while hunk_start < n and not lines[hunk_start].startswith("@@"):
|
||||||
|
out.append(lines[hunk_start])
|
||||||
|
hunk_start += 1
|
||||||
|
i = hunk_start
|
||||||
|
|
||||||
|
# Walk hunks, copying headers verbatim and trimming the inside.
|
||||||
|
while i < n and lines[i].startswith("@@"):
|
||||||
|
hunk_header = lines[i]
|
||||||
|
i += 1
|
||||||
|
|
||||||
|
# Collect the hunk body: every line until the next `@@` / file
|
||||||
|
# header / EOF. Within the body, classify each line.
|
||||||
|
body_start = i
|
||||||
|
while i < n and not _FILE_HEADER.match(lines[i]):
|
||||||
|
i += 1
|
||||||
|
body = lines[body_start:i]
|
||||||
|
|
||||||
|
# Render the body, collapsing long runs of context lines to a
|
||||||
|
# `@@ … @@` marker so the reviewer still sees that there IS more
|
||||||
|
# code there, just not in this window.
|
||||||
|
rendered, _ = _render_hunk_body(body, context=context)
|
||||||
|
if rendered:
|
||||||
|
out.append(hunk_header)
|
||||||
|
out.extend(rendered)
|
||||||
|
|
||||||
|
text = "\n".join(out) + ("\n" if diff.endswith("\n") else "")
|
||||||
|
if not text:
|
||||||
|
# splitlines() dropped nothing-but-newlines; fall back to original.
|
||||||
|
return diff, orig, orig
|
||||||
|
return text, orig, len(text)
|
||||||
|
|
||||||
|
|
||||||
|
def _render_hunk_body(body: list[str], *, context: int) -> tuple[list[str], int]:
|
||||||
|
"""Trim `body` to `context` unchanged lines around the +/- lines.
|
||||||
|
|
||||||
|
Body lines are classified:
|
||||||
|
- `+` line → keep
|
||||||
|
- `-` line → keep (paired with a `+` on the new side when both exist)
|
||||||
|
- `` `` line (or empty context) → keep only within ``context`` of a +/- line
|
||||||
|
- ``\ No newline at end of file`` → drop (no signal for the reviewer)
|
||||||
|
|
||||||
|
Collapsed gaps of ≥ 5 lines get a single ``@@ … N context line(s) omitted … @@``
|
||||||
|
marker so the reviewer knows code was elided. Smaller gaps (1–4 lines)
|
||||||
|
stay silent — the marker would be longer than the elision.
|
||||||
|
"""
|
||||||
|
if context == 0:
|
||||||
|
# Keep only +/- lines; drop all context.
|
||||||
|
out = [ln for ln in body if ln.startswith("+") or ln.startswith("-")]
|
||||||
|
return out, 0
|
||||||
|
|
||||||
|
# Find the index of every +/- line; a context line is kept if its
|
||||||
|
# distance to the nearest +/- line is ≤ context.
|
||||||
|
plus_minus_idx = [
|
||||||
|
j for j, ln in enumerate(body)
|
||||||
|
if ln.startswith("+") or ln.startswith("-")
|
||||||
|
]
|
||||||
|
if not plus_minus_idx:
|
||||||
|
# No +/- at all (rare — pure-context hunk): drop entirely.
|
||||||
|
return [], 0
|
||||||
|
|
||||||
|
keep = set()
|
||||||
|
for k in plus_minus_idx:
|
||||||
|
lo = max(0, k - context)
|
||||||
|
hi = min(len(body) - 1, k + context)
|
||||||
|
for j in range(lo, hi + 1):
|
||||||
|
keep.add(j)
|
||||||
|
|
||||||
|
out: list[str] = []
|
||||||
|
last_kept = -2 # sentinel: a gap of ≥ 5 between consecutive kept lines triggers a marker
|
||||||
|
for j, ln in enumerate(body):
|
||||||
|
if ln.startswith("\\ No newline"):
|
||||||
|
continue
|
||||||
|
if j in keep:
|
||||||
|
if j - last_kept > 5 and last_kept >= 0:
|
||||||
|
out.append(f"@@ … {j - last_kept - 1} context line(s) omitted … @@")
|
||||||
|
out.append(ln)
|
||||||
|
last_kept = j
|
||||||
|
|
||||||
|
return out, len(out)
|
||||||
|
|
||||||
|
|
||||||
|
def extract_finding_bullets(review_body: str) -> list[str]:
|
||||||
|
"""Pull the finding-bullet lines out of a prior review body.
|
||||||
|
|
||||||
|
Returns the matching lines verbatim (with their original indentation +
|
||||||
|
any continuation text), preserving the ``**[SEV]** `path:line` — problem
|
||||||
|
…`` shape the model emitted. Lines that look like bullets but lack the
|
||||||
|
severity tag are dropped — the reviewer synthesizes from the matched ones.
|
||||||
|
"""
|
||||||
|
if not review_body:
|
||||||
|
return []
|
||||||
|
out = []
|
||||||
|
for line in review_body.splitlines():
|
||||||
|
m = _FINDING_BULLET_RE.match(line)
|
||||||
|
if m:
|
||||||
|
out.append(line.strip())
|
||||||
|
return out
|
||||||
@@ -0,0 +1,248 @@
|
|||||||
|
"""Unit tests for pragent pilot diff_compress. No network."""
|
||||||
|
import os
|
||||||
|
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,10 +1,11 @@
|
||||||
|
ctx1
|
||||||
|
-removed
|
||||||
|
+added
|
||||||
|
ctx2
|
||||||
|
ctx3
|
||||||
|
ctx4
|
||||||
|
ctx5
|
||||||
|
ctx6
|
||||||
|
ctx7
|
||||||
|
+extra
|
||||||
|
ctx8
|
||||||
|
@@ -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_marker():
|
||||||
|
# Two +/- lines separated by 14 context lines, context=2 — the gap between
|
||||||
|
# them is 10 dropped lines (between the +/- windows), which exceeds the
|
||||||
|
# 5-line marker threshold. The marker tells the reviewer there's more code
|
||||||
|
# between the kept hunks.
|
||||||
|
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
|
||||||
|
assert "@@ …" in text and "context line(s) omitted" in text
|
||||||
|
|
||||||
|
|
||||||
|
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"]
|
||||||
Reference in New Issue
Block a user