Repository navigation
Split pr_review.py into a Gate, a Copilot Adapter, and a Cleanup Script - #3410
Conversation
Read a reviewer body into a small typed result: coverage, refusal kind, finding counts by section, verdict class, headline, and the file table. Every spelling the reader matches lives in scripts/reviewers/<name>.json, so a Copilot wording change is a data edit plus one fixture. The body is parsed with markdown-it-py, and an unrecognized shape returns unknown fields and a note rather than raising. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move `cleanup` and `cleanup --sweep` with their queries into their own script and test module. `pr_review.py cleanup` forwards there, so the command lines the skills and the host-safety allow rule name keep working unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Route every body reading in pr_review.py through pr_review_copilot.py and delete the hand-written Markdown reader: CodeSpans, the fence and tag helpers, and the details and summary opener readers. The digest's three collect-split-count copies become one function, and the readers for Copilot's first body format, the statement carry, and the prose, narrative, and headline finding pairings are retired. Coverage from a file table and from a recorded partial stays. Comments and docstrings in pr_review.py and pr_cleanup.py are condensed under their ceilings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rewrite the scripts README sections for pr_review.py, the adapter, and pr_cleanup.py, dropping the readers and measurements of the retired format, and state the runbook's coverage rule without the statement carry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Name 40 as no Copilot review on the head, 41 as a refusal or quota state, and 42 as stated partial coverage, retiring the unrecognized shape and unstated coverage exits. Merge Gate items 2 and 4 read the digest's coverage= and shapes= fields instead, and the mirrors are regenerated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the adapter's cases into test_pr_review_copilot.py over constructed bodies in the current format only, delete the fixtures of the retired format and readers, and hold pr_review.py, the adapter, and pr_cleanup.py under 3,000, 1,500, and 1,200 lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Match a file table's glob rows one segment at a time, collect notes from every round, read the reviewer logins from the vocabulary, find `cleanup` behind a leading flag, decode the adapter's input as UTF-8, and restore the cleanup rationale a later edit needs. GOVERNANCE.md, the conduct skill, the README, and the evaluation doc stop describing the retired statement carry, and the new readings carry tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A count stated in prose outside the fleet-review marker reads as unknown coverage with a note, so a file table cannot stand in over a round saying it read part of the diff. A body the reader trips on is a note and never a refusal, each note names its round, the login drift note prints once, and the 41 and cleanup docs name where each reading appears. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…split # Conflicts: # .claude-plugin/fleet-skills/.source-digests/drive-pr # .claude-plugin/fleet-skills/.source-digests/merge-and-release # .claude-plugin/fleet-skills/.source-digests/pr-review-conduct # .claude-plugin/fleet-skills/.source-digests/repo-worktree # scripts/README.md
Hold pr_review.py, the adapter, and pr_cleanup.py under 3,000, 1,500, and 1,200 lines from tests/test_pr_review_size.py, the shape tests/test_guard_size.py sets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hub's tests import scripts that declare markdown-it-py inline, so a committed tests/requirements.txt names it. The Windows leg installs it with pip before unittest, and the validator's lint-only arm passes it to uvx with --with-requirements where a directory carries one. A requirements file under tests/ is not one beside the directory, so the hub root stays on the lint-only arm rather than moving to pytest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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. Comment |
|
A recorded local strict-review pass covers head |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3410 +/- ##
==========================================
Coverage ? 80.28%
==========================================
Files ? 21
Lines ? 9643
Branches ? 0
==========================================
Hits ? 7742
Misses ? 1901
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
A stated partial now outranks an unbelievable reading beside it, so exit 42 cannot be hidden. A count Copilot states in prose reads as partial, full, or unknown from its own numbers, only from paragraph text outside any section, and never overrides the marker, so an earlier prose partial keeps a table from standing in. The host-setup docs, the Python host-tool entry, and TODO.md stop saying the hub's scripts need only the standard library, the README states the reader's failure path, and the workflow comment comes out no longer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in coverage handling, dependency and entry-point behavior, cleanup URL encoding, and nested-link parsing.
7 open issues
Preserve dependency-free Python entry point · New Treat prose coverage counts as blocking · New Preserve partial readings over unknown markers · New Keep workflow comment within replacement length · New Document prose counts as UNKNOWN · New Accurately document adapter failure reading · New Use dependency-aware command in test docstring · New
What changed in this PR
This pull request separates review-gate logic, Copilot parsing, and cleanup operations into focused scripts. It also moves reviewer vocabulary into JSON and updates tests, CI dependencies, documentation, and generated skill mirrors.
Changes:
- Extracts reviewer parsing into
scripts/pr_review_copilot.pywith shared JSON vocabularies. - Moves cleanup functionality into
scripts/pr_cleanup.pywhile preserving the existing forwarding interface. - Updates workflows, tests, documentation, governance, and generated skill distributions for the new structure.
| File | Description |
|---|---|
tests/test_release_guards.py |
Updates validator assertions for test dependencies. |
tests/test_pr_review_size.py |
Adds script size ceiling coverage. |
tests/test_pr_review_copilot.py |
Tests adapter parsing and vocabulary contracts. |
tests/test_cli_usage.py |
Adds coverage for the new command-line entry points. |
tests/requirements.txt |
Declares markdown-it-py for the test environment. |
scripts/reviewers/qodo.json |
Defines Qodo vocabulary and parsing patterns. |
scripts/reviewers/copilot.json |
Defines Copilot vocabulary and parsing patterns. |
scripts/reviewers/coderabbit.json |
Defines CodeRabbit vocabulary and parsing patterns. |
scripts/README.md |
Documents the split scripts, dependencies, commands, and exit codes. |
scripts/pr_review_copilot.py |
Parses reviewer bodies into typed readings. |
scripts/pr_cleanup.py |
Implements branch cleanup and sweep operations. |
pyproject.toml |
Configures mypy for the optional Markdown parser dependency. |
OPERATIONS.md |
Updates local test and coverage commands. |
GOVERNANCE.md |
Updates the review-loop contract. |
docs/pr-reviewer-reference.md |
Updates request-recording and exit-code guidance. |
docs/pr-reviewer-evaluation.md |
Documents the extracted adapter and noted shapes. |
.github/workflows/validate-task.yml |
Supplies test requirements to validation coverage runs. |
.github/workflows/test-pull-request.yml |
Installs test dependencies before unittest execution. |
.github/skills/unattended-handoff/SKILL.md |
Updates promotion coverage handling. |
.github/skills/repo-worktree/SKILL.md |
Documents the cleanup script forwarding contract. |
.github/skills/python-codestyle/references/profiles.md |
Documents requirements-aware test execution. |
.github/skills/pr-review-conduct/SKILL.md |
Updates coverage, refusal, finding, and shape guidance. |
.github/skills/merge-and-release/SKILL.md |
Updates promotion coverage guidance. |
.github/skills/drive-pr/SKILL.md |
Updates unrecognized-shape handling. |
.github/copilot-instructions.md |
Updates review-loop instructions and exit codes. |
.claude-plugin/fleet-skills/skills/unattended-handoff/SKILL.md |
Regenerates the unattended-handoff skill mirror. |
.claude-plugin/fleet-skills/skills/repo-worktree/SKILL.md |
Regenerates the repo-worktree skill mirror. |
.claude-plugin/fleet-skills/skills/python-codestyle/references/profiles.md |
Regenerates the Python profile mirror. |
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md |
Regenerates the review-conduct skill mirror. |
.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md |
Regenerates the merge-and-release skill mirror. |
.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md |
Regenerates the drive-pr skill mirror. |
.claude-plugin/fleet-skills/.source-digests/unattended-handoff |
Refreshes the generated skill digest. |
.claude-plugin/fleet-skills/.source-digests/repo-worktree |
Refreshes the generated skill digest. |
.claude-plugin/fleet-skills/.source-digests/python-codestyle |
Refreshes the generated skill digest. |
.claude-plugin/fleet-skills/.source-digests/pr-review-conduct |
Refreshes the generated skill digest. |
.claude-plugin/fleet-skills/.source-digests/merge-and-release |
Refreshes the generated skill digest. |
.claude-plugin/fleet-skills/.source-digests/drive-pr |
Refreshes the generated skill digest. |
.agents/skills/unattended-handoff/SKILL.md |
Updates promotion coverage handling. |
.agents/skills/repo-worktree/SKILL.md |
Documents the extracted cleanup script. |
.agents/skills/python-codestyle/references/profiles.md |
Documents requirements-aware test execution. |
.agents/skills/pr-review-conduct/SKILL.md |
Updates coverage, refusal, finding, and shape rules. |
.agents/skills/merge-and-release/SKILL.md |
Updates promotion coverage guidance. |
.agents/skills/drive-pr/SKILL.md |
Updates unrecognized-shape handling. |
Review effort: Lite
A sentence describing Copilot's count, or the round's headline, no longer reads as its statement, and a count wrapped over a line break still reads. A count the reader places adds no note, only one it cannot believe. The skills spell every review-script command through uv, since the script now imports markdown-it-py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The headline paragraph is skipped by its token position rather than by comparing two renderings of its text, so a headline carrying a code span or emphasis is excluded too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
The headline exclusion skipped a count Copilot states as its first paragraph, losing a stated partial silently. The opening anchor already keeps a describing sentence out, and a false partial is the loud failure, so the exclusion is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
A count stated in the vetted What-changed section beside its file table is read, so a table cannot stand in over it, and only the counted finding sections are excluded. The headline is the first prose paragraph under the verdict, a count before it passed over, and the comment, test name, and README say what the opening anchor does and does not exclude. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A resolved entry or a change-list bullet opening on the count wording no longer reads as the round's statement, which could otherwise read a false full coverage silently. Only a top-level paragraph outside the counted finding sections is read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
A vetted section stating a count of its own entries, the resolved section among them, holds findings rather than a statement, so a count opening one of its nested entries is not read. The transport's docstring and the README name pr_cleanup.py among its importers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A table inside a counted finding section is a quotation rather than the round's file table, so its paths no longer join the coverage reading. The transport's docstring and the README name backlog_metrics.py among its importers and its exit contract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A table nested in a list item or a blockquote is a finding quoting one, so it joins no coverage reading, the same bound the prose count keeps. The test quoting a table now builds one, and fails without either guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A test name and four docstrings still named the retired 46 and 47 where the code now returns 41. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
A body the reader trips on reads as no refusal, unknown coverage and verdict, and a note, which the module docstring now says instead of unknown fields throughout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A body the reader trips on carries its failure as the coverage line, so COVERAGE IS UNKNOWN names it, and the usage text and README define UNKNOWN to include that case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
…#3417) ## Summary Promotes the pull requests merged into develop since release 2.0.1148. Each carries a maintainer decision of 2026-10-23 recorded on #3391. - [#3407](#3407): rewrites the write guard as a small lexer of 1,653 lines. It fails open under its stated threat model and fails closed only on requirement 4. A test holds it at 2,500 lines or fewer (item 1). - [#3409](#3409): adds CODESTYLE rules. Tests live in `tests/`, and a maintained parser replaces a hand-written one. A script declares its dependencies in PEP 723 metadata and runs as `uv run --no-project --script`. It switches the callers to that spelling and adds a managed allow rule to the agent-safety installer (item 1 follow-ups). - [#3410](#3410): splits `pr_review.py` into the gate, a Copilot adapter reading its vocabulary from JSON, and `pr_cleanup.py`. It collapses the exit codes and adds `tests/requirements.txt` and the CI runners. - [#3411](#3411): rebuilds the prose gate on Pygments and markdown-it-py, with its vocabulary and exemptions in `prose-gate.json` and a repo-local override. - [#3415](#3415): holds the Merge Gate on a part of a Copilot review body the tooling cannot place. The hold lasts until an owner acknowledges it with outcomes (item 4 follow-up). - [#3416](#3416): moves the audit self-test to `tests/` and reads YAML and Markdown with PyYAML and markdown-it-py. It adds the check registry `spec/checks.json` and splits out `spec/audit_markdown.py`. It shares `spec/carried_text.py` with carry, `build_dist`, and `canonical_review`. - [#3418](#3418): fixes the findings this promotion's first review round raised on the content above. It aligns the drive-pr escalation line with pr-review-conduct and names `tests/requirements.txt` where the prose said the tests need nothing installed. It also restores the `uvx` launcher in a release-guard assertion. - [#3419](#3419): fixes the finding the second review round raised. The Copilot adapter's prose path reads a coverage count longer than `COUNT_DIGITS` as `unknown`, sharing `count_state` with the marker path. - [#3420](#3420): fixes the finding the third review round raised, under the maintainer's ruling. `hub_fetch_run.py` falls back to `git ls-remote` where the API lookup of `main` fails, so the prose gate and its data are fetched at one commit. Where neither lookup resolves, the documented fail-open against `main` stays. - [#3421](#3421): fixes the findings the fourth review round raised. The Copilot adapter reads a finding count longer than `COUNT_DIGITS` as unknown rather than zero, through one length check and one None-aware sum. The host-setup gates-and-tests row says `uv` installs the test requirements. Closes #3408 ## Constituent coverage The Copilot rounds on this promotion ran at Lite effort and state no full coverage, so Merge Gate item 2 rests on the promotion case. Each change this promotion carries had full coverage on its own pull request into develop, at its merged head: | Pull request | Merged head | Coverage | | --- | --- | --- | | [#3407](#3407) | `9efc1002` | attested local pass, `coverage=local` | | [#3409](#3409) | `bcc7a9e1` | attested local pass, `coverage=local` | | [#3410](#3410) | `00cec726` | attested local pass, `coverage=local` | | [#3411](#3411) | `bd128136` | attested local pass, `coverage=local` | | [#3415](#3415) | `5be9db87` | attested local pass, `coverage=local` | | [#3416](#3416) | `b914f083` | attested local pass, `coverage=local` | | [#3418](#3418) | `3a86cb51` | attested local pass, and a Copilot table naming exactly the changed files, `coverage=table` | | [#3419](#3419) | `6e7b868d` | attested local pass, and a Copilot table naming exactly the changed files, `coverage=table` | | [#3420](#3420) | `9b492290` | attested local pass, and a Copilot table naming exactly the changed files, `coverage=table` | | [#3421](#3421) | `743010e6` | attested local pass, and a Copilot table naming exactly the changed files, `coverage=table` | 🤖 Generated with [Claude Code](https://claude.com/claude-code)


Split
scripts/pr_review.pyinto a gate core, a Copilot adapter whose vocabulary lives in JSON, and a separate cleanup script. The goal is containment: a change to Copilot's wording or layout becomes a data edit plus one fixture, and never reaches the gate.What Changed
scripts/pr_review.pyholds the CLI, the GraphQL and REST plumbing, the checks, the verdict, the wait loop, attest, comment, reply, and claims. It drops from 8,074 lines to 2,929.scripts/pr_review_copilot.pyis the adapter, 606 lines.read(body, reviewer, threads)returns a typed reading and never raises. The reading carries:CodeSpans, the fence and tag helpers, and the details and summary opener readers.scripts/reviewers/copilot.json,coderabbit.json, andqodo.jsoncarry the vocabulary. All three have the same keys and are read by one code path. A test holds each file to the keys the adapter reads.scripts/pr_cleanup.pycarriescleanupandcleanup --sweepunchanged, 1,155 lines.pr_review.py cleanupforwards to it, including when a flag comes before the subcommand, so the skills' command lines and the allow rule keep working.ccr-overview-v2marker reads as one note.collected, called for suppressed, previously missed, and CodeRabbit outside-diff findings.test_pr_review.pycovers the gate, CLI, and writes.test_pr_review_copilot.pycovers the adapter over constructed current-format bodies.test_pr_cleanup.pycovers cleanup.test_pr_review_size.pyholds the ceilings of 3,000, 1,500, and 1,200 lines, besidetest_guard_size.py.Exit Codes
status), or the covering review states no partial coverage and its verdict counts what it flagswaitpending at the timeout, includingCHECKS_PENDINGBLOCKED, required check stuckreplyoutcomescommentandattestoutcomesclaimsoutcomesExits 43 (unrecognized shape) and 45 (unstated coverage) are retired. The digest prints
shapes=NOTEDwith each note, and printscoverage=noneorcoverage=UNKNOWNwith anote:line. Merge Gate items 2 and 4 now read those fields rather than an exit code.Skills and Docs Changed
pr-review-conduct: Merge Gate item 2 now reads coverage fromcoverage=, and the 41 wording is updated. The promotion case keys oncoverage=none. Item 3's overview text is updated, and item 4 readsshapes=NOTED.merge-and-releaseandunattended-handoff: the promotion case readscoverage=noneinstead of exit 45.drive-pr: the unrecognized-shape bullet is updated.repo-worktree: namespr_cleanup.pyand the README contract section.python-codestylereferences/profiles.md: names--with-requirements tests/requirements.txt.build_dist.py..github/copilot-instructions.md,docs/pr-reviewer-reference.md(exit 48 is now 40), anddocs/pr-reviewer-evaluation.mdare updated.scripts/README.mdgets rewrittenpr_review.py,pr_review_copilot.py, andpr_cleanup.pysections.Runner Change
The hub's tests import scripts that need markdown-it-py, so the committed
tests/requirements.txtlists it. Each change that adds a library to a script's inline metadata adds it there too..github/workflows/test-pull-request.yml, Windows leg:python -m pip install --requirement tests/requirements.txtnow runs beforepython -m unittest discover -s tests..github/workflows/validate-task.yml, lint-only arm: where a directory carriestests/requirements.txt, it runsuvx --with-requirements tests/requirements.txt coverage@latest run -m unittest discover -s tests. Otherwise it runs plainuvx coverage@latest run ...as before.coverage.xmlis still written byuvx coverage@latest xml.python_directories.shape()counts only arequirements*.txtdirectly in the declared directory, andtests/requirements.txtsits one level down. The root stays lint-only, so there is still one runner and unittest still collectsTestCaseclasses.scripts/README.mdgive the same--with-requirementsform for local runs.pyproject.tomllets mypy run without the library installed, throughignore_missing_importsformarkdown_it.Decisions Beyond the Brief
fleet-reviewmarker. Without the table reading, every promotion would readcoverage=none. The adapter returns the table's paths, and the gate compares them with the changed files, so the comparison needs no API read.FINDINGS WITH NO THREADcounts the round's stated open count or its findings list beyond the open entries that link a thread on the pull request.statusstill exits 0 over a refusal. 41 stayswait's alone, as before.Verification
repo_gate.pyran before the develop merge. Every other result above is from the pushed content. I also ran the adapter over 42 live Copilot bodies from recent promotions, read-only. It placed every one with no note.Local-Review Findings and Their Outcomes
Every push carried a recorded
local-strict-reviewpass at thefrontiertier over the whole diff. There were sixteen passes in all.partial_on_recordsees it. A count directly under the verdict is read and is no headline.cleanupis forwarded behind a leading flag, and the adapter decodes its input as UTF-8.spec/host-tools.json, TODO.md, the evaluation doc, the transport's docstring, and every skill command line now match the code.coverage=UNKNOWN, and the usage text and README define UNKNOWN to include it.introduced, filed as Let an Earlier Partial Outrank an Unknown Head Reading in pr_review.py #3412: a head round readingunknownbeside an earlier partial reads UNKNOWN rather than 42. This fails closed, since UNKNOWN leaves Merge Gate item 2 unsatisfied. The last pass raised it after that push's edit budget was spent.style:table_matchesusingPurePosixPath.full_match. It would make[a character class, where the hand-built matcher keeps it literal on purpose.#!/usr/bin/env python3shebangs, and the write guard's remedy text naming a barescripts/pr_review.py reply. Both belong to the lane that moved callers touv.python-codestylereferences/profiles.md,spec/project-types.json, and README.md. It describes downstream lint-only subtrees.- **Files reviewed:**line no longer reads as coverage.Pre-existing Findings, Not Filed
host-setup/agent-safety/claude/install.pyand its README still register and check the bareBash(python3 scripts/pr_review.py:*)rule beside the uv spelling.Declined
🤖 Generated with Claude Code