Repository navigation
Read Every Section of a Kind When Sections Alternate in One Wrapper in pr_review.py - #3194
Conversation
…n pr_review.py marker_blocks followed its chain of bounds only while each bound matched its own marker, so a suppressed section, then a previously-missed one, then a second suppressed one in the same wrapper left the third section read by nobody. The walk now visits every bound, opening a block only where the marker matches and skipping a sibling of the other kind to its own bound. Prose past a bound is still never scanned. Fixes #3180 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Markdown previously-missed heading the test used is no vetted spelling, so the old digest reported that body as unrecognized rather than silently undercounting it. Spelled as vetted summaries, the body is the silent shape the test describes, and it now asserts shapes=ok. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
|
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 |
There was a problem hiding this comment.
🟢 Approval recommended
The remaining test assertion nit is minor and does not block approval.
1 open issue
What changed in this PR
This pull request fixes alternating section traversal in pr_review.py and adds regression coverage.
Changes:
- Traverses all sibling section bounds while matching markers.
- Tests alternating suppressed and previously missed sections.
| File | Summary |
|---|---|
tests/test_pr_review.py |
Adds alternating-section regression tests. |
scripts/pr_review.py |
Fixes section-bound traversal. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3194 +/- ##
===========================================
+ Coverage 65.44% 65.47% +0.02%
===========================================
Files 17 17
Lines 9750 9751 +1
===========================================
+ Hits 6381 6384 +3
+ Misses 3369 3367 -2
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:
|
… in pr_review.py The test checked the first and third suppressed findings but never the second, which shares the third's block. It now asserts the second sits in that block and prints once in the digest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
…o Main (#3195) - #3194 makes `marker_blocks` in `pr_review.py` visit every section bound in a wrapper, opening a block only where its own marker matches. A suppressed section after a previously-missed one in the same wrapper used to go unread under `shapes=ok`. The suppressed reader's missing vetted-but-unread check, which kept that silent, is #3193. - #3192 makes `test_spec_validate`'s corpus test skip when the `node` probe times out, the same way it skips when `node` is missing, so a slow hosted runner no longer reads as an engine disagreement. A new test pins the skip and proves that `node` refusing a pattern still errors. The change is to scripts and tests only, so no installed Skill changes at release. Closes #3180 Closes #3182 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
For #3180.
marker_blocksfollowed its chain of section bounds only while each bound matched its own marker. So one wrapper holding a suppressed section, then a previously-missed one, then a second suppressed one left the third section unread. The digest counted one suppressed finding of three undershapes=ok.sibling_headingreturns. It opens a block only where the marker matches and skips a sibling of the other kind to that sibling's own bound. Prose past a bound is still never scanned, since only bound lines are visited.outside_diff_blocksis unchanged in behavior, because CodeRabbit's marker matches no section bound.Verification
test_sections_alternating_in_one_wrapper_are_each_readbuilds the issue's body from vetted summary spellings. It asserts two suppressed blocks,suppressed=3,shapes=ok, and each finding printed once. It also covers the swapped kinds, where it asserts no unrecognized shape.develop'spr_review.py, the test fails at2 != 1. With the fix it passes.tests.test_pr_reviewsuite passes, and ruff, mypy, the prose gate, and the eol gate are clean.shapes=okfor a body whose Markdown heading is unvetted, and fcbc3de fixes that. It also found a pre-existing gap, filed as Check Suppressed Summaries for a Vetted-but-Unread Section in pr_review.py #3193: the vetted-but-unread check covers previously-missed summaries only. Round 2 found nothing.🤖 Generated with Claude Code