Skip to content

Read a Section Heading Through One Reduction in pr_review.py - #3179

Merged
ptr727 merged 4 commits into
developfrom
feature/heading-reduction
Oct 17, 2026
Merged

ptr727 merged 4 commits into
developfrom
feature/heading-reduction

Conversation

@ptr727

@ptr727 ptr727 commented Oct 17, 2026

Copy link
Copy Markdown
Owner

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 (Read a Section Heading Through One Reduction in pr_review.py #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 (Read a Section Heading Through One Reduction in pr_review.py #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 (Read a Finding Entry Titled With the Suppressed Marker as No Section in pr_review.py #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

ptr727 and others added 3 commits October 17, 2026 07:53
One reduction, `reduced`, now serves `normal`, `unwrap`, and `heading_count`, and one
`LEADING_COUNT` rule serves both vetting and counting, so a summary's vetted role and its
count cannot disagree. The vetted-but-unread check now compares which previously-missed
summaries the blocks hold rather than how many blocks there are. A finding entry's own
summary, which opens on a severity badge, is never a section heading and never bounds one.

Fixes #3174
Fixes #3173

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A section now ends at the next counted heading of either kind, its own included, so two
sections sharing one wrapper are two blocks and each is counted by its own heading. Read as
one block, the digest counted only the first heading's findings. The leading-count rationale
moves into `spaced`'s docstring, and `counted_missed` builds its section with
`entries_section`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Scanning on past a section's bound read prose naming the suppressed marker beside a `(N)` as
a second section. Past a region's first section, only the sibling heading it ends at can now
open another, so the nested pair still splits while prose past the bound opens nothing. The
`sibling_heading` docstring keeps its "the two" bound to the paragraph that names them, and
its CodeRabbit sentence states what the count requirement actually does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 17, 2026 15:09
@ptr727

ptr727 commented Oct 17, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head 04a1841bc68e107b1f2dbeda24110570a03a7009, the content this pull request carries at that commit against develop, and it recorded 2 findings.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1377d4b8-c794-4d33-ae9f-56126fc74ead
  • 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.

🟡 Changes recommended

A critical issue remains in deriving read sections from each block’s own heading.

3 open issues
What changed in this PR

This PR centralizes heading normalization and section parsing in pr_review.py, with regression tests for section boundaries and finding entries.

Changes:

  • Consolidates heading reduction and count parsing.
  • Prevents finding entries from being treated as sections.
  • Adds regression coverage for nested and count-first headings.
File Description
scripts/​pr_review.py Updates heading parsing, section detection, and unread-summary tracking.
tests/​test_pr_review.py Adds regression tests for the new parsing behavior.

Review effort: Lite

Comment thread scripts/pr_review.py
Comment thread scripts/pr_review.py Outdated
Comment thread tests/test_pr_review.py Outdated
@codecov

codecov Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.14%. Comparing base (8198ee0) to head (aad5801).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3179      +/-   ##
===========================================
+ Coverage    64.08%   64.14%   +0.05%     
===========================================
  Files           17       17              
  Lines         9183     9197      +14     
===========================================
+ Hits          5885     5899      +14     
  Misses        3298     3298              
Flag Coverage Δ
python-3.13 64.14% <100.00%> (+0.05%) ⬆️

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.

The `sibling_heading` docstring now says a same-kind section bounds where it follows
directly, which is what the chain in `marker_blocks` does. The overlong sentence in the new
test's docstring is split.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ptr727

ptr727 commented Oct 17, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head aad58012c15499a5421c151224c13fa1395c8e81, the content this pull request carries at that commit against develop, and it recorded 0 findings.

@ptr727
ptr727 merged commit f4eb405 into develop Oct 17, 2026
13 checks passed
@ptr727
ptr727 deleted the feature/heading-reduction branch October 17, 2026 15:31
ptr727 added a commit that referenced this pull request Oct 17, 2026
…ain (#3183)

- #3179 reads a section heading in `pr_review.py` through 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 #3180.
- #3181 pairs a `Review findings:` or `Open findings remain:` entry with
its thread past a `:LINE` or `:START-END` suffix. 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](https://claude.com/claude-code)
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.

2 participants