LGTM-with-comments docs and tests; fix order_changed false positive; webhook env validation

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: chefsale

src/ai_reviewer/github/formatter.py

@@ -410,6 +410,9 @@ def get_review_action_with_delta(
     ) -> str:
         """Determine GitHub review action considering the delta.
 
+        LGTM-with-comments: REQUEST_CHANGES only for critical findings. When there are
+        only warnings, suggestions, or nitpicks, returns COMMENT so the author isn't blocked.
+
         Args:
             review: Consolidated review
             delta: Review delta
@@ -418,25 +421,25 @@ def get_review_action_with_delta(
         Returns:
             GitHub review action
         """
-        # If all issues are resolved, approve (if allowed)
         if delta.all_issues_resolved and allow_approve:
             return "APPROVE"
 
-        # If there are critical issues (new or open), request changes
-        # But only if allow_approve is True (i.e., not in GitHub Actions)
-        # GitHub Actions can't approve, and REQUEST_CHANGES blocks merging
+        # Block merge only when there are critical findings (not warnings/suggestions/nitpicks)
         has_critical = any(
             f.severity.value == "critical" for f in delta.new_findings + delta.open_findings
         )
         if has_critical and allow_approve:
             return "REQUEST_CHANGES"
 
-        # In GitHub Actions or no critical issues, just comment
+        # No critical: COMMENT (includes only nits/suggestions/warnings — don't block author)
         return "COMMENT"
 
     def get_review_action(self, review: ConsolidatedReview, allow_approve: bool = True) -> str:
         """Determine the GitHub review action based on findings.
 
+        LGTM-with-comments: REQUEST_CHANGES only for critical findings. When there are
+        only warnings, suggestions, or nitpicks, returns COMMENT so the author isn't blocked.
+
         Args:
             review: Consolidated review
             allow_approve: Whether to allow APPROVE/REQUEST_CHANGES actions
@@ -445,14 +448,12 @@ def get_review_action(self, review: ConsolidatedReview, allow_approve: bool = Tr
         Returns:
             GitHub review action: "APPROVE", "REQUEST_CHANGES", or "COMMENT"
         """
-        # REQUEST_CHANGES blocks merging - only use when allow_approve is True
-        # (i.e., running locally with proper permissions)
+        if not review.findings and allow_approve:
+            return "APPROVE"
+        # Block merge only on critical; warnings/suggestions/nitpicks → COMMENT
         if review.has_critical_issues and allow_approve:
             return "REQUEST_CHANGES"
-        elif not review.findings and allow_approve:
-            return "APPROVE"
-        else:
-            return "COMMENT"
+        return "COMMENT"
 
 
 def format_review_as_json(review: ConsolidatedReview) -> dict:

src/ai_reviewer/github/webhook.py

@@ -140,10 +140,32 @@ async def default_review_handler(repo: str, pr_number: int) -> None:
             logger.error("GITHUB_TOKEN not set")
             return
 
+        try:
+            cursor_timeout = int(os.environ.get("CURSOR_TIMEOUT", "300"))
+        except ValueError:
+            cursor_timeout = 300
+            logger.warning("CURSOR_TIMEOUT invalid, using default 300")
+
+        try:
+            num_agents = int(os.environ.get("NUM_AGENTS", "3"))
+        except ValueError:
+            num_agents = 3
+            logger.warning("NUM_AGENTS invalid, using default 3")
+
+        try:
+            min_agreement = float(os.environ.get("MIN_VALIDATION_AGREEMENT", str(2 / 3)))
+        except ValueError:
+            min_agreement = 2 / 3
+            logger.warning("MIN_VALIDATION_AGREEMENT invalid, using default 2/3")
+
         cursor_config = CursorConfig(
             api_key=cursor_api_key,
             base_url=os.environ.get("CURSOR_BASE_URL", "https://api.cursor.com/v0"),
-            timeout=int(os.environ.get("CURSOR_TIMEOUT", "300")),
+            timeout=cursor_timeout,
+        )
+
+        enable_cross_review = (
+            os.environ.get("ENABLE_CROSS_REVIEW", "true").lower() != "false"
         )
 
         try:
@@ -152,12 +174,9 @@ async def default_review_handler(repo: str, pr_number: int) -> None:
                 pr_number=pr_number,
                 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))
-                ),
+                num_agents=num_agents,
+                enable_cross_review=enable_cross_review,
+                min_validation_agreement=min_agreement,
             )
 
             if review.all_agents_failed:

src/ai_reviewer/review.py

@@ -333,9 +333,10 @@ def apply_cross_review(
 
     new_findings = [x[0] for x in kept]
     n_dropped = len(review.findings) - len(new_findings)
-    original_ids = [f.id for f in review.findings]
     new_ids = [f.id for f in new_findings]
-    order_changed = original_ids != new_ids
+    # Compare only relative order of retained findings (avoid false positive when findings dropped)
+    remaining_original_order = [f.id for f in review.findings if f.id in set(new_ids)]
+    order_changed = remaining_original_order != new_ids
 
     summary = review.summary
     if n_dropped > 0 or order_changed:

tests/test_github.py

@@ -304,6 +304,79 @@ def test_determines_review_action(self):
         # This is intentional - REQUEST_CHANGES blocks merging and Actions can't approve to unblock
         assert formatter.get_review_action(critical_review, allow_approve=False) == "COMMENT"
 
+        # LGTM-with-comments: only nits/suggestions (no critical/warning) → COMMENT (don't block author)
+        nits_only_review = ConsolidatedReview(
+            id="review-nits",
+            created_at=datetime.now(),
+            repo="test/repo",
+            pr_number=42,
+            findings=[
+                ConsolidatedFinding(
+                    id="f1",
+                    file_path="test.py",
+                    line_start=1,
+                    line_end=2,
+                    severity=Severity.SUGGESTION,
+                    category=Category.STYLE,
+                    title="Consider renaming",
+                    description="Optional",
+                    suggested_fix=None,
+                    consensus_score=0.5,
+                    agreeing_agents=["a1"],
+                    confidence=0.8,
+                ),
+                ConsolidatedFinding(
+                    id="f2",
+                    file_path="test.py",
+                    line_start=3,
+                    line_end=3,
+                    severity=Severity.NITPICK,
+                    category=Category.STYLE,
+                    title="Nit: trailing space",
+                    description="Style",
+                    suggested_fix=None,
+                    consensus_score=0.3,
+                    agreeing_agents=["a1"],
+                    confidence=0.7,
+                ),
+            ],
+            summary="Minor suggestions",
+            agent_count=2,
+            review_quality_score=0.9,
+            total_review_time_ms=1500,
+        )
+        assert formatter.get_review_action(nits_only_review) == "COMMENT"
+        assert formatter.get_review_action(nits_only_review, allow_approve=True) == "COMMENT"
+
+        # Only warnings (no critical) → COMMENT (we don't block on warnings)
+        warnings_only_review = ConsolidatedReview(
+            id="review-warn",
+            created_at=datetime.now(),
+            repo="test/repo",
+            pr_number=42,
+            findings=[
+                ConsolidatedFinding(
+                    id="f1",
+                    file_path="test.py",
+                    line_start=1,
+                    line_end=1,
+                    severity=Severity.WARNING,
+                    category=Category.LOGIC,
+                    title="Edge case",
+                    description="Consider handling",
+                    suggested_fix=None,
+                    consensus_score=1.0,
+                    agreeing_agents=["a1"],
+                    confidence=0.85,
+                ),
+            ],
+            summary="One warning",
+            agent_count=1,
+            review_quality_score=0.85,
+            total_review_time_ms=1000,
+        )
+        assert formatter.get_review_action(warnings_only_review) == "COMMENT"
+
 
 class TestResolveFixedComments:
     """Tests for duplicate detection and resolved comment handling."""