Skip to content

Promote One-Line Details Section Bounding in pr_review.py to Main - #3205

Merged
ptr727 merged 1 commit into
mainfrom
develop
Oct 18, 2026
Merged

ptr727 merged 1 commit into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Oct 18, 2026

Copy link
Copy Markdown
Owner
  • Bound a Section Whose Summary Shares Its Details Line in pr_review.py #3204 makes pr_review.py bound a section whose <summary> shares its line with the <details> that opens it. A suppressed block used to run on into a following previously-missed section opened that way and print its findings twice, with nothing reporting the shape. heading_of reads past the same opener, so a count opening such a heading is read. The attribute separator after a tag name in DETAILS_OPENER, SUMMARY, FOLD_OPENER, and FOLD_STOP is now HTML whitespace rather than Python's \s.

The change is to scripts and tests only, so no installed Skill changes at release.

Closes #3201

🤖 Generated with Claude Code

…#3204)

## Summary

A section whose `<summary>` shares its line with the `<details>` that
opens it was not a heading line to `sibling_heading`, so a sibling
opened that way never bounded the block before it. A suppressed block
then ran on into a following previously-missed section and printed its
findings twice, under the suppressed banner. Each kind's reader still
found its own section, so the vetted-but-unread check had nothing to
report.

- `HEADING` admits a `<details>` opener ahead of `<summary>` on the same
line, through a shared `DETAILS_OPENER` pattern.
- `heading_of` reads past that opener where a summary follows it on the
line. Without that, a count opening such a heading (`<strong>2
previously missed issues</strong>`) went unread: the section counted as
one finding and its sibling never bounded, where the base had reported
the body as unrecognized. The local strict review found this regression
in the first cut.

## Tests

- `test_a_section_opening_on_its_details_line_bounds_the_one_before`
runs a count-closing and a count-opening previously-missed heading after
a one-line suppressed section. Both subtests fail on the base. With only
the `heading_of` change reverted, the count-opening one fails.
- Full suite: 2969 tests OK, 7 skipped. Ruff, mypy, and the prose gate
in both modes are clean.

## Local review

Three recorded passes at the `frontier` tier. The first two raised
findings that were fixed in this branch, and the third raised none.

Refs #3201

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved parsing of headings inside HTML `<details>` sections,
including headings that share a line with the opening tag.
* Updated tag matching to handle standard HTML whitespace consistently.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Copilot AI lite review requested due to automatic review settings October 18, 2026 18:35
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 83e43f12-e43e-4692-a423-b30e27aa5a8e
📥 Commits

Reviewing files that changed from the base of the PR and between cd5b9f9 and 213ac08.

📒 Files selected for processing: 2
  • scripts/pr_review.py
  • tests/test_pr_review.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

A moderate parsing issue remains in the <summary> tag matching branch.

No open issues 🎉

What changed in this PR

Updates pr_review.py to correctly parse one-line <details><summary> sections and HTML whitespace, with regression coverage.

Changes:

  • Fixes section boundaries and heading extraction.
  • Adds tests for inline sections and whitespace handling.
  • Requires a tag-name boundary after <summary> in one parsing branch.
File Description
tests/​test_pr_review.py Adds regression coverage for section boundaries and whitespace.
scripts/​pr_review.py Updates section parsing, heading detection, and folding patterns.

Review effort: Lite

@ptr727

ptr727 commented Oct 18, 2026

Copy link
Copy Markdown
Owner Author

Answering the headline of Copilot's round on 213ac082, which opened no thread and counted 0 findings (### Needs a closer look, "A moderate parsing issue remains in the <summary> tag matching branch.", with the change list naming "a tag-name boundary after <summary> in one parsing branch"), at scripts/pr_review.py HEADING.

Deferred to #3206. HEADING's <summary branch requires no tag-name boundary, so <summaryx> would open a heading line. That gap predates #3204, which left the branch as it was apart from admitting a <details> opener ahead of it. No reviewer body carries a <summary-prefixed tag of another name, so nothing is misread today. The file recognizes these openers at seven sites with four different boundary rules, and #3204 already had to sweep four of them by hand. #3206 moves every site onto one primitive rather than patching this one on a promotion.

@codecov

codecov Bot commented Oct 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.49%. Comparing base (3b1fe04) to head (213ac08).
⚠️ Report is 396 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3205      +/-   ##
==========================================
+ Coverage   65.48%   65.49%   +0.01%     
==========================================
  Files          17       17              
  Lines        9754     9758       +4     
==========================================
+ Hits         6387     6391       +4     
  Misses       3367     3367              
Flag Coverage Δ
python-3.13 65.49% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ptr727

ptr727 commented Oct 18, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bound a Suppressed Block at a One-Line Previously-Missed Section in pr_review.py

2 participants