Skip to content

Read a suppressed section nested inside the review-details wrapper - #564

Merged
ptr727 merged 1 commit into
developfrom
suppressed-nested-heading
Aug 5, 2026
Merged

Read a suppressed section nested inside the review-details wrapper#564
ptr727 merged 1 commit into
developfrom
suppressed-nested-heading

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Fixes the false clean reported in #562: scripts/pr_review.py printed suppressed=0 over a review body carrying ### Suppressed comments (2), so the gate that decides whether a review round is answered reported a clean round over findings no thread will ever surface.

Cause

The reviewer moved the section from its own <details> wrapper to a markdown heading nested inside the Review details wrapper, and suppressed_blocks() missed it twice over:

  • The primary path matched the heading against the wrapper's <summary>, which now reads Review details.
  • The fallback that exists for exactly this case scanned DETAILS.sub('', body), which deletes every <details> block, including the one the heading now sits in.

Fix

Each region (every wrapper's contents, plus what is left outside them all) is scanned line by line for the heading, so the section is found as its own wrapper's <summary>, as a markdown heading nested inside another wrapper, or bare in the body. Neither old shape is retargeted away, since both appear across the rounds of a single pull request. A block now starts at its own heading, so finding_count() reads the heading's own (N) rather than the wrapper's, which would have floored two findings to one.

Verification

python3 -m unittest discover -s scripts passes (247 tests), with four cases added for the nested shape: that it reports at all, that the count is the heading's own, that both shapes report together in one run, and that the neighbouring file-summary wrapper is still not read as a finding.

Replayed over the reviewer's own bodies on the last seventeen pull requests of this repository, the new parse recovers 6 findings across 5 rounds that previously read as clean (on #546 and #561), and changes no count that was already right.

The runbook section in .github/copilot-instructions.md gains the same fact, since its rule to match more than one phrasing did not say the section also moves, and a filter reading a wrapper's <summary> reports zero on the nested shape.

Related: #562

The reviewer moved the suppressed-findings section from its own <details> wrapper to a
markdown heading inside the Review details wrapper, and the parse missed it twice over.
The primary path matched the heading against the wrapper's <summary>, which now reads
'Review details', and the fallback for a moved wrapper scanned the body with every
<details> block deleted, which deletes the region the heading sits in. The digest then
reported suppressed=0 over a body carrying two findings, the false clean the whole gate
exists to prevent.
Each region is now scanned line by line for the heading, so the section is found inside a
wrapper, nested in another wrapper's contents, or bare in the body, and a block starts at
its own heading so the count is the heading's (N) rather than the wrapper's. Replayed over
the reviewer's bodies on the last seventeen pull requests, this recovers six findings across
five rounds that read as clean and changes no count that was already right.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI lite review requested due to automatic review settings August 5, 2026 14:40

CopilotAI 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.

Pull request overview

Updates the PR-review digest parser so it reliably detects and counts Copilot’s “suppressed / low confidence” findings even when the section is nested inside another <details> wrapper, preventing false “clean round” reports.

Changes:

  • Teach suppressed_blocks() to scan all non-overlapping regions (each <details> body plus the outside text) for suppressed headings, including nested markdown headings.
  • Make heading_of() and finding_count() operate on a block that starts at the finding’s own heading so counts come from the correct (N).
  • Add focused unit tests covering the nested shape, correct counting, coexistence with the legacy wrapper shape, and avoiding false positives from file-summary tables.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

FileDescription
scripts/pr_review.pyFix suppressed-section detection by scanning each region for headings and counting from the finding’s own heading.
scripts/test_pr_review.pyAdd regression tests for nested suppressed sections and correct counting/printing behavior.
.github/copilot-instructions.mdDocument that the suppressed section can move (including nesting) and must be matched wherever it appears.

@ptr727
ptr727 merged commit 94e488c into developAug 5, 2026
7 checks passed
@ptr727
ptr727 deleted the suppressed-nested-heading branch August 5, 2026 14:50
ptr727 added a commit that referenced this pull request Aug 6, 2026
…574)
Promotes the nine commits `develop` has carried since the previous
promotion (#555) to `main`. Merge commit only, no squash, and `develop`
is not deleted.
## What lands
- **#560** Standardize the static-site-deploy repo type and its
destination, including the deploy-verification guarantee and a retention
rule that records which side owns the prune.
- **#561** Ask for a blocked decision instead of reporting it, and scope
the clickable-link rule to a surface that renders markdown.
- **#564** Read a suppressed section nested inside the review-details
wrapper, so `scripts/pr_review.py` stops reporting a clean round over
findings that reach no thread.
- **#566** Settle Markdown capitalization in `CODESTYLE.md`, and fix the
two `TODO.md` prose defects the digest defect had hidden.
- **#568** Check a `driftNote` on every run rather than only on an
otherwise clean audit, and state what the audit does not evaluate.
- **#569** Name who trips the production ref gate, and state that a
harness refusal is a different thing from the maintainer's permission
and is not lifted by it.
- **#571** Re-audit `ptr727/Blog` against the hugo type and bump the
conformance matrix.
- **#573** Scope the semicolon exemption to the sentence its list lives
in, with the 44 newly reported occurrences recorded on #519 rather than
swept here.
- **#572** Exempt a verbatim section from the coordination-reference
scan.
## Issues closed
Fixes#562. Closes#565.
The closing keywords sit on this promotion rather than on the feature
pull requests, because GitHub fires them only on a merge into the
default branch. #456, #558, #563, #567, and #570 are already closed
against their merged fixes.
## Verification
`git merge-tree` reports no conflict between `origin/main` and
`origin/develop`, so this promotion needs no throwaway resolution
branch. Every constituent pull request merged green with its review loop
closed.
Sign up for freeto 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

@ptr727