Restore mainline compatibility after rebase

Author: matej

claudecode/github_action_audit.py

@@ -17,6 +17,7 @@
 # Import existing components we can reuse
 from claudecode.findings_filter import FindingsFilter
 from claudecode.json_parser import parse_json_with_fallbacks
+from claudecode.format_pr_comments import format_pr_comments_for_prompt, is_bot_comment
 from claudecode.prompts import get_unified_review_prompt  # Backward-compatible import for tests/extensions.
 from claudecode.review_orchestrator import ReviewModelConfig, ReviewOrchestrator
 from claudecode.constants import (
@@ -27,6 +28,7 @@
     SUBPROCESS_TIMEOUT
 )
 from claudecode.logger import get_logger
+from claudecode.review_schema import REVIEW_OUTPUT_SCHEMA
 
 logger = get_logger(__name__)
 
@@ -201,6 +203,56 @@ def get_pr_diff(self, repo_name: str, pr_number: int) -> str:
         response.raise_for_status()
 
         return self._filter_generated_files(response.text)
+
+    def get_pr_comments(self, repo_name: str, pr_number: int) -> List[Dict[str, Any]]:
+        """Get all review comments for a PR with pagination."""
+        all_comments = []
+        page = 1
+        per_page = 100
+
+        while True:
+            url = f"https://api.github.com/repos/{repo_name}/pulls/{pr_number}/comments"
+            params = {'per_page': per_page, 'page': page}
+
+            try:
+                response = requests.get(url, headers=self.headers, params=params)
+                response.raise_for_status()
+                comments = response.json()
+
+                if not comments:
+                    break
+
+                all_comments.extend(comments)
+                if len(comments) < per_page:
+                    break
+                page += 1
+            except requests.RequestException as e:
+                logger.warning(f"Failed to fetch comments page {page}: {e}")
+                break
+
+        return all_comments
+
+    def get_comment_reactions(self, repo_name: str, comment_id: int) -> Dict[str, int]:
+        """Get reactions for a specific comment, excluding bot reactions."""
+        url = f"https://api.github.com/repos/{repo_name}/pulls/comments/{comment_id}/reactions"
+
+        try:
+            response = requests.get(url, headers=self.headers)
+            response.raise_for_status()
+            reactions = response.json()
+
+            counts: Dict[str, int] = {}
+            for reaction in reactions:
+                user = reaction.get('user', {})
+                if user.get('type') == 'Bot':
+                    continue
+                content = reaction.get('content', '')
+                if content:
+                    counts[content] = counts.get(content, 0) + 1
+            return counts
+        except requests.RequestException as e:
+            logger.debug(f"Failed to fetch reactions for comment {comment_id}: {e}")
+            return {}
 
     def _is_excluded(self, filepath: str) -> bool:
         """Check if a file should be excluded based on directory or file patterns."""
@@ -298,7 +350,8 @@ def run_prompt(self, repo_dir: Path, prompt: str, model: Optional[str] = None) -
                 'claude',
                 '--output-format', 'json',
                 '--model', model_name,
-                '--disallowed-tools', 'Bash(ps:*)'
+                '--disallowed-tools', 'Bash(ps:*)',
+                '--json-schema', json.dumps(REVIEW_OUTPUT_SCHEMA),
             ]
 
             NUM_RETRIES = 3
@@ -370,11 +423,14 @@ def _extract_review_findings(self, claude_output: Any) -> Dict[str, Any]:
                     # Try to extract JSON from the result text
                     success, result_json = parse_json_with_fallbacks(result_text, "Claude result text")
                     if success and result_json and 'findings' in result_json:
+                        if 'pr_summary' not in result_json:
+                            result_json['pr_summary'] = {}
                         return result_json
 
         # Return empty structure if no findings found
         return {
             'findings': [],
+            'pr_summary': {},
             'analysis_summary': {
                 'files_reviewed': 0,
                 'high_severity': 0,
@@ -676,6 +732,34 @@ def main():
             print(json.dumps({'error': f'Failed to fetch PR data: {str(e)}'}))
             sys.exit(EXIT_GENERAL_ERROR)
 
+        # Backward-compatible context collection for review thread history.
+        review_context = None
+        try:
+            pr_comments = github_client.get_pr_comments(repo_name, pr_number)
+            if pr_comments:
+                bot_comment_threads = []
+                for comment in pr_comments:
+                    if is_bot_comment(comment):
+                        reactions = github_client.get_comment_reactions(repo_name, comment['id'])
+                        replies = [
+                            c for c in pr_comments
+                            if c.get('in_reply_to_id') == comment['id']
+                        ]
+                        replies.sort(key=lambda c: c.get('created_at', ''))
+                        bot_comment_threads.append({
+                            'bot_comment': comment,
+                            'replies': replies,
+                            'reactions': reactions,
+                        })
+                bot_comment_threads.sort(key=lambda t: t['bot_comment'].get('created_at', ''))
+                if bot_comment_threads:
+                    review_context = format_pr_comments_for_prompt(bot_comment_threads)
+                    if review_context:
+                        logger.info(f"Fetched previous review context ({len(review_context)} chars)")
+        except Exception as e:
+            logger.warning(f"Failed to fetch review context (continuing without it): {e}")
+            review_context = None
+
         max_diff_lines = get_max_diff_lines()
         diff_line_count = len(pr_diff.splitlines())
         if max_diff_lines and diff_line_count > max_diff_lines:

claudecode/prompts.py

@@ -40,6 +40,31 @@ def _build_hybrid_diff_section(pr_diff: str, max_lines: int) -> str:
         "Use this as a starting point only. You MUST validate findings with repository tool reads.\n"
     )
 
+def _build_diff_section(pr_diff: Optional[str], include_diff: bool) -> str:
+    """Build unified-prompt diff section for backward compatibility."""
+    if pr_diff and include_diff:
+        return f"""
+
+PR DIFF CONTENT:
+```
+{pr_diff}
+```
+
+Review the complete diff above. This contains all code changes in the PR.
+"""
+
+    return """
+
+IMPORTANT - FILE READING INSTRUCTIONS:
+You have access to the repository files. For each file listed above, use the Read tool to examine the changes.
+Focus on the files that are most likely to contain issues based on the PR context.
+
+To review effectively:
+1. Read each modified file to understand the current code
+2. Look at surrounding code context when needed to understand the changes
+3. Check related files if you need to understand dependencies or usage patterns
+"""
+
 
 def _base_context_block(pr_data: Dict[str, Any], pr_diff: str, max_diff_lines: int) -> str:
     """Shared context block used across prompts."""
@@ -281,48 +306,76 @@ def get_unified_review_prompt(
     include_diff: bool = True,
     custom_review_instructions: Optional[str] = None,
     custom_security_instructions: Optional[str] = None,
+    review_context: Optional[str] = None,
 ) -> str:
-    """Backward-compatible unified prompt used by tests and direct calls.
-
-    The unified prompt now enforces hybrid behavior: even when diff is included,
-    repository context reads are still mandatory.
-    """
-    diff_text = pr_diff if include_diff else ""
-    max_lines = len(diff_text.splitlines()) if diff_text else 0
-    combined = build_quality_prompt(
-        pr_data,
-        diff_text,
-        max_lines,
-        discovered_context={},
-        custom_review_instructions=custom_review_instructions,
-    )
-    security = build_security_prompt(
-        pr_data,
-        diff_text,
-        max_lines,
-        discovered_context={},
-        custom_security_instructions=custom_security_instructions,
-    )
+    """Backward-compatible unified prompt used by tests and direct calls."""
+    files_changed = _format_files_changed(pr_data)
+    diff_section = _build_diff_section(pr_diff, include_diff)
+    custom_review_section = f"\n{custom_review_instructions}\n" if custom_review_instructions else ""
+    custom_security_section = f"\n{custom_security_instructions}\n" if custom_security_instructions else ""
 
-    file_reading_block = (
-        "\nIMPORTANT - FILE READING INSTRUCTIONS:\n"
-        "You MUST read changed files and nearby context with repository tools before final findings.\n"
-    )
-    if include_diff and diff_text:
-        diff_anchor = f"\nPR DIFF CONTENT:\n```diff\n{diff_text}\n```\n"
-    else:
-        diff_anchor = "\n"
+    pr_description = (pr_data.get('body', '') or '').strip()
+    pr_description_section = ""
+    if pr_description:
+        if len(pr_description) > 2000:
+            pr_description = pr_description[:2000] + "... (truncated)"
+        pr_description_section = f"\nPR Description:\n{pr_description}\n"
 
-    return (
-        f"You are a senior engineer conducting a comprehensive code review of GitHub PR #{pr_data.get('number', 'unknown')}.\n"
-        "CONTEXT:\n"
-        f"- Title: {pr_data.get('title', 'unknown')}\n"
-        "OBJECTIVE:\n"
-        "Perform a high-signal code quality and security review with mandatory repository context validation.\n"
-        + diff_anchor
-        + file_reading_block
-        + "\nREQUIRED OUTPUT FORMAT:\nJSON only.\n\n"
-        + combined
-        + "\n\n"
-        + security
-    )
+    review_context_section = review_context or ""
+
+    return f"""
+You are a senior engineer conducting a comprehensive code review of GitHub PR #{pr_data.get('number', 'unknown')}: "{pr_data.get('title', 'unknown')}"
+
+CONTEXT:
+- Repository: {pr_data.get('head', {}).get('repo', {}).get('full_name', 'unknown')}
+- Author: {pr_data.get('user', 'unknown')}
+- Files changed: {pr_data.get('changed_files', 0)}
+- Lines added: {pr_data.get('additions', 0)}
+- Lines deleted: {pr_data.get('deletions', 0)}
+{pr_description_section}
+Files modified:
+{files_changed}{diff_section}{review_context_section}
+
+OBJECTIVE:
+Perform a focused, high-signal code review to identify HIGH-CONFIDENCE issues introduced by this PR.
+
+CODE QUALITY CATEGORIES:
+- correctness, reliability, performance, maintainability, testing
+{custom_review_section}
+SECURITY CATEGORIES:
+- input validation, authn/authz, crypto/secrets, code execution, data exposure
+{custom_security_section}
+
+REQUIRED OUTPUT FORMAT:
+
+{{
+  "pr_summary": {{
+    "overview": "2-4 sentence summary of what this PR changes and why it matters",
+    "file_changes": [
+      {{
+        "label": "src/auth.py",
+        "files": ["src/auth.py"],
+        "changes": "Brief description of changes (~10 words)"
+      }}
+    ]
+  }},
+  "findings": [
+    {{
+      "file": "path/to/file.py",
+      "line": 42,
+      "severity": "HIGH|MEDIUM|LOW",
+      "category": "correctness|reliability|performance|maintainability|testing|security",
+      "title": "Short summary of the issue",
+      "description": "What is wrong and where it happens",
+      "impact": "Concrete impact or failure mode",
+      "recommendation": "Actionable fix or mitigation",
+      "confidence": 0.95
+    }}
+  ]
+}}
+
+PR SUMMARY GUIDELINES:
+- overview: 2-4 sentences describing WHAT changed and WHY (purpose/goal)
+- file_changes: One entry per file or group of related files
+- changes: Brief description (~10 words), focus on purpose not implementation
+"""

scripts/comment-pr-findings.bun.test.js

@@ -1076,7 +1076,7 @@ describe('comment-pr-findings.js', () => {
     test('should update existing review in place when state is unchanged and no inline comments', async () => {
       // Existing APPROVED review, new findings also result in APPROVED (no HIGH severity)
       const mockReviews = [
-        { id: 101, state: 'APPROVED', user: { type: 'Bot' }, body: 'No issues found. Changes look good.' }
+        { id: 101, state: 'APPROVED', user: { type: 'Bot' }, body: '<!-- nutrient-code-review-action -->\nNo issues found. Changes look good.' }
       ];
 
       readFileSyncSpy.mockImplementation((path) => {
@@ -1129,7 +1129,7 @@ describe('comment-pr-findings.js', () => {
     test('should dismiss and create new review when state changes', async () => {
       // Existing APPROVED review, but new findings have HIGH severity = CHANGES_REQUESTED
       const mockReviews = [
-        { id: 101, state: 'APPROVED', user: { type: 'Bot' }, body: 'No issues found. Changes look good.' }
+        { id: 101, state: 'APPROVED', user: { type: 'Bot' }, body: '<!-- nutrient-code-review-action -->\nNo issues found. Changes look good.' }
       ];
 
       const mockFindings = [{