Skip to content

Report the suppressed findings the thread poll cannot see - #477

Merged
ptr727 merged 2 commits into
developfrom
feature/pr-review-suppressed-findings
Jul 31, 2026
Merged

Report the suppressed findings the thread poll cannot see#477
ptr727 merged 2 commits into
developfrom
feature/pr-review-suppressed-findings

Conversation

@ptr727

@ptr727 ptr727 commented Jul 31, 2026

Copy link
Copy Markdown
Owner

What

scripts/pr_review.py now reads each review's body and reports the collapsed low-confidence findings the reviewThreads poll cannot see.

  • Q_FULL carries body on each review. Q_LIVE is untouched, so a liveness poll still fetches two scalars.
  • The digest line gains suppressed=N, counting the blocks carried by a review on the current head, so a finding answered before a push does not re-open after it.
  • Each block prints whole, under a SUPPRESSED: marker naming what closing it takes, because a suppressed finding has no thread to reply on or resolve.

Why

GOVERNANCE.md merge-gate condition 3 counts these as outstanding findings, and the runbook says to read them, but the one command written to answer "is this PR clean" polled threads only. That made the gate depend on remembering to run a second query by hand.

Details worth reviewing

  • The pattern is the runbook's alternation (Suppressed comments|low confidence) rather than one phrasing, since the heading has already appeared two ways. A contract case asserts the script's pattern is the alternation the runbook publishes, rather than a copy that can drift.
  • A body that names the block but carries no <details> wrapper is reported whole. Reporting zero because the markup moved would be the same false clean, one level up.
  • Verified against the real review bodies on Carry a multi-line string in every syntax, and answer the router-split items #473, where a per-file summary <details> sits beside the suppressed one and is correctly not reported.
  • scripts/README.md documents the behavior, and its own six spaced hyphens are corrected in passing, per the rule that a warn-only class leaves a hub-only file as it is next edited.

Checks

python3 -m unittest discover -s scripts = 175 pass (test_pr_review.py 18 -> 25). scripts/prose_lint.py --diff origin/develop, scripts/repo_gate.py, spec/validate.py, spec/audit.py --selftest, editorconfig-checker and markdownlint all clean.

The merge gate counts a review body's collapsed low-confidence block as an outstanding
finding, but the digest read reviewThreads alone, where that block never appears, so the
one command written to answer "is this PR clean" reported clean while findings stood. The
gate depended on remembering to run a second query by hand, which is what failed the last
time it mattered.

Q_FULL now carries each review's body, and the digest reports suppressed=N for the blocks
on the current head, so a finding answered before a push does not re-open after it. Q_LIVE
is untouched: a liveness poll still fetches two scalars. Each block prints whole where a
thread body truncates at 160, since a thread can be re-read at its id and a suppressed
finding has no thread, and it prints under a marker naming what closing it takes.

The pattern is the runbook's alternation rather than one phrasing, and a case asserts it is
the published one rather than a copy that can drift. A body naming the block outside a
details wrapper reports whole, because reporting zero when the markup moves is the same
false clean one level up. Verified against the real bodies on PR #473, whose per-file
summary block sits beside the suppressed one and is not reported as a finding.

The scripts README's own spaced hyphens are corrected in passing, per the rule that a
warn-only class leaves a hub-only file as it is next edited.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 17:30

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.

Pull request overview

This PR enhances the repo’s Copilot PR-review polling tooling by extracting and reporting “suppressed / low confidence” findings that appear only inside review-body <details> blocks (and therefore are invisible to reviewThreads polling), aligning the script’s behavior with the PR review etiquette/runbook expectations.

Changes:

  • Extend scripts/pr_review.py to fetch review bodies in the full query and emit suppressed-finding blocks in the digest output (scoped to reviews on the current head SHA).
  • Add targeted unit tests covering suppressed-block detection, scoping to head, and non-finding <details> blocks.
  • Document the new suppressed= digest behavior in scripts/README.md (plus small prose formatting fixes).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
scripts/pr_review.py Adds review-body parsing to detect and print suppressed findings in the digest output.
scripts/test_pr_review.py Adds test coverage for suppressed findings parsing and reporting behavior.
scripts/README.md Documents suppressed-findings reporting and updates related prose/list formatting.

Comment thread scripts/pr_review.py
Copilot's review found the first half: suppressed= counted blocks, so a body heading
"Suppressed comments (2)" reported 1 and read as a single finding. The count now comes
from the heading's (N), summed across the rounds on the head, floored at one so a block
never reports as zero and reads as a clean pass.

Running the tool on its own PR found the second half. The match was against the whole body,
so this PR's review, whose overview prose discusses suppressed and low confidence findings
while carrying none, reported suppressed=1. The match is now on the block's heading, taken
from its <summary>. Prose that merely names the phrase is not a finding, and a field that
cries wolf trains the reader to skim it.

The markup-moved fallback survives, since reporting zero when the wrapper changes is the
same false clean, but it now reads only the text outside the details blocks and requires a
heading carrying a count. Verified against the live bodies on both PRs: #473 reads one
finding from its real block, and #477 now reads zero.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 17:37

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.

Pull request overview

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

@ptr727
ptr727 merged commit ad065da into develop Jul 31, 2026
7 checks passed
@ptr727
ptr727 deleted the feature/pr-review-suppressed-findings branch July 31, 2026 17:42
ptr727 added a commit that referenced this pull request Jul 31, 2026
…479)

Forward promotion of four commits. `main` carries no content beyond the
merge-base (`f7c77d8`, #473), so this is a clean forward merge with no
conflicts.

## What promotes

- **#475 - Gate US English spelling where cspell does not reach.** The
cspell gate reads README and HISTORY only, so a British spelling
anywhere else in the tree had nothing checking it. The `spelling` rule
generates its banned words from stems rather than listing them one by
one, since an inflected spelling is as wrong as its base and a
hand-listed family drifts.
- **#476 - Sweep the spaced-hyphen prose class out of the carried
docs.** All seven carried documents to zero: `GOVERNANCE.md` 219,
`CODESTYLE.md` 53, `WORKFLOW.md` 51, `.github/copilot-instructions.md`
40, `AUDIT.md` 29, `repo-config/README.md` 20, `AGENTS.md` 3. The
remaining ~538 are hub-only (`reports/`, `docs/`, `spec/*.md`,
`STANDUP.md`, `catalog/`), stay warn-only, and are corrected as each
file is next edited, which works because nothing vendors them.
- **#477 - Report the suppressed findings the thread poll cannot see.**
The merge gate counts a review body's collapsed low-confidence block as
an outstanding finding, but the digest read `reviewThreads` alone, where
that block never appears, so the one command written to answer "is this
PR clean" reported clean while findings stood.
- **#478 - Sweep the semicolon prose class out of the carried docs.**
The exemption is fixed first (list-wide comma, per-table-cell scoping,
bullet-label colon), then 44 genuine clause joins recast across five
files. All six remaining carried files reach zero, the bar
`GOVERNANCE.md` already met.

## Verification on the merged `develop`

Both prose classes report zero across all seven carried documents. The
full 180-case script suite, `scripts/repo_gate.py`, `spec/validate.py`,
and `spec/audit.py --selftest` pass. The blocking prose run (charset,
dupword, spelling) is clean tree-wide, markdownlint reports 0 issues
over 39 files, and editorconfig-checker is clean. cspell's gated scope
(README + HISTORY) is clean; the tree-wide backlog is unchanged.

## Release

A human merge never auto-publishes, so this fires no release, as
designed. The latest release stays `2.0.108`.

## Re-vendor debt

This promotion changes prose in all seven carried documents, so every
downstream repo goes further out of date against canonical. That debt is
pre-existing and still untracked - no issue covers it and filing one has
not been authorized. `spec/audit.py --branch <ref>` checks a repo's
convergence before it promotes.

🤖 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