Repository navigation
Conversation
## Summary Implements #3457 (the closing keyword goes on the promotion pull request). - `scripts/pr_review_digest.py` is a new pure reader module. Its `render` takes the payload, the normalized checks, the run log's rate-limit sentence, and the seen set as arguments, and makes no call. It imports only the other readers and the reviewer adapter, never the gate. - `digest` stays in `pr_review.py` as the shell that does the reads (`full_read`, `check_nodes`, and `confirmed_quota` when a refusal is present), so the tests that patch it there keep working. - The `status` header is an ordered list of `(key, value)` pairs joined by one space, a `None` value leaving the field out, so a new field is one entry. - The window, threads-truncated, and reviews-truncated advisory lines print from an ordered `(value, template)` table, in the order they printed before. - `confirmed_quota` is now read once per digest rather than up to twice, so `refusal=QUOTA` and the `RATE LIMIT` line can no longer disagree across two run-log reads. - The new module joins the readers' import guard, the whole-source guards, and the size ceilings, and `scripts/README.md` names it. ## Verification - Every `digest` output the existing suite produces, 256 renderings across `tests/test_pr_review.py`, was recorded before and after the move and compared byte for byte, identical. - `TestDigestRenderIsPinned` pins a crowded and a quiet digest whole, with text captured from the pre-move code. - The full suite passes, `python -m unittest discover -s tests`, 2963 tests, 7 skipped. Ruff, mypy, and the pre-commit hook are clean. - A local strict review ran two rounds. Round 1 raised two `introduced` findings, the new helper's name colliding with `pr_review_rounds.refusal_of` and the pin test's docstring claiming more than it pins, both fixed in 023589f. Round 2 raised none. ## Pre-existing findings, not filed - `scripts/README.md:9`, `scripts/README.md:190`, `scripts/pr_review.py:16`: `pre-existing`, each an older sentence over the opt-in sentence-length check's 25 words, flagged only because its line is in this diff. The `comments` label is applied because four comment lines moved verbatim from `pr_review.py` into the new module. 🤖 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
The final comments are minor documentation nits and no approval-blocking issues were identified.
1 open issue
What changed in this PR
This pull request promotes the digest-rendering refactor to main, extracting pure rendering into a new leaf module while keeping pr_review.py responsible for reading data and orchestration.
Changes:
- Adds
scripts/pr_review_digest.pyfor ordered digest rendering and refusal classification. - Updates
scripts/pr_review.pyto delegate rendering while retaining payload and quota reads. - Adds whole-output digest regression cases and a size ceiling for the new module.
- Updates documentation and reader guard coverage.
| File | Summary | Review notes |
|---|---|---|
tests/test_pr_review.py |
Adds pinned crowded and quiet digest cases and updates reader guard coverage. | Nit: avoid hard-coding “these two cases” in the docstring. |
tests/test_pr_review_size.py |
Adds the new digest module’s size ceiling. | No final comments. |
scripts/README.md |
Documents the expanded reader structure and ordered digest layout. | Nit: remove the incidental “three reader modules” count. |
scripts/pr_review.py |
Retains data reads and delegates digest rendering to the new module. | No final comments. |
scripts/pr_review_digest.py |
Provides pure digest rendering with ordered output structures. | No final comments. |
Review effort: Lite
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3494 +/- ##
==========================================
+ Coverage 81.04% 81.07% +0.02%
==========================================
Files 25 26 +1
Lines 8922 8928 +6
==========================================
+ Hits 7231 7238 +7
+ Misses 1691 1690 -1
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:
|
## Summary Answers the finding on promotion [#3494](#3494) that `scripts/README.md:9` counted the reader modules ("three"), a second copy of the list just before it that goes stale when a reader is added. The sentence now reads "The reader modules `pr_review.py` imports are tested through `test_pr_review.py`.", which also stops it leaning on "the gate", since the same paragraph defines a gate as a `_lint` or `_gate` script and lists `pr_review.py` as a utility. ## Verification A local strict review ran two rounds. Round 1 raised the "the gate's" antecedent, fixed in `aab59769`, and round 2 raised none. The prose gate is clean. ## Pre-existing findings, not filed - `scripts/README.md:9`: `pre-existing`, the older "Where a case covers a table" sentence runs 43 words against the opt-in sentence-length check's 25. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…a Table (#3497) ## Summary Implements #3458 (the closing keyword goes on the promotion pull request). - The whole `wait` body moves out of `main` into `run_wait(owner, repo, a)`, unchanged, so `main` is parsing and dispatch. - Flag ownership is read from `FLAG_OWNERS`, one row of `(flag, owners, late)` per flag, checked in two passes around the `cleanup` and `number` checks so a command line with several bad flags names the same one first. Listing `cleanup` among `--checkout`'s owners lets the error text be the owners joined, and the `cleanup` check right after it still refuses that command. - The four negative-value checks are one loop. - `COMMANDS` is the parser's choices, and the owner test reads it. ## Verification - The first error line and the exit of 1904 bad command lines, every subcommand crossed with pairs of misplaced, negative, and malformed flags, were recorded before and after with every subprocess stubbed out, and are identical. - `test_a_command_line_with_two_bad_flags_names_the_same_one_first` pins six multi-flag orderings, and passes against the pre-change code as well. - The full suite passes, `python -m unittest discover -s tests`, 2965 tests, 7 skipped. Ruff, mypy, and the pre-commit hook are clean. - A local strict review ran two rounds on the first push. Round 1 raised three `introduced` findings, each fixed in `d145242d`. Round 2 raised one, `COMMANDS` landing between the `FLAG_OWNERS` comment and the table, which the second push fixed in `2ca2e75d`. Its own pass raised none. ## Pre-existing findings, filed - `--timeout`, `--min-rounds`, and `--ignore-quota-signal` are still accepted silently on subcommands other than `wait`: #3496. The `comments` label is applied for the new table and loop comments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
ptr727
changed the title
Owner
Author
|
Answering the headline of the round on 162d1aa ("Two nit findings remain"): the round names no finding and its body lists no open issue. The one finding raised on this promotion, the reader-module count at |
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.1189.
scripts/pr_review.pyinto a new pure reader module,scripts/pr_review_digest.py.digeststays as the shell that does the reads. Thestatusheader is an ordered field list and the window and truncation advisories an ordered table. Output is byte-identical over every existing case, and two new cases pin a crowded and a quiet digest whole.scripts/README.mddrops its incidental count of the reader modules and namespr_review.pyas what imports them, which this promotion's first review found.wait's body moves out ofmainintorun_wait, and the per-subcommand flag checks are read from aFLAG_OWNERStable in two passes, so a new owned flag is one row. The first error printed over 1904 bad command lines is unchanged, and a case pins the multi-flag order.Closes #3457
Closes #3458
Constituent coverage
Each change this promotion carries had coverage on its own pull request into develop:
3eb58d60develop. A Copilot round on the earlier head023589f8opened one finding, fixed in3eb58d60and resolved, so no Copilot round covers the merged headaab59769develop, and a Copilot table on the merged head naming exactly its 1 changed file, no open findings2ca2e75ddevelop. A Copilot round on the earlier headd145242dopened one finding, already fixed in2ca2e75dand resolved, so no Copilot round covers the merged head🤖 Generated with Claude Code