diff --git a/src/ai_reviewer/__pycache__/diff.cpython-311.pyc b/src/ai_reviewer/__pycache__/diff.cpython-311.pyc index 02caa6b..9ae434f 100644 Binary files a/src/ai_reviewer/__pycache__/diff.cpython-311.pyc and b/src/ai_reviewer/__pycache__/diff.cpython-311.pyc differ diff --git a/src/ai_reviewer/diff.py b/src/ai_reviewer/diff.py index f677305..5d0bf7c 100644 --- a/src/ai_reviewer/diff.py +++ b/src/ai_reviewer/diff.py @@ -46,7 +46,7 @@ def run_git_diff(repo: str, base: str, head: str) -> str: base_candidates = _build_ref_candidates(base) head_candidates = _build_ref_candidates(head) - stdout, stderr = _diff_with_candidates(repo, base_candidates, head_candidates) + stdout, stderr, fallback_pairs = _diff_with_candidates(repo, base_candidates, head_candidates) if stdout is not None: return stdout @@ -60,12 +60,20 @@ def run_git_diff(repo: str, base: str, head: str) -> str: # Ignore fetch errors; we'll raise the original error below. pass - stdout, stderr = _diff_with_candidates(repo, base_candidates, head_candidates) + 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(stderr or first_error or "git diff failed") + raise RuntimeError(fallback_error or stderr or first_error or "git diff failed") def _build_ref_candidates(ref: str) -> list[str]: @@ -75,26 +83,24 @@ def _build_ref_candidates(ref: str) -> list[str]: return candidates -def _diff_with_candidates(repo: str, base_candidates: list[str], head_candidates: list[str]) -> tuple[str | None, str]: +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: - 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) + result = _run_git_diff_command(repo, base_ref, head_ref, symmetric=True) if result.returncode in (0, 1): - return result.stdout, "" + return result.stdout, "", [] if result.stderr: - last_error = result.stderr.strip() - return None, last_error + 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]: @@ -193,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 diff --git a/tests/__pycache__/test_diff.cpython-311-pytest-9.0.2.pyc b/tests/__pycache__/test_diff.cpython-311-pytest-9.0.2.pyc index f12f287..70cd5e8 100644 Binary files a/tests/__pycache__/test_diff.cpython-311-pytest-9.0.2.pyc and b/tests/__pycache__/test_diff.cpython-311-pytest-9.0.2.pyc differ diff --git a/tests/test_diff.py b/tests/test_diff.py index 2c6fbf6..604e808 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -38,8 +38,11 @@ def test_run_git_diff_falls_back_to_origin_refs_after_fetch(): 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 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") @@ -55,3 +58,29 @@ def test_run_git_diff_falls_back_to_origin_refs_after_fetch(): 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)