Repository navigation
Conversation
## Summary `check_nodes` in `scripts/pr_review_checks.py` reads a rollup node of an unrecognized GraphQL union member as a node with empty values and an `unreadable` key. `held_checks_open` and `checks_never_ran` tested for an empty rollup with `not nodes`. So a rollup holding only such nodes was not read as empty. It released a `BLOCKED`, `BEHIND`, or `DRAFT` merge that an empty rollup holds, and under `DIRTY` it let `wait` exit `0` rather than `45`. Both readers now count readable checks through the existing `checks_tally`, so a rollup of only unreadable types is judged as one with no check. The `DIRTY` release names the unread types. `checks_unread` now returns each unread typename once, sorted, so that message and the digest's `CHECKS PARTIALLY UNREAD` line share one form. The `wait` help, the `CHECKS_PENDING` status line, and `scripts/README.md` say the same. `scripts/pr_review_checks.py` is at 447 lines against its 450 ceiling. Refs #3488 ## Verification - A new end-to-end test holds a rollup of only an unreadable node to `30` under `BLOCKED`, `BEHIND`, and `DRAFT`, and exits it `45` under `DIRTY` with the unread type named. It fails on the unfixed code for all four merges. - The `checks_never_ran` unit test adds a mixed rollup, which still releases, and an unread-only one of two nodes, which names the type once. - The full suite ran 3091 tests, OK (skipped=7). - The local strict review ran three rounds at the `frontier` tier. Round one found a stale status line, two stale help paragraphs, a repeated type name, and two sentences over the bound. Round two found the fix's wording problems and a duplicated dedupe. All were fixed. Round three found one, which spent the edit budget and was listed here as open. Splitting the exit-45 help sentence had dropped "Either", so the payload case read as true of every exit `45`. It is fixed in 1b86688, which names each case as one of three, and its own recorded pass found nothing. - A Copilot round on 3885af8 raised the same help finding, answered by 1b86688. It also found that no test pinned the sorted order. 31a86e7 adds two distinct types in reverse order, and that test fails against an input-order dedupe. Its recorded pass found nothing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the unreadable-only rollup handling change.
No open issues 🎉
What changed in this PR
Promotes unreadable-only rollup handling from develop to main, treating rollups without readable checks as empty during merge-wait decisions.
Changes:
- Holds
BLOCKED,BEHIND, andDRAFTmerges with unreadable-only rollups. - Returns exit code
45forDIRTYmerges and reports sorted, deduplicated unreadable types. - Updates wait messaging and documentation.
- Adds end-to-end and unit test coverage.
| File | Summary |
|---|---|
tests/test_pr_review.py |
Adds coverage for unreadable-only rollups and diagnostic ordering. |
scripts/README.md |
Documents updated exit-code and wait behavior. |
scripts/pr_review.py |
Updates wait help and pending status messaging. |
scripts/pr_review_checks.py |
Counts readable checks and normalizes unreadable-type reporting. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3633 +/- ##
=======================================
Coverage 81.62% 81.63%
=======================================
Files 26 26
Lines 9078 9081 +3
=======================================
+ Hits 7410 7413 +3
Misses 1668 1668
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes the #3488 unread-only rollup hold from
developtomain.waitholds aBLOCKED,BEHIND, orDRAFTmerge and exits45underDIRTYrather than releasing on checks it never read.Closes #3488
🤖 Generated with Claude Code