From c5d5968e905236ef94ecde80eddca1bd52f87418 Mon Sep 17 00:00:00 2001
From: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
Date: Mon, 18 May 2026 15:03:46 +0000
Subject: [PATCH] =?UTF-8?q?fix(triage):=20greptile=20=E2=80=94=20fail-safe?=
 =?UTF-8?q?=20fetch=5Fpr=5Fcomments=20so=20a=20single=20API=20hiccup=20doe?=
 =?UTF-8?q?sn't=20abort=20the=20sweep?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fetch_pr_author_association already wraps gh api in a try/except
CalledProcessError to keep the daily sweep moving when the lookup for
one PR transiently fails. fetch_pr_comments lacked the same guard — a
single 5xx mid-loop would crash the run before later PRs were
evaluated. Mirror the fail-safe pattern: return [] on CalledProcessError
or malformed paginated JSON; downstream that becomes a conservative
skip-no-greptile-score for the affected PR and the sweep continues to
the next one (which will be re-evaluated tomorrow).

Pinned with two new TestFetchPrComments tests.
---
 .github/scripts/close_low_quality_prs.py      | 18 ++++++++++---
 .../test_github_close_low_quality_prs.py      | 25 +++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/.github/scripts/close_low_quality_prs.py b/.github/scripts/close_low_quality_prs.py
index 9be0d6e9e0f..a05636d5376 100644
--- a/.github/scripts/close_low_quality_prs.py
+++ b/.github/scripts/close_low_quality_prs.py
@@ -154,19 +154,31 @@ def is_external_pr_author(pr: dict, repo: str | None) -> bool:
 
 
 def fetch_pr_comments(pr_number: int, repo: str | None) -> list[dict]:
-    """Fetch issue-level comments on a PR (where Greptile posts its summary)."""
+    """Fetch issue-level comments on a PR (where Greptile posts its summary).
+
+    Returns [] on API failure so a transient hiccup on any single PR doesn't
+    abort the whole daily sweep mid-loop. Matches the fail-safe pattern in
+    `fetch_pr_author_association`; downstream the empty list becomes a
+    `skip-no-greptile-score` action and the PR is re-evaluated on the next run.
+    """
     endpoint = (
         f"repos/{repo}/issues/{pr_number}/comments?per_page=100"
         if repo
         else f"repos/{{owner}}/{{repo}}/issues/{pr_number}/comments?per_page=100"
     )
-    raw = gh("api", "--paginate", endpoint)
+    try:
+        raw = gh("api", "--paginate", endpoint)
+    except subprocess.CalledProcessError:
+        return []
     comments: list[dict] = []
     for line in raw.strip().splitlines():
         line = line.strip()
         if not line:
             continue
-        parsed = json.loads(line)
+        try:
+            parsed = json.loads(line)
+        except json.JSONDecodeError:
+            return []
         if isinstance(parsed, list):
             comments.extend(parsed)
         else:
diff --git a/tests/test_litellm/test_github_close_low_quality_prs.py b/tests/test_litellm/test_github_close_low_quality_prs.py
index 7050683b681..3b2918324b1 100644
--- a/tests/test_litellm/test_github_close_low_quality_prs.py
+++ b/tests/test_litellm/test_github_close_low_quality_prs.py
@@ -113,6 +113,31 @@ class TestExtractGreptileScore:
         assert closer_module.extract_greptile_score([]) is None
 
 
+class TestFetchPrComments:
+    """`fetch_pr_comments` must fail-safe so a transient `gh api` error on
+    one PR doesn't abort the whole daily sweep mid-loop. Pinning the
+    empty-list return matches the fail-safe pattern in
+    `fetch_pr_author_association`.
+    """
+
+    def test_should_return_empty_list_when_gh_api_fails(
+        self, closer_module, monkeypatch
+    ):
+        import subprocess
+
+        def _failing_gh(*args, **kwargs):
+            raise subprocess.CalledProcessError(1, ["gh", *args])
+
+        monkeypatch.setattr(closer_module, "gh", _failing_gh)
+        assert closer_module.fetch_pr_comments(123, repo="x/y") == []
+
+    def test_should_return_empty_list_when_paginated_output_is_malformed(
+        self, closer_module, monkeypatch
+    ):
+        monkeypatch.setattr(closer_module, "gh", lambda *a, **kw: "not json\n")
+        assert closer_module.fetch_pr_comments(123, repo="x/y") == []
+
+
 class TestEvaluatePr:
     @pytest.fixture(autouse=True)
     def _now(self):