diff --git a/python/cube/commands/peer_review.py b/python/cube/commands/peer_review.py
index c3c961e7..a264fb48 100644
--- a/python/cube/commands/peer_review.py
+++ b/python/cube/commands/peer_review.py
@@ -88,9 +88,7 @@ def _archive_decisions(
# resumed judge will see prompt-level instructions to ignore
# any prior decision content anyway.
pass
- print_info(
- f"📦 Archived {len(matches)} prior decision file(s) → " f".prompts/decisions/archive/{archive_root.name}/"
- )
+ print_info(f"📦 Archived {len(matches)} prior decision file(s) → .prompts/decisions/archive/{archive_root.name}/")
def _format_thread_context(threads: list) -> str:
@@ -333,7 +331,7 @@ def _ensure_pr_review_worktree(
timeout=60,
)
if result.returncode != 0:
- raise RuntimeError(f"PR review worktree sync failed ({' '.join(cmd)}): " f"{result.stderr.strip()}")
+ raise RuntimeError(f"PR review worktree sync failed ({' '.join(cmd)}): {result.stderr.strip()}")
else:
worktree.parent.mkdir(parents=True, exist_ok=True)
add = subprocess.run(
@@ -456,7 +454,7 @@ def _reconcile_panel_results(
),
log_path=None,
recovery_command=(
- f"cube prv {task_id[len('pr-'):]}"
+ f"cube prv {task_id[len('pr-') :]}"
if disk_status == "missing" and task_id.startswith("pr-")
else None
),
@@ -500,7 +498,7 @@ def _reconcile_panel_results(
attempts=existing.attempts,
error_message="Decision file disappeared after panel completion",
log_path=existing.log_path,
- recovery_command=(f"cube prv {task_id[len('pr-'):]}" if task_id.startswith("pr-") else None),
+ recovery_command=(f"cube prv {task_id[len('pr-') :]}" if task_id.startswith("pr-") else None),
forcing_follow_up_used=False,
)
)
@@ -564,7 +562,10 @@ def _run_pr_review(
except Exception as e:
print_warning(f"Could not fetch from origin: {e}")
- if not pr.diff.strip():
+ # Use the cheap boolean check (git diff --quiet, exit-status only)
+ # instead of fetching diff content. Judges go through the worktree
+ # via Read; we don't need the diff string here.
+ if not pr.has_changes():
print_warning("PR has no diff - nothing to review")
raise typer.Exit(0)
@@ -774,7 +775,7 @@ def _run_pr_review(
if missing_judges:
names = ", ".join(j.label for j in missing_judges)
print_warning(
- f"Partial panel failure: {len(missing_judges)} expected judge(s) " f"failed to produce a decision: {names}"
+ f"Partial panel failure: {len(missing_judges)} expected judge(s) failed to produce a decision: {names}"
)
print_info("Continuing with available judge decisions...")
@@ -854,11 +855,15 @@ def _run_pr_review(
# Use AI deduper to intelligently combine/dedupe all feedback AND merge
# human_calls across lenses asking the same question.
+ # Lazily compute the diff right before dedupe — capped at 8000
+ # lines, fetched via local git (no API cap). The dedupe prompt
+ # already truncates to ~8000 chars, so this is the right place
+ # to enforce a hard ceiling.
dedupe_result = run_async(
run_dedupe_agent(
feedback=all_feedback,
existing_comments=existing_comments,
- pr_diff=pr.diff,
+ pr_diff=pr.get_diff(),
pr_number=pr_number,
human_calls=raw_human_calls,
)
@@ -937,7 +942,7 @@ def _run_pr_review(
+ (f" (+{len(gate.reasons) - 3} more)" if len(gate.reasons) > 3 else "")
)
elif post_action == "APPROVE":
- print_success(f"Auto-approve gate passed — posting APPROVE " f"({approvals}/{expected_total} judges APPROVED)")
+ print_success(f"Auto-approve gate passed — posting APPROVE ({approvals}/{expected_total} judges APPROVED)")
# Append the gate body to the summary so the GitHub review body explains
# what the gate saw (judges + reasons, or the clean approval message).
summary = summary + "\n\n" + gate.body
@@ -1165,7 +1170,7 @@ def _run_branch_review(
names = ", ".join(j.label for j in missing_judges)
missing_suffix = f" ⚠️ {len(missing_judges)} judge(s) failed to produce a decision: {names}."
print_warning(
- f"Partial panel failure: {len(missing_judges)} expected judge(s) " f"failed to produce a decision: {names}"
+ f"Partial panel failure: {len(missing_judges)} expected judge(s) failed to produce a decision: {names}"
)
print_info("Continuing with available judge decisions...")
@@ -1238,7 +1243,7 @@ def _run_branch_review(
for inline in dedupe_result.inline_comments:
comment = inline.comment
console.print(
- f"[yellow]{comment.severity.upper():8}[/yellow] " f"{_rich_escape(str(comment.path))}:{comment.line}"
+ f"[yellow]{comment.severity.upper():8}[/yellow] {_rich_escape(str(comment.path))}:{comment.line}"
)
console.print(f"[dim]{_rich_escape(', '.join(inline.judges))}[/dim] {_rich_escape(comment.body)}")
console.print()
diff --git a/python/cube/commands/ui_review.py b/python/cube/commands/ui_review.py
index d7496150..cd9bfbb9 100644
--- a/python/cube/commands/ui_review.py
+++ b/python/cube/commands/ui_review.py
@@ -71,7 +71,18 @@ def _fetch_pr_ui_diff(pr_number: int, repo: Optional[str] = None) -> tuple[str,
raise RuntimeError("gh CLI not installed or not authenticated. Run: gh auth login")
pr = fetch_pr(pr_number, cwd=str(PROJECT_ROOT), repo=repo)
- ui_diff = _filter_ui_diff(pr.diff)
+ # UI review filters the diff by extension. Fetch with a generous
+ # ceiling so a huge mixed PR (lots of non-UI changes before the
+ # first UI file) doesn't get its UI hunks dropped to the
+ # truncation marker. The filter shrinks the result to UI-only;
+ # we cap AFTER filtering so the budget applies to relevant content.
+ raw_diff = pr.get_diff(max_lines=1_000_000)
+ ui_diff = _filter_ui_diff(raw_diff)
+ # Now apply a sane cap to the filtered result so prompt size is
+ # bounded even when the UI portion is genuinely huge.
+ ui_lines = ui_diff.splitlines()
+ if len(ui_lines) > 20_000:
+ ui_diff = "\n".join(ui_lines[:20_000]) + f"\n[... {len(ui_lines) - 20_000} more UI-diff lines truncated ...]"
if not ui_diff.strip():
raise RuntimeError(
@@ -458,7 +469,7 @@ def ui_review_command(
total = len(findings["P0"]) + len(findings["P1"]) + len(findings["P2"])
if total > 0:
print_success(
- f"UI review complete: {len(findings['P0'])} P0, " f"{len(findings['P1'])} P1, {len(findings['P2'])} P2"
+ f"UI review complete: {len(findings['P0'])} P0, {len(findings['P1'])} P1, {len(findings['P2'])} P2"
)
else:
print_warning("No findings collected from judges")
diff --git a/python/cube/github/pulls.py b/python/cube/github/pulls.py
index 2874a65a..225df28c 100644
--- a/python/cube/github/pulls.py
+++ b/python/cube/github/pulls.py
@@ -8,12 +8,25 @@
@dataclass
class PRData:
- """PR metadata and diff content."""
+ """PR metadata + lazy diff access.
+
+ ``diff`` is intentionally **not eagerly fetched** any more. Judges
+ don't read it (they go through the worktree via Read; see
+ ``judge_panel.py:353`` — "Don't pull the whole PR diff. Read files
+ individually — it's faster"). The only consumers are:
+ - the empty-PR check (which doesn't need the content, just a
+ boolean answer — see ``has_changes()``)
+ - the dedupe agent (which truncates to ~8000 chars anyway)
+
+ Eager-fetching the full diff via ``gh pr diff`` was hitting
+ GitHub's 20,000-line API cap (HTTP 406) on large PRs and aborting
+ the entire panel. Now: call ``get_diff()`` lazily only when content
+ is actually needed; ``has_changes()`` for the cheap boolean check.
+ """
number: int
title: str
body: str
- diff: str
head_branch: str
base_branch: str
head_sha: str
@@ -21,11 +34,113 @@ class PRData:
state: str = ""
merged_at: str = ""
+ # Back-compat: callers that read ``pr.diff`` directly. None until
+ # someone calls ``get_diff()`` — the old eager-populated string is
+ # gone. Keeps the attribute name so existing code reading it sees
+ # "" (no diff yet) rather than AttributeError.
+ diff: str = ""
+
+ # Stashed for the lazy diff computation. Populated by ``fetch_pr``;
+ # callers shouldn't read these directly.
+ _cwd: Optional[str] = None
+ _repo: Optional[str] = None
+ # Memoises the uncapped diff so a second get_diff() call with a
+ # larger cap doesn't re-shell out (and isn't stuck with a smaller
+ # cap from a prior call). The capped value lives in ``diff`` for
+ # back-compat; the raw lives here.
+ _uncapped_diff: str = ""
+
@property
def is_merged(self) -> bool:
"""Return True when GitHub reports this PR as merged."""
return self.state.upper() == "MERGED" or bool(self.merged_at)
+ def has_changes(self) -> bool:
+ """Cheap boolean check: does the PR touch any files?
+
+ Uses ``git diff --quiet`` exit status — never fetches content,
+ never trips the 20k-line API cap. Falls back to ``True`` if
+ local git can't answer (caller's empty-bail is "treat as
+ non-empty" so we don't skip a real PR just because git is
+ unreachable).
+
+ Cross-repo safety: when ``_repo`` is set (caller passed
+ ``--repo other/owner``), the local checkout's branches almost
+ certainly don't match the target repo. Skip the local path
+ and let the conservative default (True) hold so a same-named
+ branch in this repo doesn't produce a false-empty answer.
+ """
+ if self._repo:
+ return True
+ if self.head_branch and self.base_branch:
+ try:
+ result = subprocess.run(
+ ["git", "fetch", "--quiet", "origin", self.base_branch, self.head_branch],
+ cwd=self._cwd,
+ capture_output=True,
+ timeout=60,
+ )
+ if result.returncode == 0:
+ # `--quiet` exits 0 if NO diff, 1 if there IS a diff.
+ diff_check = subprocess.run(
+ [
+ "git",
+ "diff",
+ "--quiet",
+ f"origin/{self.base_branch}...origin/{self.head_branch}",
+ ],
+ cwd=self._cwd,
+ capture_output=True,
+ timeout=30,
+ )
+ # git diff --quiet exit code:
+ # 0 = NO diff
+ # 1 = HAS diff
+ # >1 = error (bad ref, etc — fall through to True)
+ if diff_check.returncode == 0:
+ return False
+ if diff_check.returncode == 1:
+ return True
+ # Any other code = git couldn't answer; conservative
+ # fallback (let the review proceed).
+ except (OSError, subprocess.TimeoutExpired):
+ pass
+ # Fallback: assume there are changes (don't skip the review).
+ return True
+
+ def get_diff(self, *, max_lines: int = 8000) -> str:
+ """Return the PR's diff, capped at ``max_lines`` lines.
+
+ Strategy:
+ 1. Local-git path: ``git diff origin/...origin/``
+ in the caller's cwd. No API call, no size cap from GitHub.
+ 2. Fallback to ``gh pr diff`` only when local git can't
+ resolve the refs.
+ 3. Truncate at ``max_lines`` per-call so each caller's cap is
+ independent. The uncapped diff is memoised on the instance,
+ so a 20k-line call after an 8k-line call recovers the lines
+ the smaller cap dropped — and the second call doesn't pay
+ a second git/gh round-trip.
+ """
+ if not self._uncapped_diff:
+ self._uncapped_diff = _fetch_pr_diff(
+ self.number,
+ head_branch=self.head_branch,
+ base_branch=self.base_branch,
+ cwd=self._cwd,
+ repo_flags=(["--repo", self._repo] if self._repo else []),
+ )
+ # Truncate per-call so each caller's cap is independent.
+ lines = self._uncapped_diff.splitlines()
+ if len(lines) > max_lines:
+ text = "\n".join(lines[:max_lines]) + f"\n[... {len(lines) - max_lines} more lines truncated ...]"
+ else:
+ text = self._uncapped_diff
+ # Keep self.diff populated for back-compat readers, with the
+ # most-recently-requested cap.
+ self.diff = text
+ return text
+
def check_gh_installed() -> bool:
"""Check if gh CLI is installed and authenticated."""
@@ -82,31 +197,160 @@ def fetch_pr(pr_number: int, cwd: Optional[str] = None, repo: Optional[str] = No
except json.JSONDecodeError as e:
raise RuntimeError(f"Failed to parse PR data: {e}")
- # Get diff separately
- diff_result = subprocess.run(
- ["gh", "pr", "diff", str(pr_number)] + repo_flags,
- capture_output=True,
- text=True,
- cwd=cwd,
- )
-
- if diff_result.returncode != 0:
- raise RuntimeError(f"Failed to fetch PR diff: {diff_result.stderr.strip()}")
-
+ # NOTE: we deliberately do NOT eagerly fetch the diff here. Most
+ # callers (the judge panel, panel orchestration) never read it —
+ # judges go through the worktree via Read instead. The only
+ # consumers are the empty-PR check (cheap boolean via
+ # ``has_changes()``) and the dedupe agent (lazy via ``get_diff()``,
+ # truncated at 8000 lines).
+ #
+ # The old eager ``gh pr diff`` call was hitting GitHub's 20,000-line
+ # API cap (HTTP 406) on large PRs, aborting peer review before any
+ # judge ran. Lazy + local-git-first removes the cap entirely.
return PRData(
number=data.get("number", pr_number),
title=data.get("title", ""),
body=data.get("body") or "",
- diff=diff_result.stdout,
head_branch=data.get("headRefName", ""),
base_branch=data.get("baseRefName", ""),
head_sha=data.get("headRefOid", ""),
existing_reviews=data.get("reviews") or [],
state=data.get("state", ""),
merged_at=data.get("mergedAt") or "",
+ _cwd=cwd,
+ _repo=repo,
)
+def _fetch_pr_diff(
+ pr_number: int,
+ *,
+ head_branch: str,
+ base_branch: str,
+ cwd: Optional[str],
+ repo_flags: list[str],
+) -> str:
+ """Return the PR's diff text.
+
+ Strategy:
+ 1. Local-git path (fast, no API cap): ``git fetch origin ``
+ then ``git diff origin/...origin/`` (three-dot —
+ diff from merge-base, matching what GitHub computes).
+ 2. Fall back to ``gh pr diff `` only when (1) fails — e.g. cube
+ is invoked outside a checkout, or origin doesn't track the PR.
+ The fallback can hit GitHub's 20,000-line hard cap (HTTP 406);
+ we surface that with a clear message pointing at the workaround.
+
+ Why this matters: large PRs (e.g. >20k LOC diff, large vendor
+ bumps, generated-code regenerations) silently broke ``cube prv``
+ on the API path. The local checkout has the full diff with no cap.
+ """
+ # Cross-repo safety: when the caller targeted a non-default repo
+ # via ``--repo other/owner``, the LOCAL checkout's branches don't
+ # belong to that repo. A same-named local branch would produce a
+ # diff from this repo instead of the requested one — silently
+ # wrong output. Skip the local path entirely in that case.
+ cross_repo = bool(repo_flags)
+ if head_branch and base_branch and not cross_repo:
+ local_diff = _try_local_git_diff(
+ head_branch=head_branch,
+ base_branch=base_branch,
+ cwd=cwd,
+ )
+ if local_diff is not None:
+ return local_diff
+
+ # API fallback. Same shape as the legacy path so callers see the
+ # same diff format (unified diff text) on either route. Bounded
+ # timeout to match the local-git path — a stalled gh call must
+ # not hang the panel indefinitely (this PR exists for the cases
+ # where the GH API is slow or angry on large PRs).
+ try:
+ diff_result = subprocess.run(
+ ["gh", "pr", "diff", str(pr_number)] + repo_flags,
+ capture_output=True,
+ text=True,
+ cwd=cwd,
+ timeout=120,
+ )
+ except subprocess.TimeoutExpired as e:
+ raise RuntimeError(
+ f"`gh pr diff {pr_number}` timed out after 120s. "
+ f"For large PRs, run cube from inside the checkout so the "
+ f"local-git diff path can be used instead."
+ ) from e
+ if diff_result.returncode != 0:
+ stderr = diff_result.stderr.strip()
+ hint = ""
+ if "exceeded the maximum" in stderr or "too_large" in stderr or "406" in stderr:
+ hint = (
+ "\n\nThe PR is too large for GitHub's diff API (20,000-line cap). "
+ "cube normally falls back to local `git diff` for this case — "
+ "ensure the cwd is inside the cube checkout and `origin` has the "
+ f"PR's head + base refs (`git fetch origin {head_branch} {base_branch}`)."
+ )
+ raise RuntimeError(f"Failed to fetch PR diff: {stderr}{hint}")
+ return diff_result.stdout
+
+
+def _try_local_git_diff(
+ *,
+ head_branch: str,
+ base_branch: str,
+ cwd: Optional[str],
+) -> Optional[str]:
+ """Compute the three-dot diff via local git. Returns None on any
+ failure so the caller falls back to the API path.
+
+ Three-dot intentionally matches what ``gh pr diff`` returns (diff
+ from the merge-base, not the current tip of the base branch) —
+ keeps caller assertions stable across the two paths.
+
+ Fetches origin first so refs are fresh; uses ``--no-tags`` and
+ ``--depth=...`` aren't used here because cube's CI / dev
+ environments are usually full clones. Failures (no origin,
+ detached HEAD, network) all degrade silently to the API path.
+ """
+ if not head_branch or not base_branch:
+ return None
+
+ try:
+ # Ensure the refs are present locally. -q to suppress noise.
+ # Failing to fetch (offline, no origin) is not fatal — we just
+ # try the diff with whatever's already there.
+ subprocess.run(
+ ["git", "fetch", "--quiet", "origin", base_branch, head_branch],
+ cwd=cwd,
+ capture_output=True,
+ timeout=60,
+ )
+ except (OSError, subprocess.TimeoutExpired):
+ # Network / git unavailable — let diff try with stale local
+ # refs; the diff command itself reports failure if neither is
+ # resolvable.
+ pass
+
+ try:
+ diff_result = subprocess.run(
+ [
+ "git",
+ "diff",
+ "--no-color",
+ f"origin/{base_branch}...origin/{head_branch}",
+ ],
+ cwd=cwd,
+ capture_output=True,
+ text=True,
+ check=False,
+ timeout=120,
+ )
+ except (OSError, subprocess.TimeoutExpired):
+ return None
+ if diff_result.returncode != 0:
+ return None
+ return diff_result.stdout
+
+
def fetch_failed_ci_logs(pr_number: int, cwd: Optional[str] = None, max_log_lines: int = 200) -> list[dict]:
"""Fetch logs from failed CI/GHA checks on a PR.
diff --git a/tests/cli/test_pulls_local_diff.py b/tests/cli/test_pulls_local_diff.py
new file mode 100644
index 00000000..77113947
--- /dev/null
+++ b/tests/cli/test_pulls_local_diff.py
@@ -0,0 +1,409 @@
+"""Tests for the lazy / local-git-first PR diff path.
+
+Why this matters: judges go through the worktree via Read, NOT the
+PR diff. ``judge_panel.py:353``: *"Don't pull the whole PR diff. Read
+files individually — it's faster."* Eagerly fetching the diff via
+``gh pr diff`` was hitting GitHub's 20,000-line API cap (HTTP 406)
+and aborting peer review for no reason — judges never read it.
+
+The fix has two parts:
+
+1. **fetch_pr no longer eagerly fetches the diff** — it stores
+ cwd/repo on the PRData and returns immediately. Cuts a network
+ round-trip from every panel launch.
+
+2. **PRData.has_changes() / PRData.get_diff()** lazily compute when
+ the (rare) caller actually needs them. has_changes() is a cheap
+ ``git diff --quiet`` boolean. get_diff() is the local-first /
+ API-fallback path, capped at max_lines so the dedupe consumer
+ never sees more than it uses.
+"""
+
+from __future__ import annotations
+
+import json
+import subprocess
+
+import pytest
+from cube.github import pulls
+
+
+def _gh_view_stub(head: str = "feat/big-branch", base: str = "main") -> str:
+ return json.dumps(
+ {
+ "number": 1234,
+ "title": "Big PR",
+ "body": "body",
+ "headRefName": head,
+ "baseRefName": base,
+ "headRefOid": "abc123",
+ "reviews": [],
+ "state": "OPEN",
+ "mergedAt": None,
+ }
+ )
+
+
+# ---------------------------------------------------------------------------
+# fetch_pr no longer eagerly fetches the diff at all.
+# ---------------------------------------------------------------------------
+
+
+def test_fetch_pr_does_not_eagerly_fetch_diff(monkeypatch):
+ """Hot-path contract: ``fetch_pr`` only calls ``gh pr view``.
+ No diff command (git or gh) runs eagerly — judges never read it,
+ so paying for it on every panel launch was pure overhead AND
+ risked the 20k-line API cap."""
+ calls: list[list[str]] = []
+
+ def _fake_run(cmd, *args, **kwargs):
+ calls.append(list(cmd))
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ assert data.diff == "", "diff must be empty until get_diff() is called"
+ assert not any(cmd[:3] == ["gh", "pr", "diff"] for cmd in calls), "API diff must NOT be eager"
+ assert not any(cmd[:3] == ["git", "diff", "--no-color"] for cmd in calls), "local diff must NOT be eager"
+
+
+# ---------------------------------------------------------------------------
+# get_diff: lazy, local-git first, API fallback.
+# ---------------------------------------------------------------------------
+
+
+def test_get_diff_uses_local_git_when_refs_resolvable(monkeypatch):
+ """When something does call get_diff(), local git is tried first.
+ API NOT touched in the happy case (so the 20k cap can't bite)."""
+ calls: list[list[str]] = []
+
+ def _fake_run(cmd, *args, **kwargs):
+ calls.append(list(cmd))
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="diff --git a/x b/x\n+content\n", stderr="")
+ if cmd[:3] == ["gh", "pr", "diff"]:
+ raise AssertionError("API path called despite local git succeeding")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ diff = data.get_diff()
+ assert "content" in diff
+ assert any(cmd[:3] == ["git", "diff", "--no-color"] for cmd in calls)
+
+
+def test_get_diff_falls_back_to_api_when_local_git_fails(monkeypatch):
+ """Local git refs unresolvable → fall back to API. Keeps cube
+ usable outside a checkout."""
+ api_called = {"v": False}
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ return subprocess.CompletedProcess(cmd, 128, stdout="", stderr="unknown revision")
+ if cmd[:3] == ["gh", "pr", "diff"]:
+ api_called["v"] = True
+ return subprocess.CompletedProcess(cmd, 0, stdout="api-diff", stderr="")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ assert data.get_diff() == "api-diff"
+ assert api_called["v"] is True
+
+
+def test_get_diff_is_memoised(monkeypatch):
+ """Two calls only invoke git once. Cheap consumers (the dedupe
+ path) shouldn't pay twice if has_changes() already triggered it."""
+ git_diff_calls = [0]
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ git_diff_calls[0] += 1
+ return subprocess.CompletedProcess(cmd, 0, stdout="diff\n", stderr="")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ data.get_diff()
+ data.get_diff()
+ assert git_diff_calls[0] == 1
+
+
+def test_get_diff_respects_max_lines_independently_per_call(monkeypatch):
+ """Memoisation caches the UNCAPPED diff. A larger cap on a later
+ call must recover the lines the smaller cap dropped — without
+ paying for a second git/gh round-trip. Without this, the first
+ caller's max_lines silently constrains later callers."""
+ git_diff_calls = [0]
+ big_diff = "\n".join(f"line {i}" for i in range(500))
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ git_diff_calls[0] += 1
+ return subprocess.CompletedProcess(cmd, 0, stdout=big_diff, stderr="")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ small = data.get_diff(max_lines=50)
+ assert "truncated" in small
+ big = data.get_diff(max_lines=1000)
+ assert "truncated" not in big, "later larger-cap call must recover the lines"
+ assert len(big.splitlines()) == 500
+ assert git_diff_calls[0] == 1, "second call must use the memoised uncapped diff"
+
+
+def test_api_fallback_has_timeout(monkeypatch):
+ """Stalled ``gh pr diff`` must not hang the panel forever.
+ subprocess.TimeoutExpired raised by gh propagates as a clear
+ RuntimeError so cube prints something actionable."""
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ return subprocess.CompletedProcess(cmd, 128, stdout="", stderr="bad ref")
+ if cmd[:3] == ["gh", "pr", "diff"]:
+ # The contract: the call MUST pass a timeout= kwarg. Pin
+ # it by inspecting kwargs rather than letting the stub
+ # silently accept anything.
+ assert "timeout" in kwargs, "gh pr diff must be called with a timeout"
+ raise subprocess.TimeoutExpired(cmd, kwargs["timeout"])
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ with pytest.raises(RuntimeError) as exc:
+ data.get_diff()
+ assert "timed out" in str(exc.value)
+
+
+def test_get_diff_truncates_to_max_lines(monkeypatch):
+ """The dedupe agent caps prompt input at ~8k chars. get_diff()
+ enforces a hard line ceiling so a 200k-line PR can't blow the
+ prompt budget even when the API somehow returns it."""
+ big_diff = "\n".join(f"+line {i}" for i in range(20_000))
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=big_diff, stderr="")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ diff = data.get_diff(max_lines=100)
+ assert "truncated" in diff
+ assert len(diff.splitlines()) == 101 # 100 kept + the truncation marker line
+
+
+# ---------------------------------------------------------------------------
+# 20k-line API failure surfaces a workaround hint.
+# ---------------------------------------------------------------------------
+
+
+def test_too_large_api_error_includes_workaround_hint(monkeypatch):
+ """Both local git and gh fail; the API failure is the 20k cap.
+ The error must point operators at the local-git workaround so
+ they don't have to grep cube source."""
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--no-color"]:
+ return subprocess.CompletedProcess(cmd, 128, stdout="", stderr="")
+ if cmd[:3] == ["gh", "pr", "diff"]:
+ return subprocess.CompletedProcess(
+ cmd,
+ 1,
+ stdout="",
+ stderr="HTTP 406: Sorry, the diff exceeded the maximum number of lines (20000)\nPullRequest.diff too_large",
+ )
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ with pytest.raises(RuntimeError) as exc:
+ data.get_diff()
+ msg = str(exc.value)
+ assert "20,000-line cap" in msg or "exceeded the maximum" in msg
+ assert "git fetch origin" in msg, "must surface the workaround command"
+
+
+# ---------------------------------------------------------------------------
+# has_changes: cheap boolean for the empty-PR bail.
+# ---------------------------------------------------------------------------
+
+
+def test_has_changes_true_when_diff_exists(monkeypatch):
+ """`git diff --quiet` exits 1 when there IS a diff. has_changes
+ interprets that as True so the empty-PR bail proceeds correctly."""
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--quiet"]:
+ return subprocess.CompletedProcess(cmd, 1, stdout="", stderr="") # 1 = HAS diff
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ assert data.has_changes() is True
+
+
+def test_has_changes_false_on_identical_refs(monkeypatch):
+ """`git diff --quiet` exits 0 when there's NO diff. has_changes
+ returns False so the caller bails the review (empty PR)."""
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--quiet"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") # 0 = NO diff
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ assert data.has_changes() is False
+
+
+def test_has_changes_falls_back_to_true_on_git_error_exit(monkeypatch):
+ """`git diff --quiet` exits 0 (no diff), 1 (diff), >1 (error).
+ Treating any non-0 as True would mask a real error as "has diff";
+ treating non-1 as False would mask an error as "empty PR" and
+ skip the review. Conservative default on error: True so the
+ review proceeds rather than silently bailing."""
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+ if cmd[:3] == ["git", "diff", "--quiet"]:
+ # 128 = bad revision (git's standard error code)
+ return subprocess.CompletedProcess(cmd, 128, stdout="", stderr="bad revision")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ # NOT False (would falsely report empty PR); NOT crash. Conservative True.
+ assert data.has_changes() is True
+
+
+def test_has_changes_skips_local_path_when_cross_repo(monkeypatch):
+ """`--repo other/owner` means the local checkout's branches
+ belong to THIS repo, not the target. A same-named branch would
+ answer based on this repo's diff — silently wrong. Skip the
+ local path entirely; default True so the review proceeds."""
+ git_called = {"v": False}
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"] or cmd[:3] == ["git", "diff", "--quiet"]:
+ git_called["v"] = True
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234, repo="other/owner")
+ assert data.has_changes() is True
+ assert git_called["v"] is False, "local git must NOT be consulted for cross-repo PRs"
+
+
+def test_get_diff_skips_local_path_when_cross_repo(monkeypatch):
+ """Same cross-repo safety for the content diff: skip local git,
+ go straight to ``gh pr diff --repo other/owner`` so the diff
+ actually comes from the targeted repo."""
+ calls: list[list[str]] = []
+
+ def _fake_run(cmd, *args, **kwargs):
+ calls.append(list(cmd))
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:3] == ["gh", "pr", "diff"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout="cross-repo diff content", stderr="")
+ # Any local git call here means the cross-repo guard failed.
+ if cmd[:2] == ["git", "fetch"] or cmd[:3] == ["git", "diff", "--no-color"]:
+ raise AssertionError("local git must NOT be consulted for cross-repo PR diff")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234, repo="other/owner")
+ assert data.get_diff() == "cross-repo diff content"
+ assert any(
+ cmd[:3] == ["gh", "pr", "diff"] and "--repo" in cmd for cmd in calls
+ ), "cross-repo fetch must pass --repo flag to gh"
+
+
+def test_has_changes_defaults_to_true_on_git_failure(monkeypatch):
+ """If git can't answer (offline, no origin, etc), assume the PR
+ has changes — better than silently skipping a real PR because the
+ network blipped."""
+
+ def _fake_run(cmd, *args, **kwargs):
+ if cmd[:3] == ["gh", "pr", "view"]:
+ return subprocess.CompletedProcess(cmd, 0, stdout=_gh_view_stub(), stderr="")
+ if cmd[:2] == ["git", "fetch"]:
+ return subprocess.CompletedProcess(cmd, 128, stdout="", stderr="fatal: no origin")
+ return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+ monkeypatch.setattr(pulls, "check_gh_installed", lambda: True)
+ monkeypatch.setattr(subprocess, "run", _fake_run)
+
+ data = pulls.fetch_pr(1234)
+ assert data.has_changes() is True