Repository navigation
Conversation
…validate (#3192) ## Summary For #3182. The corpus test in `test_spec_validate` runs `node` under a 60 second timeout. A cold start on a slow hosted Windows runner raised `TimeoutExpired` and failed the suite as though the schema patterns disagreed. - A `TimeoutExpired` from the probe now skips the test with that reason, the same way a host without `node` already skips. - Every other failure is unchanged: a disagreement still fails `assertEqual`, and a pattern `new RegExp` rejects still fails through `check=True`. - Runaway backtracking can't hide behind the skip. Python's `re.search` evaluates every pattern over the same corpus before `node` starts, so the run hangs there instead. ## Verification - A forced `TimeoutExpired` from the `node` call: the test reports `skipped=1`, with no failures or errors. - `python3 -m unittest tests.test_spec_validate` passes with `node` present. - ruff, mypy, the prose gate, and the eol gate are clean. - A local strict review found nothing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…n pr_review.py (#3194) ## Summary For #3180. `marker_blocks` followed 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 under `shapes=ok`. - The walk now visits every bound `sibling_heading` returns. 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_blocks` is unchanged in behavior, because CodeRabbit's marker matches no section bound. ## Verification - The new test `test_sections_alternating_in_one_wrapper_are_each_read` builds 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. - With `develop`'s `pr_review.py`, the test fails at `2 != 1`. With the fix it passes. - The full `tests.test_pr_review` suite passes, and ruff, mypy, the prose gate, and the eol gate are clean. - The local strict review took two rounds. Round 1 found that the test's docstring claimed `shapes=ok` for a body whose Markdown heading is unvetted, and fcbc3de fixes that. It also found a pre-existing gap, filed as #3193: the vetted-but-unread check covers previously-missed summaries only. Round 2 found nothing. 🤖 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.
🟢 Approval recommended
No blocking issues were identified; only a minor documentation wording nit remains.
No open issues 🎉
What changed in this PR
Promotes fixes for alternating-section parsing and Node probe timeout handling.
Changes:
- Traverses all bounded review sections.
- Skips corpus checks when Node times out.
- Adds regression tests for both behaviors.
| File | Summary |
|---|---|
tests/test_spec_validate.py |
Tests timeout skipping and probe failures. |
tests/test_pr_review.py |
Tests alternating section parsing. |
scripts/pr_review.py |
Updates section traversal logic. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3195 +/- ##
=======================================
Coverage 65.44% 65.44%
=======================================
Files 17 17
Lines 9750 9751 +1
=======================================
+ Hits 6381 6382 +1
Misses 3369 3369
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.
marker_blocksinpr_review.pyvisit 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 undershapes=ok. The suppressed reader's missing vetted-but-unread check, which kept that silent, is Check Suppressed Summaries for a Vetted-but-Unread Section in pr_review.py #3193.test_spec_validate's corpus test skip when thenodeprobe times out, the same way it skips whennodeis missing, so a slow hosted runner no longer reads as an engine disagreement. A new test pins the skip and proves thatnoderefusing 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