Cross-review: diff truncate at newline; min-agreement help; webhook env vars; tests

Co-authored-by: Cursor <[email protected]>

Author: chefsale

src/ai_reviewer/cli.py

@@ -68,7 +68,7 @@ def cli(verbose: bool) -> None:
     "min_agreement",
     type=click.FloatRange(0.0, 1.0),
     default=2 / 3,
-    help="Fraction of assessing agents that must mark a finding valid to keep it (default: 2/3)",
+    help="Fraction of assessing agents that must mark a finding valid (default: 2/3; with 2 agents, both must agree)",
 )
 def review_pr(
     repo: str,

src/ai_reviewer/github/webhook.py

@@ -153,6 +153,11 @@ async def default_review_handler(repo: str, pr_number: int) -> None:
                 cursor_config=cursor_config,
                 github_token=github_token,
                 num_agents=int(os.environ.get("NUM_AGENTS", "3")),
+                enable_cross_review=os.environ.get("ENABLE_CROSS_REVIEW", "true").lower()
+                != "false",
+                min_validation_agreement=float(
+                    os.environ.get("MIN_VALIDATION_AGREEMENT", str(2 / 3))
+                ),
             )
 
             if review.all_agents_failed:

src/ai_reviewer/review.py

@@ -216,6 +216,11 @@ def get_cross_review_prompt(
         )
     findings_text = "\n".join(findings_blob)
 
+    # Truncate at newline boundary to avoid cutting mid-line or mid-hunk
+    diff_excerpt = diff[:_CROSS_REVIEW_DIFF_MAX_CHARS]
+    if "\n" in diff_excerpt:
+        diff_excerpt = diff_excerpt.rsplit("\n", 1)[0]
+
     return f"""You are in a **cross-review round**. Multiple agents already produced the findings below for this PR. Your job is to validate them and rank by importance.
 
 ## PR
@@ -224,7 +229,7 @@ def get_cross_review_prompt(
 
 ## Code diff (excerpt)
 ```diff
-{diff[:_CROSS_REVIEW_DIFF_MAX_CHARS]}
+{diff_excerpt}
 ```
 
 ## Findings to validate and rank

tests/test_review.py

@@ -1,12 +1,20 @@
 """Tests for the review module, particularly aggregate_findings."""
 
+from datetime import datetime
+
 import pytest
 
+from ai_reviewer.models.context import ReviewContext
+from ai_reviewer.models.findings import Category, ConsolidatedFinding, Severity
+from ai_reviewer.models.review import ConsolidatedReview
 from ai_reviewer.review import (
     _cluster_raw_findings,
     _detect_pr_type,
     _raw_findings_similar,
     aggregate_findings,
+    apply_cross_review,
+    get_cross_review_prompt,
+    parse_cross_review_response,
 )
 
 
@@ -381,3 +389,189 @@ def test_full_consensus_all_agents_find_same_issue(self):
         assert len(result.findings) == 1
         assert result.findings[0].consensus_score == pytest.approx(1.0, rel=0.01)
         assert len(result.findings[0].agreeing_agents) == 3
+
+
+def _make_finding(fid: str, severity: Severity = Severity.WARNING) -> ConsolidatedFinding:
+    """Minimal ConsolidatedFinding for cross-review tests."""
+    return ConsolidatedFinding(
+        id=fid,
+        file_path="src/foo.py",
+        line_start=10,
+        line_end=12,
+        severity=severity,
+        category=Category.LOGIC,
+        title="Test finding",
+        description="Description",
+        suggested_fix=None,
+        consensus_score=1.0,
+        agreeing_agents=["agent-1"],
+        confidence=0.9,
+    )
+
+
+def _make_review(findings: list[ConsolidatedFinding]) -> ConsolidatedReview:
+    """Minimal ConsolidatedReview for cross-review tests."""
+    return ConsolidatedReview(
+        id="review-1",
+        created_at=datetime.now(),
+        repo="test/repo",
+        pr_number=1,
+        findings=findings,
+        summary="Summary",
+        agent_count=3,
+        review_quality_score=0.9,
+        total_review_time_ms=0,
+        failed_agents=[],
+    )
+
+
+class TestParseCrossReviewResponse:
+    """Tests for parse_cross_review_response."""
+
+    def test_valid_json(self):
+        content = '{"assessments": [{"id": "finding-1", "valid": true, "rank": 1}], "summary": "OK"}'
+        assessments, summary = parse_cross_review_response(content)
+        assert len(assessments) == 1
+        assert assessments[0]["id"] == "finding-1"
+        assert assessments[0]["valid"] is True
+        assert assessments[0]["rank"] == 1
+        assert summary == "OK"
+
+    def test_markdown_fenced_json(self):
+        content = """Some text
+```json
+{"assessments": [{"id": "f1", "valid": false, "rank": 2}], "summary": "Done"}
+```
+"""
+        assessments, summary = parse_cross_review_response(content)
+        assert len(assessments) == 1
+        assert assessments[0]["id"] == "f1"
+        assert assessments[0]["valid"] is False
+        assert summary == "Done"
+
+    def test_malformed_input_returns_empty(self):
+        assessments, summary = parse_cross_review_response("not json at all")
+        assert assessments == []
+        assert summary == ""
+
+    def test_invalid_json_returns_empty(self):
+        content = '{"assessments": [invalid]}'
+        assessments, summary = parse_cross_review_response(content)
+        assert assessments == []
+        assert summary == ""
+
+
+class TestApplyCrossReview:
+    """Tests for apply_cross_review."""
+
+    def test_no_assessments_returns_unchanged(self):
+        review = _make_review([_make_finding("f1")])
+        result = apply_cross_review(review, [])
+        assert result.findings == review.findings
+        assert result.summary == review.summary
+
+    def test_no_votes_for_finding_kept(self):
+        """Findings with zero votes are kept (not counted as rejected)."""
+        review = _make_review([_make_finding("f1"), _make_finding("f2")])
+        # Only agent assesses f1; f2 gets no votes
+        all_assessments = [
+            ("agent-1", [{"id": "f1", "valid": True, "rank": 1}]),
+        ]
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=0.5)
+        assert len(result.findings) == 2
+        ids = [f.id for f in result.findings]
+        assert "f1" in ids and "f2" in ids
+
+    def test_partial_votes_uses_len_votes_not_n_agents(self):
+        """Valid ratio is over assessing agents, not total agents."""
+        review = _make_review([_make_finding("f1")])
+        # 1 valid out of 1 assessing agent -> ratio 1.0, kept
+        all_assessments = [("agent-1", [{"id": "f1", "valid": True, "rank": 1}])]
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=0.5)
+        assert len(result.findings) == 1
+        # 1 valid, 1 invalid from 2 agents that assessed it -> 0.5
+        all_assessments = [
+            ("agent-1", [{"id": "f1", "valid": True, "rank": 1}]),
+            ("agent-2", [{"id": "f1", "valid": False, "rank": 5}]),
+        ]
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=0.5)
+        assert len(result.findings) == 1
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=0.67)
+        assert len(result.findings) == 0
+
+    def test_threshold_drops_below_keeps_at_or_above(self):
+        review = _make_review([_make_finding("f1")])
+        # 2/3 valid
+        all_assessments = [
+            ("a1", [{"id": "f1", "valid": True, "rank": 1}]),
+            ("a2", [{"id": "f1", "valid": True, "rank": 2}]),
+            ("a3", [{"id": "f1", "valid": False, "rank": 3}]),
+        ]
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=2 / 3)
+        assert len(result.findings) == 1
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=0.9)
+        assert len(result.findings) == 0
+
+    def test_reordered_by_avg_rank_then_severity(self):
+        f1 = _make_finding("f1", Severity.WARNING)
+        f2 = _make_finding("f2", Severity.CRITICAL)
+        f3 = _make_finding("f3", Severity.SUGGESTION)
+        review = _make_review([f1, f2, f3])
+        # f1 rank 3, f2 rank 1, f3 rank 2 -> order f2, f3, f1
+        all_assessments = [
+            ("a1", [{"id": "f1", "valid": True, "rank": 3}, {"id": "f2", "valid": True, "rank": 1}, {"id": "f3", "valid": True, "rank": 2}]),
+        ]
+        result = apply_cross_review(review, all_assessments)
+        assert [x.id for x in result.findings] == ["f2", "f3", "f1"]
+
+    def test_summary_unchanged_when_nothing_dropped_or_reordered(self):
+        review = _make_review([_make_finding("f1")])
+        all_assessments = [("a1", [{"id": "f1", "valid": True, "rank": 1}])]
+        result = apply_cross_review(review, all_assessments)
+        assert result.summary == review.summary
+
+    def test_summary_appends_when_dropped(self):
+        review = _make_review([_make_finding("f1"), _make_finding("f2")])
+        all_assessments = [
+            ("a1", [{"id": "f1", "valid": True, "rank": 1}, {"id": "f2", "valid": False, "rank": 2}]),
+            ("a2", [{"id": "f1", "valid": True, "rank": 1}, {"id": "f2", "valid": False, "rank": 2}]),
+        ]
+        result = apply_cross_review(review, all_assessments, min_validation_agreement=1.0)
+        assert len(result.findings) == 1
+        assert "1 finding(s) dropped" in result.summary
+
+
+class TestGetCrossReviewPrompt:
+    """Tests for get_cross_review_prompt."""
+
+    def test_diff_truncated_at_newline(self):
+        """Diff excerpt does not cut mid-line."""
+        context = ReviewContext(
+            repo_name="test/repo",
+            pr_number=1,
+            pr_title="Title",
+            pr_description="",
+            base_branch="main",
+            head_branch="feature",
+            author="dev",
+            changed_files_count=1,
+            additions=10,
+            deletions=2,
+        )
+        review = _make_review([_make_finding("finding-1")])
+        # Create diff that would be cut at 50 chars (mid-line)
+        line = "a" * 30 + "\n" + "b" * 30
+        diff = line + "\nlast"
+        import ai_reviewer.review as review_mod
+
+        old_max = review_mod._CROSS_REVIEW_DIFF_MAX_CHARS
+        try:
+            review_mod._CROSS_REVIEW_DIFF_MAX_CHARS = 50
+            prompt = get_cross_review_prompt(context, review, diff)
+            # Excerpt should end at newline, not mid "b"
+            assert "```diff" in prompt
+            excerpt = prompt.split("```diff")[1].split("```")[0].strip()
+            assert excerpt.endswith("a" * 30)
+            assert not excerpt.endswith("b")
+        finally:
+            review_mod._CROSS_REVIEW_DIFF_MAX_CHARS = old_max