Skip to content

Print a Clean Verdict's Headline as an Advisory Line in pr_review.py - #3221

Merged
ptr727 merged 3 commits into
developfrom
feature/clean-headline-advisory
Oct 19, 2026
Merged

ptr727 merged 3 commits into
developfrom
feature/clean-headline-advisory

Conversation

@ptr727

@ptr727 ptr727 commented Oct 19, 2026

Copy link
Copy Markdown
Owner

Summary

Implements the maintainer's decision on #2567, "print it, do not gate".

A clean-verdict Copilot round can state no open finding, open no thread, and collapse no suppressed or previously-missed block while its headline sentence still names a remaining nit. pr_review.py read every such round as a clean pass and printed nothing.

  • uncounted_verdict takes the verdict set as a parameter, so the flagging reading behind exit 50 and the new clean reading share one guard.
  • The digest prints a clean round's headline under CLEAN VERDICT HEADLINE. The line is advisory and changes no exit code. It does not try to tell a new nit from one already answered, per the decision.
  • The usage text, scripts/README.md, and pr-review-conduct describe the advisory line, with the skill distributions regenerated.

Tests

  • A clean verdict whose headline names findings prints the headline, and status and wait each exit 0. Each command's output is read on its own.
  • A clean headline naming an answered thread prints and still exits 0.
  • A clean verdict with no headline prose prints no advisory.
  • A flagging verdict prints no advisory and still exits 50.

Run against the base pr_review.py, the two positive tests fail and the two negative tests pass. The full tests.test_pr_review suite passes, as do ruff, mypy, prose_lint (including --check sentence-length), the eol gate, spec/validate.py, build_dist.py --check, markdownlint, cspell, and editorconfig-checker.

Local strict review

There were three recorded passes. Round 1 found an overstated exit-code claim on three surfaces, a docstring summary made false by the new parameter, a test docstring with a false causal claim, a merged output buffer, and two long sentences. Round 2 found a pronoun that bound to the wrong noun. Each was fixed. Round 3 found nothing, so no introduced finding remains open.

Refs #2567

🤖 Generated with Claude Code

ptr727 and others added 3 commits October 19, 2026 00:51
A clean-verdict Copilot round that counts no finding, opens no thread, and
collapses no suppressed block can still name a remaining nit in its headline
sentence. The digest now prints that headline under CLEAN VERDICT HEADLINE,
through the same reader the flagging verdict's exit 50 uses, and the exit
stays 0. Nothing in the body says whether the nit is new or an answered
thread, so the line is advisory and adds no exit code.

Closes #2567

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The usage text, the README, and pr-review-conduct said a clean verdict
on those terms exits 0, while status still returns 42, 43, or 45 ahead
of it. Each now says the line changes no exit code, as the digest does.
The test reads status and wait output separately.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"counts none" bound to `verdicts`, the only plural before it. The
summary now says the round counts no finding.

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

ptr727 commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

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

@coderabbitai

coderabbitai Bot commented Oct 19, 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: 2fcf4e05-9196-4c29-855a-f6e954c18cc2
  • 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.

🟢 Approval recommended

No unresolved review findings remain, and the behavior is covered by tests and documentation.

No open issues 🎉

What changed in this PR

Adds advisory output for clean Copilot verdict headlines without changing exit codes.

Changes:

  • Generalizes verdict detection for clean and flagging results.
  • Adds advisory headline output and focused tests.
  • Updates documentation and generated skill distributions.
File Description
tests/​test_pr_review.py Tests advisory output and exit behavior.
scripts/​README.md Documents advisory behavior.
scripts/​pr_review.py Implements clean-verdict headline reporting.
.github/​skills/​pr-review-conduct/​SKILL.md Updates generated review guidance.
.claude-plugin/​fleet-skills/​skills/​pr-review-conduct/​SKILL.md Updates generated Claude guidance.
.claude-plugin/​fleet-skills/​.source-digests/​pr-review-conduct Updates the generated source digest.
.agents/​skills/​pr-review-conduct/​SKILL.md Updates canonical review guidance.

Review effort: Lite

@codecov

codecov Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@66b2382). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #3221   +/-   ##
==========================================
  Coverage           ?   65.52%           
==========================================
  Files              ?       17           
  Lines              ?     9771           
  Branches           ?        0           
==========================================
  Hits               ?     6402           
  Misses             ?     3369           
  Partials           ?        0           
Flag Coverage Δ
python-3.13 65.52% <100.00%> (?)

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 merged commit 48c1e76 into develop Oct 19, 2026
14 checks passed
@ptr727
ptr727 deleted the feature/clean-headline-advisory branch October 19, 2026 08:10
ptr727 added a commit that referenced this pull request Oct 19, 2026
…in (#3222)

- #3221 makes `pr_review.py` print a clean-verdict Copilot round's
headline under `CLEAN VERDICT HEADLINE` when the round counts no
finding, opens no thread, and collapses no block. The line is advisory
and changes no exit code. It shares one guard with the flagging reading
behind exit `50`, through a verdict-set parameter on
`uncounted_verdict`. This implements the maintainer's "print it, do not
gate" decision on #2567.
- #3219 renames the carried hook helper
`catalog/snippets/hub-fetch-run.py` to `hub_fetch_run.py`. It updates
every reference and keeps accepting the former name wherever a carrier
that copied it earlier would otherwise misclassify.

#3221 edits `pr-review-conduct`, so this machine's installed Skills
change at release.

Closes #2567
Closes #3143

🤖 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