Repository navigation
Conversation
## Summary Answers #3424. `status=COVERAGE_IS_PARTIAL` said "the review covering the head states it read fewer files than the pull request changed" on the two carry paths in `coverage()` too. On those paths no round covering the head states partial coverage, and the bracketed reason said so. - `pr_review_rounds.py` now holds the carry reason's prefix as `CARRIED_PARTIAL`. `report_verdict` keys on it, so the line follows the reason `coverage()` gave rather than re-deriving which path it took. - On a carry path the line now leads with that reason and names the remedy that clears it: a round on the head stating full coverage, unless a round on it already states a reading this cannot believe. The direct path keeps its text and its remedy. - Tests: two carry-path cases (the head states nothing, and the head states `reviewed=4 changed=3`), each asserting the carry lead and remedy. The direct-path case now asserts its own lead and the absence of the carry wording. A mutation back to the old source fails both carry cases. ## Verification - Full suite: `uv run --no-project --with-requirements tests/requirements.txt python -m unittest discover -s tests`, 2948 tests run, OK (7 skipped). - Local strict review: two rounds. Round 1 raised two `introduced` findings, both fixed in 81ca8d8: the branch condition re-derived `coverage()`'s path, and the remedy tail contradicted the carry path. Round 2 raised none. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Summary Answers #3436. `cleanup_remote_delete` read the ref back once, right after `deleteRef`. GitHub's ref read can lag a delete that landed, so a landed delete could print `status=DELETE_NOT_CONFIRMED` and exit 2 before the local half ran. - A ref still at the deleted tip is now read again on `DELETE_SETTLE_DELAYS = (1, 2, 4)` through the existing `core.backoff` helper, so about seven seconds at most, before it is reported. Exit 2 still follows once that schedule is spent. - A read back at a tip other than the deleted one stops the loop and gets its own message ("a tip other than the one deleted, so something pushed to the branch"), since re-reading it settles nothing. - `scripts/README.md`'s `cleanup` paragraph says so. - Tests: a read lagging then gone settles to `DELETED` and exit 0. A ref present on every read exits 2 after exactly the scheduled reads. A settle read that does not answer exits 2. A new tip stops the loop and names that tip. The first two fail against the unfixed source. The existing suite patches the schedule to `()`, which reproduces the old single read. ## Known limits, not defects - A branch pushed back at the deleted tip inside the settle window cannot be told from lag. It ends `still reads at <tip> after the delete`, exit 2, which is the conservative outcome. - `scripts/handoff.py` reads state back once after a write in `create_label`, `close`, and `edit_issue`. No lag has been seen on label or issue reads, so these are the sites to name in one issue only if it is. ## Verification - Full suite: `uv run --no-project --with-requirements tests/requirements.txt python -m unittest discover -s tests`, 2952 tests run, OK (7 skipped), at 278e49a. The commit after it changes message wording and a test name, and `test_pr_*.py` (628 tests) passes on it. - Local strict review: three passes. Round 1 raised three `introduced` findings (a README sentence over the length limit, no test for a settle read that does not answer, and the loop re-reading a ref at a new tip), all fixed in 278e49a. Round 2 raised three (an overclaiming message, a stale docstring, a long line), fixed in 2c71411. The third pass raised none. ## Pre-existing findings, not filed - `scripts/README.md:264`, `pre-existing`: five sentences this change did not edit run over the 25-word sentence-length check. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Summary Answers #3461. Run as a script, the gate is `__main__`, so `pr_review.py cleanup` imported `pr_cleanup`, whose `import pr_review as core` loaded a second copy of the gate with its own memoized readers and constants. - The `cleanup` dispatch in `main` now runs `sys.modules.setdefault("pr_review", sys.modules[__name__])` before `import pr_cleanup`, so that import resolves to the running module. When the gate is imported as `pr_review` (the tests), `setdefault` changes nothing. - This takes neither route the issue suggested. A separate entry point would break the `pr_review.py cleanup` command that the host-safety allow rule and the `drive-pr` and `merge-and-release` skills name. Moving the six helpers `pr_cleanup` takes into a new module would also move every test patch point, while adding no safety today: `pr_cleanup` is the only module importing `pr_review`. - Test: `TestCleanupLoadsOneGate` runs the real script as `python -X importtime scripts/pr_review.py cleanup --help`. It asserts `pr_cleanup` is imported and `pr_review` is not. It fails against the unfixed source. In script mode a second copy can only come from a fresh import under that name, so the import log answers the issue's `sys.modules` condition. ## Verification - Full suite: `uv run --no-project --with-requirements tests/requirements.txt python -m unittest discover -s tests`, 2953 tests run, OK (7 skipped). - Local strict review: one pass, no findings. It checked every launch mode (`python3`, `uv run --script`, `python -m`, import, `runpy`), every sibling's local imports for the same cycle, and the test's parsing. 🤖 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
All reviewed files have no unresolved findings, and the approval-readiness assessments are unanimous.
No open issues 🎉
What changed in this PR
This pull request promotes three fixes from develop to main: clearer partial-coverage status messages, resilient remote-branch deletion confirmation, and single-module cleanup dispatch.
Changes:
- Clarifies carried partial-coverage messages and remedies.
- Adds bounded retries and diagnostics for remote branch deletion confirmation.
- Reuses the running gate module during cleanup.
- Adds regression tests and updates cleanup documentation.
| File | Description |
|---|---|
tests/test_pr_review.py |
Tests direct and carried partial-coverage status messages. |
tests/test_pr_cleanup.py |
Tests deletion retries, unreadable responses, moved tips, and single-module loading. |
scripts/README.md |
Documents delayed confirmation of remote branch deletion. |
scripts/pr_review.py |
Updates coverage messaging and aliases the running module for cleanup dispatch. |
scripts/pr_review_rounds.py |
Defines and propagates the carried partial-coverage reason. |
scripts/pr_cleanup.py |
Adds bounded retries and distinct handling for deleted, unread, and moved refs. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3482 +/- ##
==========================================
+ Coverage 80.98% 81.00% +0.01%
==========================================
Files 25 25
Lines 8895 8903 +8
==========================================
+ Hits 7204 7212 +8
Misses 1691 1691
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.
Summary
Promotes the pull requests merged into develop since release 2.0.1181.
status=COVERAGE_IS_PARTIALno longer says the head's own review read fewer files on the carry paths. It leads with the carried reason there and names the remedy that clears it.cleanupre-reads a ref still at the deleted tip for a few seconds before it reportsDELETE_NOT_CONFIRMED, since the ref read can lag a delete that landed.pr_review.py cleanuploads one copy of the gate rather than a second one under the namepr_review.Closes #3424
Closes #3436
Closes #3461
Constituent coverage
Each change this promotion carries had coverage on its own pull request into develop, at its merged head:
81ca8d88develop, and a Copilot table on the merged head naming exactly its 3 changed files, no open findings2c71411cdevelop, and a Copilot table on the merged head naming exactly its 3 changed files, no open findings1747bd78develop, and a Copilot table on the merged head naming exactly its 2 changed files, no open findings🤖 Generated with Claude Code