Repository navigation
Review gate: read CodeRabbit's icon notice and Codex's commit-less receipt - #1291
Conversation
…ceipt Two bot comments that hold no finding were counted as unanswered findings. CodeRabbit's "Review limit reached" summary notice has put an icon in front of each folded summary since 2026-10-23: a <picture> of two SVGs from storage.googleapis.com/coderabbit_public_assets/review-comment-icons-v1, then "Run configuration", "Commits", "Files selected for processing: N" or "Files ignored due to path filters: N". No such line was a listed shape. Of the 56 notices without a walkthrough on ejc3/fcvm PRs updated from 2026-10-01 to 2026-10-25, the gate on main claimed the 31 with icons; with this change it claims none. Codex's receipt can name no commit: on #1282 it was the receipt marker, the phrase and its sign-off, with no reviewed-commit line. It is a verdict that covers nothing. - scripts/check-review-threads.sh: - cr_icon_summary_re($icon; $name), and four payload shapes built from it, each pairing one icon with the name it labels. - is_verdict accepts the phrase line alone when the body's first line is "<!-- codex-review-receipt:v1 -->". verdict_sha returns "" for it, so it binds no head. Without the marker the bare phrase stays claimable (finding 21). - Comments describe both shapes. - scripts/test-check-review-threads.sh, finding 51: the #1285 notice (ids replaced) needs no disposition and covers no head; with a finding appended, a finding as an icon summary's name, or an icon URL changed, it is claimable. The #1282 receipt needs no disposition and covers no head; without its marker, with a finding appended, or from a human, it is claimable. - tests/test_log_scan.rs: the same two notice tests and two claim guards. The #1154 case from the issue already passes on main (c954079, 6ddf1ce added "Delightful!" and the 2026-10 notice layout). The sign-off list stays enumerated: an open tail accepts "Didn't find any major issues. P1: ..." as a verdict (finding 21). All 148 receipt-shaped Codex comments on PRs updated since 2026-10-01 match. Tested: - bash scripts/test-check-review-threads.sh: passed=341 failed=0. On main's gate the two notice cases fail (UNANSWERED). With the marker check removed "the #1282 receipt without its marker is claimable" fails; with the icon name loosened to .* "a finding written as an icon summary's name is claimable" fails. - make test-unit FILTER="-E 'binary(test_log_scan)'": 40 passed. On main's gate the two notice tests fail; each mutation above fails its claim guard. - make lint: passes.
|
@codex review |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Codex receipt verdict recognition scripts/check-review-threads.sh, scripts/test-check-review-threads.sh, tests/test_log_scan.rs |
is_verdict accepts a single-line Codex no-findings verdict when the body starts with the receipt marker. Tests cover comments without a reviewed-commit line and altered or human-posted receipts. |
CodeRabbit icon-summary notice recognition scripts/check-review-threads.sh, scripts/test-check-review-threads.sh, tests/test_log_scan.rs |
The notice payload validation accepts icon-wrapped summaries for Run configuration, Commits, Files selected, and Files ignored. Tests check the exact notice and modified notices for disposition, head coverage, and claimability. |
Priority:
Estimated code review effort: 2 (Simple) | ~15 minutes
Change: Bug fix · Severity of issue fixed: Medium
Merge Risk: 
6f2b0
-
The recognized bot notices retain the intended claimability boundaries. No identified issue blocks merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the two main changes: CodeRabbit icon notices and Codex receipts without a reviewed commit. Linked Issues check Directly linked issue #1155has two coding requirements. Both are met at head 6f2b069.
1. CodeRabbit review-limit notice [#1155]: scripts/check-review-threads.sh adds cr_icon_summary_re($icon; $name)…Out of Scope Changes check The whole-PR diff against the merge base changes three files: scripts/check-review-threads.sh, scripts/test-check-review-threads.sh, and tests/test_log_scan.rs. All changes serve #1155.
- scripts/che…Docstring Coverage Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. Full details: Linked Issues check
-
Explanation
Directly linked issue
#1155has two coding requirements. Both are met at head 6f2b069.-
CodeRabbit review-limit notice [
#1155]: scripts/check-review-threads.sh adds cr_icon_summary_re($icon; $name) and four entries to cr_notice_payload_res (cog/Run configuration, git-commit-horizontal/Commits, file-plus-corner/Files selected for processing: [0-9]+, file-minus-corner/Files ignored due to path filters: [0-9]+). Each entry matches one full line exactly, with ^ and $ anchors. Added text or an unlisted icon URL stays claimable. Red-first tests exist in scripts/test-check-review-threads.sh (finding 51 block) and tests/test_log_scan.rs (review_gate_treats_the_coderabbit_1285_icon_notice_as_a_notice and the _with_additions_as_a_claim test). They cover a finding appended, a finding in an icon summary name, and a non-CodeRabbit icon. -
Codex no-findings receipt [
#1155]: is_verdict adds a one-line form. It requires the phrase line from codex_line_re and the body starting with codex_receipt_re (\A<!-- codex-review-receipt:v1 -->\n). verma_sha returns "" for this form, so it covers no head. The two-line form with a Reviewed commit line is unchanged, so a receipt that names the head still covers it. Tests in both harnesses cover: no-commit receipt needs no disposition; it covers no head; the same receipt without the marker is claimable; a finding appended is claimable; a human poster is claimable.
The issue asks to keep matching line for line. The PR does this with enumerated phrases. The issue's "consider" suggestion to match fixed parts was not adopted. The PR gives a reason: an open tail would accept a finding such as "Didn't find any major issues. P1: ...". That is a reasoned choice, not an unmet requirement. The Delightful and delve-into-next wordings from
#1155are already in codex_line_re on main, as the PR states.Limit: I read the diffs and test fixtures. I did not re-run the test suites. The pass counts in the PR description are author-reported.
-
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff against the merge base changes three files: scripts/check-review-threads.sh, scripts/test-check-review-threads.sh, and tests/test_log_scan.rs. All changes serve
#1155.- scripts/check-review-threads.sh: the codex_receipt_re and is_verdict one-line form (Codex receipt without a commit). The cr_icon_summary_re definition and the four payload entries (CodeRabbit icon notice). The comment block edits that describe these two shapes. The comment on finding 21 explains why the bare phrase stays claimable.
- scripts/test-check-review-threads.sh and tests/test_log_scan.rs: regression cases for the two shapes and their claim guards. Test code supports the requirement directly.
The PR does not change the review-body path, other CodeRabbit notice shapes, or unrelated gate rules. The PR description lists those as follow-up. I found no change unconnected to
#1155.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
NOT-A-DEFECT: review-limit notices from CodeRabbit, and review bodies whose findings are answered in their threads, carry no open claim. |
Two bot comments that hold no finding were counted by
scripts/check-review-threads.shas unanswered findings. Fixes #1155.What changed
<picture>of two SVGs from CodeRabbit'sreview-comment-icons-v1set, followed by "Run configuration", "Commits", "Files selected for processing: N" or "Files ignored due to path filters: N". None of those lines was a listed shape.cr_icon_summary_re($icon; $name)builds four payload shapes, each pairing one icon with the name it labels. Of the 56 notices without a walkthrough on ejc3/fcvm PRs updated from 2026-10-01 to 2026-10-25, main's gate claimed the 31 with icons; this branch claims none.is_verdictnow accepts the phrase line alone when the body's first line is<!-- codex-review-receipt:v1 -->. It names no commit, so it covers no head. Without the marker the bare phrase stays claimable (finding 21).Matching stays line for line: the same text with anything added, or from another account, is still a claim.
The cases named in the issue
The #1154 bodies (Codex "Delightful!" receipt, CodeRabbit notice in the 2026-10
<dl>layout) already pass on main through c954079 and 6ddf1ce. Run against #1154's payload with the hand-written NOT-A-DEFECT review removed, main reports CLEAR with the head covered by two verdicts. This PR covers the wordings the bots moved to after those commits.The sign-off list stays enumerated rather than matching the receipt by marker plus phrase with any tail: an open tail accepts "Didn't find any major issues. P1: drops data" as a verdict, which finding 21 exists to reject. Every Codex no-findings comment on PRs updated since 2026-10-01 (148) is classified as a verdict on this branch.
No CodeRabbit review body (as opposed to a comment) carried a "Review limit reached" notice on PRs updated since 2026-10-01, so review bodies are unchanged.
Contract, impact, evidence
Contract: those two shapes need no disposition and cover no head; anything added to them stays a claim. Impact: the merge gate only. Evidence:
bash scripts/test-check-review-threads.sh: passed=341 failed=0. On main's gate, finding 51's two notice cases fail withUNANSWERED coderabbitai/UNANSWERED chatgpt-codex-connector. With the marker check removed, "the fleet coord: launch the GPU hosts with an instant EC2 Fleet, spot then on demand in one pass #1282 receipt without its marker is claimable" fails; with the icon name loosened to.*, "a finding written as an icon summary's name is claimable" fails.make test-unit FILTER="-E 'binary(test_log_scan)'": 40 passed. On main's gatereview_gate_treats_the_coderabbit_1285_icon_notice_as_a_noticeandreview_gate_treats_a_codex_receipt_naming_no_commit_as_a_verdict_without_coveragefail; each mutation above fails its claim guard.make lint: passes.Left as follow-up
Other CodeRabbit notice shapes are still unlisted and cost one disposition each: "Review skipped" for bot users, drafts and path filters, the auto-pause notice, and a "Too many files" skip notice with a new usage-credits line (#1172). Comments holding a notice beside a walkthrough stay claimable by design.
Summary by CodeRabbit