set even longer timeout

.github/workflows/ai-review.yml

@@ -7,24 +7,25 @@ on:
 jobs:
   review:
     runs-on: ubuntu-latest
-    services:
-      ollama:
-        image: ollama/ollama:latest
-        ports:
-          - 11434:11434
-        options: >-
-          --health-cmd="curl -sSf http://192.168.1.92:11434/ || exit 1" --health-interval=10s --health-timeout=5s --health-retries=12
+    container:
+      image: node:20-bookworm
 
     steps:
+      - name: Install system deps + Python 3.11
+        run: |
+          apt-get update
+          apt-get install -y --no-install-recommends \
+            git curl ca-certificates \
+            python3.11 python3.11-venv python3-pip
+          ln -sf /usr/bin/python3.11 /usr/local/bin/python
+          python --version
+          node --version
+          git --version
+
       - name: Checkout
         uses: actions/checkout@v4
 
-      - name: Set up Python
-        uses: actions/setup-python@v4
-        with:
-          python-version: '3.11'
-
-      - name: Create venv and install
+      - name: Create venv and install project
         run: |
           python -m venv venv
           . venv/bin/activate
@@ -33,44 +34,28 @@ jobs:
 
       - name: Wait for Ollama
         run: |
-          for i in $(seq 1 30); do
-            if curl -sSf http://192.168.1.92:11434/ >/dev/null 2>&1; then
-              echo "ollama ready" && break
+          for i in $(seq 1 60); do
+            if curl -sSf http://192.168.1.92:11434/api/tags >/dev/null 2>&1; then
+              echo "ollama ready" && exit 0
             fi
             sleep 1
           done
-
-      - name: (Optional) Pull model into Ollama
-        run: |
-          . venv/bin/activate
-          ollama pull qwen2.5-coder:7b || true
+          echo "ollama not reachable" >&2
+          exit 1
 
       - name: Run ai-reviewer
         env:
           OLLAMA_HOST: http://192.168.1.92:11434
         run: |
           . venv/bin/activate
-          ai-reviewer review --repo . --base "${{ github.event.pull_request.base.ref }}" --head "${{ github.head_ref }}" --format json > review.json
+          ai-reviewer review \
+            --repo . \
+            --base "${{ github.event.pull_request.base.ref }}" \
+            --head "${{ github.head_ref }}" \
+            --format json > review.json
 
-      - name: Post PR comment with findings
-        uses: actions/github-script@v6
-        with:
-          script: |
-            const fs = require('fs');
-            let body = '{}';
-            try {
-              body = fs.readFileSync('review.json', 'utf8');
-            } catch (e) {
-              body = JSON.stringify({ error: 'missing-review', message: String(e) });
-            }
-            let parsed = {};
-            try { parsed = JSON.parse(body); } catch (e) { parsed = { error: 'invalid-json', raw: body }; }
-            const findings = parsed.findings || [];
-            const summary = findings.length === 0 ? 'AI Reviewer: no findings.' : `AI Reviewer found ${findings.length} findings.`;
-            const commentBody = `${summary}\n\n<details><summary>Full JSON</summary>\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n` + '```json\n' + JSON.stringify(parsed, null, 2) + '\n```\n</details>';
-            await github.rest.issues.createComment({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              issue_number: context.issue.number,
-              body: commentBody,
-            });
+      # Temporarily remove github-script; it also needs GitHub env
+      - name: Show result
+        run: |
+          ls -la
+          head -c 2000 review.json || true

src/ai_reviewer/diff.py

@@ -43,19 +43,64 @@ class DiffChunk:
 
 
 def run_git_diff(repo: str, base: str, head: str) -> str:
-    cmd = [
-        "git",
-        "-C",
-        repo,
-        "diff",
-        f"{base}...{head}",
-        "--unified=3",
-        "--no-color",
-    ]
-    result = subprocess.run(cmd, check=False, capture_output=True, text=True)
-    if result.returncode not in (0, 1):
-        raise RuntimeError(result.stderr.strip() or "git diff failed")
-    return result.stdout
+    base_candidates = _build_ref_candidates(base)
+    head_candidates = _build_ref_candidates(head)
+
+    stdout, stderr, fallback_pairs = _diff_with_candidates(repo, base_candidates, head_candidates)
+    if stdout is not None:
+        return stdout
+
+    # If the diff failed because revisions are missing (common in CI when the
+    # PR head/base aren't fetched), try fetching from origin and retry once.
+    first_error = stderr
+    try:
+        fetch_cmd = ["git", "-C", repo, "fetch", "origin", base, head]
+        subprocess.run(fetch_cmd, check=False, capture_output=True, text=True)
+    except Exception:
+        # Ignore fetch errors; we'll raise the original error below.
+        pass
+
+    stdout, stderr, fallback_pairs_after_fetch = _diff_with_candidates(
+        repo, base_candidates, head_candidates
+    )
+    if stdout is not None:
+        return stdout
+
+    stdout, fallback_error = _fallback_diff_without_merge_base(
+        repo, fallback_pairs + fallback_pairs_after_fetch
+    )
+    if stdout is not None:
+        return stdout
+
+    # If still failing, raise a helpful error including stderr from git
+    raise RuntimeError(fallback_error or stderr or first_error or "git diff failed")
+
+
+def _build_ref_candidates(ref: str) -> list[str]:
+    candidates = [ref]
+    if ref and not ref.startswith("origin/"):
+        candidates.append(f"origin/{ref}")
+    return candidates
+
+
+def _diff_with_candidates(
+    repo: str, base_candidates: list[str], head_candidates: list[str]
+) -> tuple[str | None, str, list[tuple[str, str]]]:
+    last_error = ""
+    no_merge_base_pairs: list[tuple[str, str]] = []
+    for base_ref in base_candidates:
+        for head_ref in head_candidates:
+            result = _run_git_diff_command(repo, base_ref, head_ref, symmetric=True)
+            if result.returncode in (0, 1):
+                return result.stdout, "", []
+
+            if result.stderr:
+                err = result.stderr.strip()
+                last_error = err
+                if "no merge base" in err.lower():
+                    no_merge_base_pairs.append((base_ref, head_ref))
+
+    return None, last_error, no_merge_base_pairs
 
 
 def parse_diff(diff_text: str) -> list[FileDiff]:
@@ -154,3 +199,33 @@ def chunk_files(files: Iterable[FileDiff], max_lines: int = 350) -> list[DiffChu
             )
 
     return chunks
+
+
+def _run_git_diff_command(
+    repo: str, base_ref: str, head_ref: str, *, symmetric: bool
+) -> subprocess.CompletedProcess[str]:
+    cmd = ["git", "-C", repo, "diff"]
+    if symmetric:
+        cmd.append(f"{base_ref}...{head_ref}")
+    else:
+        cmd.extend([base_ref, head_ref])
+    cmd.extend(["--unified=3", "--no-color"])
+    return subprocess.run(cmd, check=False, capture_output=True, text=True)
+
+
+def _fallback_diff_without_merge_base(
+    repo: str, pairs: list[tuple[str, str]]
+) -> tuple[str | None, str]:
+    last_error = ""
+    seen: set[tuple[str, str]] = set()
+    for base_ref, head_ref in pairs:
+        key = (base_ref, head_ref)
+        if key in seen:
+            continue
+        seen.add(key)
+        result = _run_git_diff_command(repo, base_ref, head_ref, symmetric=False)
+        if result.returncode in (0, 1):
+            return result.stdout, ""
+        if result.stderr:
+            last_error = result.stderr.strip()
+    return None, last_error

src/ai_reviewer/ollama.py

@@ -17,7 +17,7 @@ class OllamaClient:
             "stream": False,
             "options": {"temperature": 0},
         }
-        with httpx.Client(timeout=60) as client:
+        with httpx.Client(timeout=3600) as client:
             response = client.post(url, json=payload)
         response.raise_for_status()
         data = response.json()

tests/test_diff.py

@@ -0,0 +1,86 @@
+from __future__ import annotations
+
+from typing import Any
+from unittest.mock import patch
+
+import subprocess
+
+from ai_reviewer.diff import run_git_diff
+
+
+def _completed(cmd: list[str], returncode: int, stdout: str = "", stderr: str = "") -> subprocess.CompletedProcess[Any]:
+    return subprocess.CompletedProcess(cmd, returncode, stdout, stderr)
+
+
+def test_run_git_diff_prefers_direct_refs():
+    calls: list[list[str]] = []
+
+    def fake_run(cmd, check=False, capture_output=False, text=False):  # type: ignore[override]
+        calls.append(cmd)
+        if cmd[3] == "diff":
+            if cmd[4] == "base...head":
+                return _completed(cmd, 0, stdout="ok")
+            return _completed(cmd, 128, stderr="fatal")
+        raise AssertionError("fetch should not run when refs exist locally")
+
+    with patch("ai_reviewer.diff.subprocess.run", side_effect=fake_run):
+        output = run_git_diff(".", "base", "head")
+
+    assert output == "ok"
+    # Only one diff attempt should be needed when local refs exist.
+    assert len(calls) == 1
+
+
+def test_run_git_diff_falls_back_to_origin_refs_after_fetch():
+    fetched = False
+    diff_attempts: list[str] = []
+
+    def fake_run(cmd, check=False, capture_output=False, text=False):  # type: ignore[override]
+        nonlocal fetched
+        if cmd[3] == "diff":
+            if len(cmd) == 7:
+                spec = cmd[4]
+            else:
+                spec = " ".join(cmd[4:6])
+            diff_attempts.append(spec)
+            if spec == "origin/base...origin/head" and fetched:
+                return _completed(cmd, 0, stdout="remote-ok")
+            return _completed(cmd, 128, stderr="missing ref")
+        if cmd[3] == "fetch":
+            fetched = True
+            return _completed(cmd, 0)
+        raise AssertionError("unexpected git invocation")
+
+    with patch("ai_reviewer.diff.subprocess.run", side_effect=fake_run):
+        output = run_git_diff(".", "base", "head")
+
+    assert output == "remote-ok"
+    assert "origin/base...origin/head" in diff_attempts
+    # Ensure the fallback only succeeds after fetch.
+    assert diff_attempts.index("origin/base...origin/head") > diff_attempts.index("base...head")
+
+
+def test_run_git_diff_fallbacks_to_two_dot_when_merge_base_missing():
+    fetched = False
+    diff_attempts: list[str] = []
+
+    def fake_run(cmd, check=False, capture_output=False, text=False):  # type: ignore[override]
+        nonlocal fetched
+        if cmd[3] == "diff":
+            if len(cmd) == 7:
+                spec = cmd[4]
+                diff_attempts.append(f"sym:{spec}")
+                return _completed(cmd, 128, stderr="fatal: origin/main...origin/head: no merge base")
+            spec = " ".join(cmd[4:6])
+            diff_attempts.append(f"two:{spec}")
+            return _completed(cmd, 0, stdout="linear-diff")
+        if cmd[3] == "fetch":
+            fetched = True
+            return _completed(cmd, 0)
+        raise AssertionError("unexpected git invocation")
+
+    with patch("ai_reviewer.diff.subprocess.run", side_effect=fake_run):
+        output = run_git_diff(".", "base", "head")
+
+    assert output == "linear-diff"
+    assert any(attempt.startswith("two:") for attempt in diff_attempts)