From 770581bf53ad13b43d65aee6e6e85cdc617b3157 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 20 Aug 2026 16:12:33 +0000 Subject: [PATCH] feat(input): add diff_compress module + prior-review compaction helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- pilot/diff_compress.py | 169 ++++++++++++++++++++ tests/pilot/test_diff_compress.py | 248 ++++++++++++++++++++++++++++++ 2 files changed, 417 insertions(+) create mode 100644 pilot/diff_compress.py create mode 100644 tests/pilot/test_diff_compress.py diff --git a/pilot/diff_compress.py b/pilot/diff_compress.py new file mode 100644 index 0000000..52649e0 --- /dev/null +++ b/pilot/diff_compress.py @@ -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 `- [,]` AND `+ [,]` 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*\*?\*?\[(?Pcritical|high|medium|low|CRITICAL|HIGH|MEDIUM|LOW)\]" + r"\*?\*?\s+(?P.+)$" +) + + +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 \ No newline at end of file diff --git a/tests/pilot/test_diff_compress.py b/tests/pilot/test_diff_compress.py new file mode 100644 index 0000000..7af959a --- /dev/null +++ b/tests/pilot/test_diff_compress.py @@ -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._ + +""" + + +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"] \ No newline at end of file