Repository navigation
Conversation
## Summary Resolves #3174 and #3173. Both decide what counts as a section heading in `pr_review.py`, so they land together. - **One reduction, one leading-count rule (#3174).** `normal`, `unwrap`, and `heading_count` now share `reduced`, which turns a badge into its word and drops emphasis. `normal` and `heading_count` also share `spaced` and one `LEADING_COUNT`. As a result, the role a summary is vetted with and the count read from it cannot disagree. Before, `<strong>2\fopen findings</strong>` was vetted as the open-findings section while its count read as none. - **Which summaries, not how many (#3174).** `unrecognized_in` now checks which vetted previously-missed summaries appear inside the blocks it read. A bare `(N)` line read in one region can no longer stand in for a summary that no reader found in another. - **One block per section.** A section ends at the next counted heading of either kind, its own kind included. So two previously-missed sections in one wrapper are two blocks, each counted by its own heading. Past a region's first section, only the sibling heading that section ends at can open another one, so prose after a section boundary never opens a section. - **A finding entry is never a section (#3173).** An entry whose summary opens on a severity badge is never read as a section heading and never ends a section, whatever its title says. ## Verification - `python3 -m unittest test_pr_review`: 855 tests pass. All 850 existing tests pass unchanged. - Each of the 5 new tests fails against the code it fixes. The four first-commit tests fail on `develop`, and `test_prose_past_a_bound_opens_no_second_section` fails on the previous branch head. - `ruff format --check`, `ruff check`, `mypy`, `spec/validate.py`, and the commit hooks' prose and EOL gates are clean. - One report string changes, on a shape that was already unrecognized: a count-first Markdown heading such as `### 2 foo` now reports as `### (N) foo`. ## Local review findings still open The edit budget for the local strict review was spent over three recorded passes. These remain for this PR's review loop: - **introduced:** the `sibling_heading` docstring says "A second section of the same kind bounds too, so each is its own block with its own count". That holds only when the two sections are adjacent. With sections interleaved in one region (suppressed, then previously missed, then suppressed again), the third section is read by nobody. The gap itself is **pre-existing**, since `develop` read only the first section per region, and no observed body interleaves this way. - **style:** the docstring of `test_prose_past_a_bound_opens_no_second_section` holds one 26-word sentence that only the opt-in `--check sentence-length` flags. The default prose gate passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…pr_review.py (#3181) ## Summary Resolves #3176. `entry_paths` now drops a trailing `:LINE` or `:START-END` from a path a list entry names in a code span. So `` `src/a.py:340` `` pairs with a thread on `src/a.py`. Before, a `Review findings:` entry in that shape never paired, and the digest reported every such entry under `FINDINGS WITH NO THREAD`, including the ones that had opened a thread. The narrative `Open findings remain:` reader shares `entry_paths`, so it gets the same pairing. A test pins it there too. ## Verification - `python3 -m unittest test_pr_review`: all pass. - Both new cases fail on `develop` at f4eb405: the prose subtest naming `lib/reader.py:340` and `writer.py:12-18`, and the narrative test naming `pkg/guard.py:12`. - `ruff format --check`, `ruff check`, `mypy`, and the commit hooks' prose and EOL gates are clean. - Two recorded local strict-review passes. The first raised 3 findings, all fixed. The second raised none. 🤖 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. ⚙️ 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 |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Minor test nits remain, and automated approval confidence is insufficient.
No open issues 🎉
What changed in this PR
Promotes heading-parsing and line-suffix pairing fixes, with regression coverage.
Changes:
- Unifies heading reduction and count parsing.
- Improves section and finding detection.
- Pairs line-suffixed findings with threads.
- Adds parser regression tests.
| File | Summary |
|---|---|
tests/test_pr_review.py |
Adds regression coverage for parser fixes. |
scripts/pr_review.py |
Updates parsing, section detection, and path pairing. |
Review effort: Lite
Owner
Author
|
Answering this round's headline, "Minor test nits remain, and automated approval confidence is insufficient". The round names no specific nit, opens no thread, and states no open finding ("No open issues"), so there is nothing concrete to fix. It is a caution rather than a finding. The tests it covers were each checked against the code they fix: every new test in #3179 and #3181 fails on the code before its fix and passes after, except one guard assertion that holds on both. Each feature PR also carried recorded local strict-review passes, all findings disposed of there. Any nit raised later as a thread or a named finding gets its own outcome. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3183 +/- ##
==========================================
+ Coverage 64.08% 64.14% +0.05%
==========================================
Files 17 17
Lines 9183 9198 +15
==========================================
+ Hits 5885 5900 +15
Misses 3298 3298
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.
pr_review.pythrough one reduction and one leading-count rule, so the role a summary is vetted with and its count cannot disagree. Its vetted-but-unread check compares which previously-missed summaries were read rather than how many blocks there are. Two sections of the same kind in one wrapper read as two blocks, each counted by its own heading. A finding entry opening on a severity badge is never read as a section heading, whatever its title says. The pre-existing gap with sections alternating in one wrapper is Read Every Section of a Kind When Sections Alternate in One Wrapper in pr_review.py #3180.Review findings:orOpen findings remain:entry with its thread past a:LINEor:START-ENDsuffix. Every entry in that shape used to read as unthreaded.The change is to scripts and tests only, so no installed Skill changes at release.
Closes #3173
Closes #3174
Closes #3176
🤖 Generated with Claude Code