Refactor: Add Optimization and better error handling (#26)

Author: HungKNguyen

.github/workflows/code-review.yml

@@ -14,6 +14,7 @@ on:
 jobs:
   code-review:
     runs-on: ubuntu-24.04
+    if: github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request)
     permissions:
       contents: read
       pull-requests: write

README.md

@@ -34,12 +34,16 @@ on:
 jobs:
   review:
     runs-on: ubuntu-latest
+    # IMPORTANT: This condition prevents the job from running on non-PR events,
+    # saving GitHub Actions minutes. Without this, the runner starts and bills
+    # for ~20-40 seconds even when the action skips internally.
+    if: github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request)
     steps:
       - uses: actions/checkout@v4
         with:
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
           fetch-depth: 2
-      
+
       - uses: PSPDFKit-labs/nutrient-code-review@main
         with:
           comment-pr: true
@@ -176,8 +180,12 @@ on:
 jobs:
   review:
     runs-on: ubuntu-latest
+    if: github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request)
     steps:
       - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha || github.sha }}
+          fetch-depth: 2
       - uses: PSPDFKit-labs/nutrient-code-review@main
         with:
           claude-api-key: ${{ secrets.CLAUDE_API_KEY }}
@@ -202,6 +210,25 @@ concurrency:
   cancel-in-progress: false  # Queues new runs instead
 ```
 
+### Billing Optimization (Recommended)
+
+**Important:** Always add a job-level `if:` condition to prevent wasting GitHub Actions minutes on non-PR events.
+
+Without this condition, the runner starts and bills for ~20-40 seconds even when the action skips internally. The `if:` condition prevents the job from starting at all:
+
+```yaml
+jobs:
+  review:
+    runs-on: ubuntu-latest
+    if: github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request)
+```
+
+**Why this matters:**
+- Without job-level `if:`: Runner starts → action installs dependencies → determines to skip → **~20-40s billed**
+- With job-level `if:`: Job never starts for non-PR events → **0s billed**
+
+This is especially important if you use `workflow_dispatch` or other event types that might not be PR-related.
+
 ## How It Works
 
 ### Architecture

action.yml

@@ -118,12 +118,7 @@ runs:
         # Install GitHub CLI for PR operations
         sudo apt-get update && sudo apt-get install -y gh
         echo "::endgroup::"
-    
-    - name: Set up Python
-      uses: actions/setup-python@v5
-      with:
-        python-version: '3.x'
-    
+
     - name: Get PR info for issue_comment events
       id: pr-info
       if: github.event_name == 'issue_comment'
@@ -349,23 +344,28 @@ runs:
       with:
         path: .claudecode-marker
         key: claudecode-${{ github.repository_id }}-pr-${{ github.event.pull_request.number || steps.pr-info.outputs.pr_number }}-${{ github.event.pull_request.head.sha || steps.pr-info.outputs.pr_sha || github.sha }}
-    
+
+    - name: Set up Python
+      if: steps.claudecode-check.outputs.enable_claudecode == 'true'
+      uses: actions/setup-python@v5
+      with:
+        python-version: '3.x'
+
     - name: Set up Node.js
       if: steps.claudecode-check.outputs.enable_claudecode == 'true'
       uses: actions/setup-node@v4
       with:
         node-version: '18'
 
     - name: Install dependencies
+      if: steps.claudecode-check.outputs.enable_claudecode == 'true'
       shell: bash
       env:
         ACTION_PATH: ${{ github.action_path }}
       run: |
         echo "::group::Install Deps"
-        if [ "${{ steps.claudecode-check.outputs.enable_claudecode }}" == "true" ]; then
-          pip install -r "$ACTION_PATH/claudecode/requirements.txt"
-          npm install -g @anthropic-ai/claude-code
-        fi
+        pip install -r "$ACTION_PATH/claudecode/requirements.txt"
+        npm install -g @anthropic-ai/claude-code
         sudo apt-get update && sudo apt-get install -y jq
         echo "::endgroup::"
     

claudecode/github_action_audit.py

@@ -395,45 +395,48 @@ def run_code_review(self, repo_dir: Path, prompt: str) -> Tuple[bool, str, Dict[
                     text=True,
                     timeout=self.timeout_seconds
                 )
-                
-                if result.returncode != 0:
-                    if attempt == NUM_RETRIES - 1:
-                        error_details = f"Claude Code execution failed with return code {result.returncode}\n"
-                        error_details += f"Stderr: {result.stderr}\n"
-                        error_details += f"Stdout: {result.stdout[:500]}..."  # First 500 chars
-                        return False, error_details, {}
-                    else:
-                        time.sleep(5*attempt)
-                        # Note: We don't do exponential backoff here to keep the runtime reasonable
-                        continue  # Retry
-                
-                # Parse JSON output
+
+                # Parse JSON output (even if returncode != 0, to detect specific errors)
                 success, parsed_result = parse_json_with_fallbacks(result.stdout, "Claude Code output")
-                
+
                 if success:
-                    # Check for "Prompt is too long" error that should trigger retry without diff
-                    if (isinstance(parsed_result, dict) and 
-                        parsed_result.get('type') == 'result' and 
+                    # Check for "Prompt is too long" error that should trigger fallback to agentic mode
+                    if (isinstance(parsed_result, dict) and
+                        parsed_result.get('type') == 'result' and
                         parsed_result.get('subtype') == 'success' and
                         parsed_result.get('is_error') and
                         parsed_result.get('result') == 'Prompt is too long'):
                         return False, "PROMPT_TOO_LONG", {}
-                    
+
                     # Check for error_during_execution that should trigger retry
-                    if (isinstance(parsed_result, dict) and 
-                        parsed_result.get('type') == 'result' and 
+                    if (isinstance(parsed_result, dict) and
+                        parsed_result.get('type') == 'result' and
                         parsed_result.get('subtype') == 'error_during_execution' and
                         attempt == 0):
                         continue  # Retry
-                    
-                    # Extract review findings
-                    parsed_results = self._extract_review_findings(parsed_result)
-                    return True, "", parsed_results
-                else:
-                    if attempt == 0:
-                        continue  # Retry once
+
+                    # If returncode is 0, extract review findings
+                    if result.returncode == 0:
+                        parsed_results = self._extract_review_findings(parsed_result)
+                        return True, "", parsed_results
+
+                # Handle non-zero return codes after parsing
+                if result.returncode != 0:
+                    if attempt == NUM_RETRIES - 1:
+                        error_details = f"Claude Code execution failed with return code {result.returncode}\n"
+                        error_details += f"Stderr: {result.stderr}\n"
+                        error_details += f"Stdout: {result.stdout[:500]}..."  # First 500 chars
+                        return False, error_details, {}
                     else:
-                        return False, "Failed to parse Claude output", {}
+                        time.sleep(5*attempt)
+                        # Note: We don't do exponential backoff here to keep the runtime reasonable
+                        continue  # Retry
+
+                # Parse failed
+                if attempt == 0:
+                    continue  # Retry once
+                else:
+                    return False, "Failed to parse Claude output", {}
 
             return False, "Unexpected error in retry logic", {}