Repository navigation
Read Copilot's Count-First Overview Sections in pr_review.py - #3172
Merged
Merged
Conversation
A later revision of Copilot's second overview format opens each section summary on its count and renames findings to issues. pr_review.py stopped on each of these shapes with exit 43: - `2 previously missed issues` is the previously-missed section, whose findings open no thread. It is now vetted, matched by PREVIOUSLY_MISSED, and counted by finding_count from its leading count rather than falling to the floor of one. - `1 open issue` and `N open issues` are the open-findings section under a new noun. They map to the same role, so the section states the round's total and each entry is checked against the threads. - `**No open issues**` followed by an emoji is the bare stated zero, which printed `?` before. The vetted-list size comment is removed with the test that kept it in sync, since its count went stale on every new spelling. Refs #3167 Refs #3170
finding_count, section_entries and sibling_heading each read a section heading's count their own way, and only finding_count learned the count-first spelling. sibling_heading still bounded a section on `(N)` alone, so a suppressed block sharing a wrapper with a count-first previously-missed sibling ran into it, and the digest printed that finding twice. heading_count now serves all three, reading `(N)` or a count opening the heading. marker_blocks keeps requiring `(N)` for a line that is not a heading, since a count opening a line cannot tell a heading from prose. The count patterns take ASCII digits only, so a run of non-ASCII digits no longer overlaps the non-ASCII opener alternative. Refs #3167 Refs #3170
A line that is not a heading qualified as a section on any `(N)` it carried. With the count-first spelling admitted by PREVIOUSLY_MISSED, a sentence such as `2 previously missed (3) rounds ago` read as a section. marker_blocks now accepts such a line only where it does not open on a count, since a leading count is how a heading states one and also how a sentence opens. PREVIOUSLY_MISSED takes a count of any length, so counted() stays the one place the digit limit applies, and a run longer than a count reads as the section at the floor of one rather than as no section at all. Refs #3167
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Owner
Author
|
A recorded local strict-review pass covers head |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved parsing defects affect section detection and multiline count handling.
3 open issues
What changed in this PR
This PR updates pr_review.py to parse Copilot’s newer count-first overview sections.
Changes:
- Supports previously missed and open issue count formats.
- Recognizes stated zero open issues.
- Adds regression coverage for the new formats.
| File | Description |
|---|---|
scripts/pr_review.py |
Adds parsing and counting support for new overview formats. |
tests/test_pr_review.py |
Adds regression tests for count-first sections and issue wording. |
Review effort: Lite
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3172 +/- ##
==========================================
Coverage ? 64.08%
==========================================
Files ? 17
Lines ? 9183
Branches ? 0
==========================================
Hits ? 5885
Misses ? 3298
Partials ? 0
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:
|
ptr727
added
the
comments
PREVIOUSLY_MISSED admitted a leading count after a tag or a `>`, which LEADING_COUNT did not strip, so `> 2 previously missed (3) rounds ago` escaped the prose check in marker_blocks and read as a section. Both patterns now share HEADING_OPENER, so what the anchor admits the prose check strips. heading_count takes the heading itself and reads all of it. Cut to its first line, a summary split as `Open` then `(2)` lost the count its role was vetted with. marker_blocks folds a `<summary>` split across lines onto its opening line before scanning, so a count-first summary whose text sits on lines of its own is read as the section its vetted spelling says it is. The comments that said a count is what prose does not carry now say which count, and the prose test's docstring says the same. Refs #3167
Sharing the wide opener with the count reader made a finding entry's severity badge read as heading markup, so an entry titled with a number stated a count and bounded its section there, dropping the entries after it. LEADING_COUNT reads past emphasis tags only again, and the prose check in marker_blocks uses OPENING_COUNT, which keeps the wide opener the anchor admits. fold_summaries folds only a line opening on `<summary`, and stops at a blank line or a line opening another wrapper or summary, so a tag named mid-sentence no longer folds a sibling heading out of reach. A folded continuation line drops its blockquote markers. The finding-entry phantom suppressed block found along the way predates this branch and is filed as #3173. Refs #3167
heading_count now reduces the badge and the emphasis the way normal does before vetting, then reads the count. A count wrapped in its own emphasis tag, or emphasis in capitals, reads as it did on develop, and a badge reduces to its word, so an entry titled with a number still states no count. LEADING_COUNT skips no tag of its own any more. fold_summaries no longer stops at a blank line, which left a summary set off by blank lines unread. It stops at a line opening another wrapper, summary, or Markdown heading. unrecognized_in reports a previously-missed summary that no reader finds as a section, since its spelling is vetted wherever it appears while the reader keys on how its line opens. A layout the reader misses is then a reported shape rather than findings nobody reads. HEADING_OPENER drops the line breaks only the count reader needed. Refs #3167
Owner
Author
|
A recorded local strict-review pass covers head |
ptr727
added a commit
that referenced
this pull request
Oct 17, 2026
…r to Main (#3175) - #3168 adds `GOVERNANCE.md` "Documentation Style Conventions" > "Incidental Counts", carried whole into `comment-and-doc-style`, and removes the incidental counts from the carried canonicals, the skill sources, `scripts/README.md`, and the `scripts/handoff.py` docstring. The rest of the sweep is #3169. - #3172 teaches `pr_review.py` Copilot's count-first overview summaries, `N previously missed issues`, `N open issue(s)`, and the `**No open issues**` stated zero, which each stopped the loop with exit 43. It reads a section heading's count in one place and reports a vetted previously-missed summary no reader finds. Its deferred local-review findings are #3173 and #3174. - #3177 names the never-tiered-down groups in `backlog-burndown` in the plural, the noun Copilot flagged on this promotion's first round. After this merges, `comment-and-doc-style` changes, so this machine's installed Skills are refreshed at release. Closes #3160 Closes #3167 Closes #3170 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
A later revision of Copilot's second overview format opens each section summary on its count and names findings issues.
pr_review.pystopped on each of these shapes with exit 43. It now reads them:2 previously missed issuesis the previously-missed section, whose findings open no thread. It is vetted, matched byPREVIOUSLY_MISSED, and counted from its leading count, sopreviously_missed=Nnames each finding.1 open issueandN open issuesare the open-findings section under a new noun. They map to the same role, so the section states the round's total and each entry is checked against the threads.**No open issues**followed by an emoji is the bare stated zero, which printed?before.heading_countis now the one reader of a section heading's count,(N)or leading, used byfinding_count,section_entries, andsibling_heading.marker_blocksaccepts a non-heading line only on a(N)and only where the line does not open on a count.The comment stating the vetted list sizes is removed with the one test that kept its count in sync, since the count went stale on every new spelling.
Refs #3167
Refs #3170
Verification
tests/test_pr_review.py: 850 tests pass. Every new test fails against the base script, and each later fix's test fails with that fix reverted.prose_lint.py --diff(default rules and--check sentence-length), andrepo_gate.py --check eolare clean.Open Local-Review Findings
The five findings the first push listed here were fixed in 9722c34, e1d9f26, and b994110, which also answer the three Copilot threads. The fix push's own final pass raised these, after its edit budget was spent, each to get an outcome in this pull request's review loop.
introduced:unrecognized_incompares the count of vetted previously-missed summaries with the count of blocks, whilemarker_blocksreturns at most one block per region. Two previously-missed sections nested in oneReview detailswrapper then report an unread shape, exit 43, wheredevelopreads one block holding both. The reverse can mask a miss. Not an observed shape. Outcome: filed as Read a Section Heading Through One Reduction in pr_review.py #3174, by the maintainer's choice.introduced:HEADING_OPENERreads past a finding entry's severity badge and may now be followed by a count, so an entry titled2 previously missed guards are re-addedinside another section reads as a previously-missed section. An entry with no badge titled with a number now bounds its section insibling_heading. This is the class Read a Finding Entry Titled With the Suppressed Marker as No Section in pr_review.py #3173 tracks, widened. Not an observed shape. Outcome: added to Read a Finding Entry Titled With the Suppressed Marker as No Section in pr_review.py #3173, by the maintainer's choice.introduced, design:heading_countis a third composition of the badge and emphasis reductionnormalandunwrapalso perform, andLEADING_COUNTis a second definition of a leading count besidenormal's. They differ on a form feed after the count. A shared reduction would close the class. Outcome: filed as Read a Section Heading Through One Reduction in pr_review.py #3174, by the maintainer's choice.🤖 Generated with Claude Code