From 190430c7aaa27aff5bdd328bec3139db48d9c287 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 00:20:28 -0700 Subject: [PATCH 1/6] Read CodeRabbit's Command-Reply Rate Limit in pr_review.py An explicit `@coderabbitai review` that CodeRabbit cannot run gets back an `auto-generated reply` marker over an `Action not completed` block saying `Review rate limited.`, a shape RATE_LIMITED never matched. On a repository where CodeRabbit never auto-reviews, that was the only rate limit the digest could meet, so `other_rate_limited` read the same for "asked and limited" as for "never asked". rate_limited_by now reads that shape from its markers and fixed block text, naming the service by login since the marker names none, and surfaces it in the same `other_rate_limited` field. The auto-review marker reads as before. Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 33 ++++++++++++++++++++-------- tests/test_pr_review.py | 48 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 6cba24a0a..7ff714b7a 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -142,7 +142,10 @@ carries its own rate-limit marker instead, a structural `` convention rather than free-text prose (observed on CodeRabbit), so reading it needs no per-bot wording model the way - Copilot's quota refusal needed `QUOTA` built for its own text. + Copilot's quota refusal needed `QUOTA` built for its own text. CodeRabbit's reply to an + explicit review command reads the same, an `auto-generated reply` marker over an + `Action not completed` block saying `Review rate limited.`, its service named from the + login since that marker names none. Two of their own finding shapes are read, though, each its own blind spot a thread poll alone cannot see. `cr_outside_diff=N (on_head=X earlier=Y)` counts CodeRabbit's own "outside diff range" findings, collapsed into the review body rather @@ -478,6 +481,11 @@ # The service name is captured rather than assumed. # A second bot using the same auto-generated-comment convention is read without a new pattern, and one that does not use it stays unread rather than guessed at. RATE_LIMITED = re.compile(r"auto-generated comment:\s*rate limited by\s*(\S+?)\s*-->") +RATE_LIMITED_REPLY = re.compile( + r"\s*(?:\s*)*" + r"
\s*[^<\n]*Action not completed\s*\s*Review rate limited\." +) +RATE_LIMIT_SERVICE = {CODERABBIT_LOGIN: "coderabbit.ai"} # Anchored to the `

` tag rather than a bare substring, since prose elsewhere can mention the phrase without being the comment it names. QODO_REVIEW_HEADING = re.compile(r"

\s*Code Review by Qodo\s*

", re.IGNORECASE) # Only the numbered heading counts as a finding, told apart from Qodo's own nested sub-summaries by starting with a number and a period (`1.`, `2.`, ...). @@ -1307,20 +1315,27 @@ def standing_quota( def rate_limited_by(pr: dict, login: str) -> str | None: """The service name a rate-limit marker gives, where this login's newest comment or review - on this pull request carries one. `None` otherwise. + on this pull request carries one. `None` otherwise. CodeRabbit's reply to an explicit review + command, `RATE_LIMITED_REPLY`, is read from its markers and its fixed `Action not completed` + block alone. That marker names no service, so `RATE_LIMIT_SERVICE` names it by login, and a + login missing there is named by itself rather than dropped. - Reads both connections since the one observed marker, CodeRabbit's, rides a - plain PR comment rather than a formal review, and a future bot using the same convention - might use either. Current pull request only, unlike Copilot's `quota_signal`: that one - generalizes from a pattern of silence across several pull requests, and this has one - observed instance to generalize from, not a history to read one out of. + Reads both connections since both observed shapes, each CodeRabbit's, ride a plain PR + comment rather than a formal review, and a future bot using the same convention might use + either. Current pull request only, unlike Copilot's `quota_signal`: that one generalizes + from a pattern of silence across several pull requests, and this reads one notice, not a + history to read one out of. """ nodes = reviewer_nodes(pr, "comments", login) + reviewer_nodes(pr, "reviews", login) if not nodes: return None newest = max(nodes, key=lambda n: n.get("createdAt") or n.get("submittedAt") or "") - match = RATE_LIMITED.search(newest.get("body") or "") - return match.group(1) if match else None + body = newest.get("body") or "" + if match := RATE_LIMITED.search(body): + return match.group(1) + if RATE_LIMITED_REPLY.search(body): + return RATE_LIMIT_SERVICE.get(login, login) + return None def newest_request_event(pr: dict) -> str | None: diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 37f9d801f..968f02f3f 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -323,6 +323,13 @@ def summarized(paths: list[str], covers: str = COVERED) -> str: "> [!WARNING]\n> ## Review limit reached\n" ) +RATE_LIMITED_REPLY_COMMENT = ( + "\n" + "\n" + "
\n\u26a0\ufe0f Action not completed\n\n" + "Review rate limited.\n
\n" +) + def thread( tid: str, @@ -957,6 +964,47 @@ def test_rate_limited_by_reads_either_connection_and_names_no_untracked_login(se self.assertEqual("coderabbit.ai", pr_review.rate_limited_by(pr, "coderabbitai")) self.assertIsNone(pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) + def test_other_rate_limited_reads_the_command_reply_shape(self) -> None: + """The reply to an explicit review command names no service, so the login supplies it.""" + self.answer( + payload( + [review()], + comments=[comment(login="coderabbitai", body=RATE_LIMITED_REPLY_COMMENT)], + ) + ) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("other_rate_limited=coderabbitai:coderabbit.ai", out) + + def test_rate_limited_by_reads_both_shapes_and_not_an_ordinary_comment(self) -> None: + """Constructed bodies, the reply shape's invocation id made up and its glyph escaped. + + The last two are near misses: the reply marker over a block that is not the rate-limit + one, and the block's own words with no reply marker over them. + """ + other_reply = ( + "\n" + "
\nAction not completed\n\nSomething else.\n" + ) + unmarked = "
\nAction not completed\n\nReview rate limited.\n" + summary = "\n" + for body, expected in ( + (RATE_LIMITED_REPLY_COMMENT, "coderabbit.ai"), + (RATE_LIMITED_COMMENT, "coderabbit.ai"), + (summary, None), + (other_reply, None), + (unmarked, None), + ): + with self.subTest(body=body): + pr = payload([review()], comments=[comment(login="coderabbitai", body=body)]) + self.assertEqual(expected, pr_review.rate_limited_by(pr, "coderabbitai")) + + def test_rate_limited_by_names_an_unmapped_login_by_itself(self) -> None: + pr = payload( + [review()], + comments=[comment(login=pr_review.QODO_LOGIN, body=RATE_LIMITED_REPLY_COMMENT)], + ) + self.assertEqual(pr_review.QODO_LOGIN, pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) + def test_rate_limited_by_reads_the_reviews_connection_too(self) -> None: """The marker rides a plain comment on the one observed instance, but the reader checks both connections, since a future bot's own marker might ride a formal review.""" From cb18080a62a2fd9bef790b8067116fdad62a1564 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 00:23:54 -0700 Subject: [PATCH 2/6] Name the Reply Shape's Service by Login Alone Drop the login fallback for an unmapped reviewer, which no input reaches, so a reply-shape notice from a login with no known service stays unread as the auto-review marker's own comment says. Correct the docstrings that said the reply marker names no service, and split the sentences the sentence-length check flagged. Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 28 ++++++++++++++-------------- tests/test_pr_review.py | 15 ++++----------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 7ff714b7a..90f5b0923 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -139,13 +139,13 @@ format and its own future task per bot. Where either has posted anything at all on the current head, `other_reviewed` names it, identity and commit only, no verdict read from what it said. `other_rate_limited` names one whose newest comment or review - carries its own rate-limit marker instead, a structural + carries its own rate-limit marker instead. That marker is a structural `` convention rather than free-text prose (observed on - CodeRabbit), so reading it needs no per-bot wording model the way - Copilot's quota refusal needed `QUOTA` built for its own text. CodeRabbit's reply to an - explicit review command reads the same, an `auto-generated reply` marker over an - `Action not completed` block saying `Review rate limited.`, its service named from the - login since that marker names none. + CodeRabbit). Reading it needs no per-bot wording model the way Copilot's quota refusal + needed `QUOTA` built for its own text. CodeRabbit's reply to an + explicit review command reads the same. That reply is an `auto-generated reply` marker + over an `Action not completed` block saying `Review rate limited.`, and its service is + named from the login. Two of their own finding shapes are read, though, each its own blind spot a thread poll alone cannot see. `cr_outside_diff=N (on_head=X earlier=Y)` counts CodeRabbit's own "outside diff range" findings, collapsed into the review body rather @@ -1317,14 +1317,14 @@ def rate_limited_by(pr: dict, login: str) -> str | None: """The service name a rate-limit marker gives, where this login's newest comment or review on this pull request carries one. `None` otherwise. CodeRabbit's reply to an explicit review command, `RATE_LIMITED_REPLY`, is read from its markers and its fixed `Action not completed` - block alone. That marker names no service, so `RATE_LIMIT_SERVICE` names it by login, and a - login missing there is named by itself rather than dropped. + block alone. Its marker says `CodeRabbit` rather than the `coderabbit.ai` the other shape + gives, so `RATE_LIMIT_SERVICE` names the service by login and both shapes read alike. A login + missing there stays unread. - Reads both connections since both observed shapes, each CodeRabbit's, ride a plain PR - comment rather than a formal review, and a future bot using the same convention might use - either. Current pull request only, unlike Copilot's `quota_signal`: that one generalizes - from a pattern of silence across several pull requests, and this reads one notice, not a - history to read one out of. + Reads both connections, since both observed shapes ride a plain PR comment rather than a + formal review. A future bot using the same convention might use either. Current pull request + only, unlike Copilot's `quota_signal`. That one generalizes from a pattern of silence across + several pull requests, while this reads one notice rather than a history. """ nodes = reviewer_nodes(pr, "comments", login) + reviewer_nodes(pr, "reviews", login) if not nodes: @@ -1334,7 +1334,7 @@ def rate_limited_by(pr: dict, login: str) -> str | None: if match := RATE_LIMITED.search(body): return match.group(1) if RATE_LIMITED_REPLY.search(body): - return RATE_LIMIT_SERVICE.get(login, login) + return RATE_LIMIT_SERVICE.get(login) return None diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 968f02f3f..7626ac81f 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -978,8 +978,8 @@ def test_other_rate_limited_reads_the_command_reply_shape(self) -> None: def test_rate_limited_by_reads_both_shapes_and_not_an_ordinary_comment(self) -> None: """Constructed bodies, the reply shape's invocation id made up and its glyph escaped. - The last two are near misses: the reply marker over a block that is not the rate-limit - one, and the block's own words with no reply marker over them. + The last two are near misses. One is the reply marker over another block, and the other + is the block's own words with no reply marker over them. """ other_reply = ( "\n" @@ -998,16 +998,9 @@ def test_rate_limited_by_reads_both_shapes_and_not_an_ordinary_comment(self) -> pr = payload([review()], comments=[comment(login="coderabbitai", body=body)]) self.assertEqual(expected, pr_review.rate_limited_by(pr, "coderabbitai")) - def test_rate_limited_by_names_an_unmapped_login_by_itself(self) -> None: - pr = payload( - [review()], - comments=[comment(login=pr_review.QODO_LOGIN, body=RATE_LIMITED_REPLY_COMMENT)], - ) - self.assertEqual(pr_review.QODO_LOGIN, pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) - def test_rate_limited_by_reads_the_reviews_connection_too(self) -> None: - """The marker rides a plain comment on the one observed instance, but the reader - checks both connections, since a future bot's own marker might ride a formal review.""" + """Both observed shapes ride a plain comment, but the reader checks both connections, + since a future bot's own marker might ride a formal review.""" pr = payload( [review(), self.other_review("coderabbitai", body=RATE_LIMITED_COMMENT)], comments=[], From 1b9668372f20d3af031659167627972b5c2f184a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 00:28:12 -0700 Subject: [PATCH 3/6] Bound the Reply Pattern and Test the Unmapped Login Keep the reply pattern's comment spans from running past a closing "-->", say the reply lands in the same field by a pattern of its own, move the not-head-scoped reason into the reader's docstring, and test that a reply from a login with no known service stays unread. Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 14 +++++++------- tests/test_pr_review.py | 7 +++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 90f5b0923..fb403ce9f 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -142,10 +142,10 @@ carries its own rate-limit marker instead. That marker is a structural `` convention rather than free-text prose (observed on CodeRabbit). Reading it needs no per-bot wording model the way Copilot's quota refusal - needed `QUOTA` built for its own text. CodeRabbit's reply to an - explicit review command reads the same. That reply is an `auto-generated reply` marker - over an `Action not completed` block saying `Review rate limited.`, and its service is - named from the login. + needed `QUOTA` built for its own text. CodeRabbit's reply to an explicit review command + lands in the same field, read by a pattern of its own for CodeRabbit's fixed wording. + That reply is an `auto-generated reply` marker over an `Action not completed` block + saying `Review rate limited.`, and its service is named from the login. Two of their own finding shapes are read, though, each its own blind spot a thread poll alone cannot see. `cr_outside_diff=N (on_head=X earlier=Y)` counts CodeRabbit's own "outside diff range" findings, collapsed into the review body rather @@ -482,7 +482,7 @@ # A second bot using the same auto-generated-comment convention is read without a new pattern, and one that does not use it stays unread rather than guessed at. RATE_LIMITED = re.compile(r"auto-generated comment:\s*rate limited by\s*(\S+?)\s*-->") RATE_LIMITED_REPLY = re.compile( - r"\s*(?:\s*)*" + r"\s*(?:\s*)*" r"
\s*[^<\n]*Action not completed\s*\s*Review rate limited\." ) RATE_LIMIT_SERVICE = {CODERABBIT_LOGIN: "coderabbit.ai"} @@ -1322,7 +1322,8 @@ def rate_limited_by(pr: dict, login: str) -> str | None: missing there stays unread. Reads both connections, since both observed shapes ride a plain PR comment rather than a - formal review. A future bot using the same convention might use either. Current pull request + formal review. A future bot using the same convention might use either. A plain comment + carries no commit, so the reading is not scoped to the head. Current pull request only, unlike Copilot's `quota_signal`. That one generalizes from a pattern of silence across several pull requests, while this reads one notice rather than a history. """ @@ -4743,7 +4744,6 @@ def digest( ) ] # The service name each tracked other-reviewer's own rate-limit marker gives, where its newest comment or review on this pull request carries one. - # Not head-scoped, unlike `other_on_head`, since the one observed marker rides a plain comment, which carries no commit to compare against the head at all. other_limited = { login: name for login in OTHER_REVIEWERS if (name := rate_limited_by(pr, login)) } diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 7626ac81f..9653123db 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -998,6 +998,13 @@ def test_rate_limited_by_reads_both_shapes_and_not_an_ordinary_comment(self) -> pr = payload([review()], comments=[comment(login="coderabbitai", body=body)]) self.assertEqual(expected, pr_review.rate_limited_by(pr, "coderabbitai")) + def test_rate_limited_by_leaves_the_reply_shape_unread_for_an_unmapped_login(self) -> None: + pr = payload( + [review()], + comments=[comment(login=pr_review.QODO_LOGIN, body=RATE_LIMITED_REPLY_COMMENT)], + ) + self.assertIsNone(pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) + def test_rate_limited_by_reads_the_reviews_connection_too(self) -> None: """Both observed shapes ride a plain comment, but the reader checks both connections, since a future bot's own marker might ride a formal review.""" From 243f67483035719477a465b45d34cffb17108284 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 00:31:18 -0700 Subject: [PATCH 4/6] Bring the Rate-Limit Comments and Docstrings in Line With Both Shapes The comments near RATE_LIMITED and in digest still described one marker shape and a service name taken from the marker. The rate_limited_by docstring gave the plain-comment observation as the reason for reading both connections, and a test docstring said the reply names no service. Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 11 ++++++----- tests/test_pr_review.py | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index fb403ce9f..abea20fbb 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -479,7 +479,8 @@ # A structural marker rather than prose, so reading it needs no per-bot wording model the way `QUOTA` above needs one for Copilot's free-text refusal. # Observed on CodeRabbit, a plain PR comment rather than a formal review: "". # The service name is captured rather than assumed. -# A second bot using the same auto-generated-comment convention is read without a new pattern, and one that does not use it stays unread rather than guessed at. +# A second bot using the same auto-generated-comment convention is read without a new pattern. +# Any other shape stays unread unless a pattern of its own, like the reply one below, reads it. RATE_LIMITED = re.compile(r"auto-generated comment:\s*rate limited by\s*(\S+?)\s*-->") RATE_LIMITED_REPLY = re.compile( r"\s*(?:\s*)*" @@ -1321,8 +1322,8 @@ def rate_limited_by(pr: dict, login: str) -> str | None: gives, so `RATE_LIMIT_SERVICE` names the service by login and both shapes read alike. A login missing there stays unread. - Reads both connections, since both observed shapes ride a plain PR comment rather than a - formal review. A future bot using the same convention might use either. A plain comment + Reads both connections. Both observed shapes ride a plain PR comment, and a future bot using + the same convention might ride a formal review instead. A plain comment carries no commit, so the reading is not scoped to the head. Current pull request only, unlike Copilot's `quota_signal`. That one generalizes from a pattern of silence across several pull requests, while this reads one notice rather than a history. @@ -4743,7 +4744,7 @@ def digest( (n.get("commit") or {}).get("oid") == head for n in reviewer_nodes(pr, "reviews", login) ) ] - # The service name each tracked other-reviewer's own rate-limit marker gives, where its newest comment or review on this pull request carries one. + # The service each tracked other-reviewer's rate-limit notice names, or its login supplies, where its newest comment or review here is one. other_limited = { login: name for login in OTHER_REVIEWERS if (name := rate_limited_by(pr, login)) } @@ -4768,7 +4769,7 @@ def digest( # Present only where at least one other tracked reviewer has posted on this exact head. # No verdict rides on it, unlike `review_on_head`, since nothing here reads what a CodeRabbit or qodo round said, only that one landed. + (f"other_reviewed={','.join(other_on_head)} " if other_on_head else "") - # Present only where a tracked other-reviewer's newest word is that marker. + # Present only where a tracked other-reviewer's newest word is one of those rate-limit notices. # The common case, neither is rate-limited right now, stays silent rather than printing `none`. + ( f"other_rate_limited={','.join(f'{login}:{name}' for login, name in other_limited.items())} " diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 9653123db..ed1061fd3 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -965,7 +965,7 @@ def test_rate_limited_by_reads_either_connection_and_names_no_untracked_login(se self.assertIsNone(pr_review.rate_limited_by(pr, pr_review.QODO_LOGIN)) def test_other_rate_limited_reads_the_command_reply_shape(self) -> None: - """The reply to an explicit review command names no service, so the login supplies it.""" + """The command reply's marker says CodeRabbit rather than the service, so the login names it.""" self.answer( payload( [review()], From e6f9ba9f439e9b92828759d13c65cf99f931d08f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 00:46:48 -0700 Subject: [PATCH 5/6] Say Above RATE_LIMITED Where the Reply Shape Gets Its Service Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index abea20fbb..d3e4a4db8 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -481,6 +481,7 @@ # The service name is captured rather than assumed. # A second bot using the same auto-generated-comment convention is read without a new pattern. # Any other shape stays unread unless a pattern of its own, like the reply one below, reads it. +# That reply's marker carries no service name, so `RATE_LIMIT_SERVICE` supplies one by login. RATE_LIMITED = re.compile(r"auto-generated comment:\s*rate limited by\s*(\S+?)\s*-->") RATE_LIMITED_REPLY = re.compile( r"\s*(?:\s*)*" From c2ac919936b107f65fd92787fcb3f43d8aad5e0a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 00:48:26 -0700 Subject: [PATCH 6/6] Say the Reply Marker Names the Product, Not the Service Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index d3e4a4db8..a2d5ae1a6 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -481,7 +481,7 @@ # The service name is captured rather than assumed. # A second bot using the same auto-generated-comment convention is read without a new pattern. # Any other shape stays unread unless a pattern of its own, like the reply one below, reads it. -# That reply's marker carries no service name, so `RATE_LIMIT_SERVICE` supplies one by login. +# That reply's marker names the product rather than the `coderabbit.ai` service, so `RATE_LIMIT_SERVICE` supplies the service by login. RATE_LIMITED = re.compile(r"auto-generated comment:\s*rate limited by\s*(\S+?)\s*-->") RATE_LIMITED_REPLY = re.compile( r"\s*(?:\s*)*"