fixed with codex

src/ai_reviewer/diff.py

@@ -43,36 +43,58 @@ class DiffChunk:
 
 
 def run_git_diff(repo: str, base: str, head: str) -> str:
-    cmd = [
+    base_candidates = _build_ref_candidates(base)
-        "git",
+    head_candidates = _build_ref_candidates(head)
-        "-C",
+
-        repo,
+    stdout, stderr = _diff_with_candidates(repo, base_candidates, head_candidates)
-        "diff",
+    if stdout is not None:
-        f"{base}...{head}",
+        return stdout
-        "--unified=3",
-        "--no-color",
-    ]
-    result = subprocess.run(cmd, check=False, capture_output=True, text=True)
-    if result.returncode in (0, 1):
-        return result.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.
-    stderr = (result.stderr or "").strip()
+    first_error = stderr
     try:
-        fetch_cmd = ["git", "-C", repo, "fetch", "origin", f"{base}", f"{head}"]
+        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
 
-    # Retry diff once
+    stdout, stderr = _diff_with_candidates(repo, base_candidates, head_candidates)
-    result2 = subprocess.run(cmd, check=False, capture_output=True, text=True)
+    if stdout is not None:
-    if result2.returncode in (0, 1):
+        return stdout
-        return result2.stdout
 
     # If still failing, raise a helpful error including stderr from git
-    raise RuntimeError(result2.stderr.strip() or stderr or "git diff failed")
+    raise RuntimeError(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]:
+    last_error = ""
+    for base_ref in base_candidates:
+        for head_ref in head_candidates:
+            cmd = [
+                "git",
+                "-C",
+                repo,
+                "diff",
+                f"{base_ref}...{head_ref}",
+                "--unified=3",
+                "--no-color",
+            ]
+            result = subprocess.run(cmd, check=False, capture_output=True, text=True)
+            if result.returncode in (0, 1):
+                return result.stdout, ""
+
+            if result.stderr:
+                last_error = result.stderr.strip()
+    return None, last_error
 
 
 def parse_diff(diff_text: str) -> list[FileDiff]:

tests/test_diff.py

@@ -0,0 +1,57 @@
+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":
+            diff_attempts.append(cmd[4])
+            spec = cmd[4]
+            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")