fix: raise GitTimeoutError instead of returning empty on git/svn timeout

code_review_graph/changes.py

@@ -33,6 +33,22 @@
 
 _GIT_TIMEOUT = int(os.environ.get("CRG_GIT_TIMEOUT", "30"))  # seconds, configurable
 
+
+class GitTimeoutError(RuntimeError):
+    """A git/svn command exceeded CRG_GIT_TIMEOUT (#913).
+
+    The caller cannot distinguish a timeout from an empty diff via the
+    empty dict alone; this exception makes the difference explicit.
+    """
+
+    def __init__(self, command: str, timeout: int) -> None:
+        self.command = command
+        self.timeout = timeout
+        super().__init__(
+            f"{command} timed out after {timeout}s; "
+            f"the diff was not read so the review analysed nothing"
+        )
+
 _SAFE_GIT_REF = re.compile(r"^[A-Za-z0-9_.~^/@{}\-]+$")
 _SAFE_SVN_REV = re.compile(r"^r?\d+(:r?\d+|:HEAD|:BASE|:COMMITTED)?$", re.IGNORECASE)
 
@@ -73,6 +89,8 @@ def parse_git_diff_ranges(
         if result.returncode != 0:
             logger.warning("git diff failed (rc=%d): %s", result.returncode, result.stderr[:200])
             return {}
+    except subprocess.TimeoutExpired as exc:
+        raise GitTimeoutError("git diff", _GIT_TIMEOUT) from exc
     except (OSError, subprocess.SubprocessError) as exc:
         logger.warning("git diff error: %s", exc)
         return {}
@@ -115,6 +133,8 @@ def parse_svn_diff_ranges(
         if result.returncode != 0:
             logger.warning("svn diff failed (rc=%d): %s", result.returncode, result.stderr[:200])
             return {}
+    except subprocess.TimeoutExpired as exc:
+        raise GitTimeoutError("svn diff", _GIT_TIMEOUT) from exc
     except (OSError, subprocess.SubprocessError) as exc:
         logger.warning("svn diff error: %s", exc)
         return {}

tests/test_git_timeout.py

@@ -0,0 +1,49 @@
+"""#913 — git/svn timeout must not masquerade as 'no changes'."""
+import subprocess
+from unittest.mock import patch
+
+import pytest
+
+from code_review_graph.changes import GitTimeoutError, parse_git_diff_ranges
+
+
+class TestGitTimeoutError:
+    def test_is_runtime_error(self):
+        assert issubclass(GitTimeoutError, RuntimeError)
+
+    def test_message_names_command_and_timeout(self):
+        exc = GitTimeoutError("git diff", 30)
+        assert "git diff" in str(exc)
+        assert "30" in str(exc)
+        assert "timed out" in str(exc)
+
+    def test_carries_command_and_timeout_attributes(self):
+        exc = GitTimeoutError("svn diff", 45)
+        assert exc.command == "svn diff"
+        assert exc.timeout == 45
+
+
+class TestParseGitDiffRangesTimeout:
+    def test_timeout_raises_git_timeout_error(self, tmp_path):
+        with patch(
+            "code_review_graph.changes.subprocess.run",
+            side_effect=subprocess.TimeoutExpired(cmd="git diff", timeout=30),
+        ):
+            with pytest.raises(GitTimeoutError, match="git diff.*timed out.*30"):
+                parse_git_diff_ranges(str(tmp_path), "HEAD")
+
+    def test_non_timeout_subprocess_error_still_returns_empty(self, tmp_path):
+        with patch(
+            "code_review_graph.changes.subprocess.run",
+            side_effect=subprocess.CalledProcessError(1, "git diff"),
+        ):
+            result = parse_git_diff_ranges(str(tmp_path), "HEAD")
+            assert result == {}
+
+    def test_oserror_still_returns_empty(self, tmp_path):
+        with patch(
+            "code_review_graph.changes.subprocess.run",
+            side_effect=OSError("git not found"),
+        ):
+            result = parse_git_diff_ranges(str(tmp_path), "HEAD")
+            assert result == {}