Hold the Merge Gate on Unplaced Review Text Until Acknowledged by ptr727 · Pull Request #3415 · ptr727/ProjectTemplate

ptr727 · 2026-10-23T22:06:34Z

Summary

Decision (ptr727, 2026-10-23) on #3391 item 4 follow-up, unrecognized shapes hold until acknowledged.

If the adapter cannot place a part of a Copilot review body on a round covering the head, that part now holds the merge gate. The hold stays until an owner, member, or collaborator acknowledges it with written outcomes. Before this change the part printed shapes=NOTED and decided no exit code. Copilot's threaded findings are safe under any format, because threads come from GraphQL. The hold therefore targets a finding that exists only in the body, inside a layout the adapter does not place.

Adapter (scripts/pr_review_copilot.py): Reading.unplaced carries each part the reader cannot place, together with its own raw text:
- For an unmarked layout or a reader failure, the part is the whole body.
- For an unknown collapsed section, it is the <details> block.
- For an unknown top-level heading or label, it runs to the next heading, label, table, top-level HTML block, effort tail, coverage statement, or placed section.
fingerprint() is the first 12 hex characters of a SHA-256 over the set's sorted text. excerpt() cuts a part to 12 lines of 160 characters. These remain plain notes, since none of them carries text a finding could sit in:
- an empty body
- an unbelievable coverage count
- a label nested in a list or in a placed section
Gate (scripts/pr_review.py): shape_hold() gathers the unplaced parts of every round covering the head. It then looks for an acknowledgment: a comment carrying  as an HTML line of its own outside a fence, with prose beside it. The comment must come from an OWNER, MEMBER, or COLLABORATOR, and must not be minimized.
- trusted_comments() reads both this marker and the attestation marker, so the trust and placement rule lives in one place.
- The digest prints shapes=HELD and an UNPLACED ON THE HEAD (fingerprint=<fp>) block holding each part's note and excerpt. Once the hold is answered it prints shapes=ACKNOWLEDGED.
- A part on an earlier round still prints under REVIEWER OUTPUT NOTED.
Acknowledgment: pr_review.py comment <n> --repo <o/r> --fingerprint <fp> --body <outcomes> posts the outcomes with the marker beneath them.
- This extends comment, the alternative the design allowed, instead of adding an acknowledge subcommand. Its parser wiring alone pushed pr_review.py past its 3,000-line ceiling. The file is now 2,988 lines.
- Posting does not check the fingerprint first. The local review ruled that check over-hardening for a trusted input. A wrong fingerprint posts a comment that clears nothing, and status keeps printing the held one.
Skills:
- pr-review-conduct Merge Gate item 4 is rewritten in place.
- Its promotion-merge bullet names the hold.
- Its "Escalate to the maintainer when" list gains one case: an unplaced item that cannot be told to be a finding, escalated with its raw text quoted.
- drive-pr's unrecognized-shape bullet matches the new item 4.
- Mirrors are regenerated.

Exit code

The hold reuses 50, and no new code is added. 50 already means that a finding the round covering the head names only in its body awaits an outcome, which is the uncounted-verdict headline. An unplaced part is the same condition. The status line is status=UNPLACED_PARTS_UNACKNOWLEDGED. The note wait prints beside a 50 while checks settle now covers both causes.

Digest wording

The fixture below is constructed, and <fp> stands for the printed fingerprint.

repo=o/r pr=7 head=aaaaaaaa ... shapes=HELD ...
  UNPLACED ON THE HEAD (fingerprint=<fp>): a round covering the head carries text the reader cannot place, where a finding can sit. Read each part whole in the review body, the excerpt only locating it, and give each item an outcome, fixed with its SHA, declined with evidence, or listed as pre-existing in the PR body. Post them with `comment 7 --repo o/r --fingerprint <fp> --body <outcomes>`
    a collapsed section this reader has no spelling for: `confidence notes (1)`
      <details>
      <summary>Confidence notes (1)</summary>
      The retry bound is off by one.
      </details>
status=UNPLACED_PARTS_UNACKNOWLEDGED the round covering the head carries text the reader cannot place, under UNPLACED ON THE HEAD. Acknowledging fingerprint <fp> with `comment --fingerprint` and each item's outcome clears this code

Merge Gate item 4 as written

4. Every part of a round covering the head that the tooling could not place is answered. The
   digest prints an excerpt of each under `UNPLACED ON THE HEAD` with a fingerprint for the set.
   It reads `shapes=HELD`, and `status` and `wait` exit `50`.
   - Read each part whole in the review body, since the excerpt only locates it. Give every item
     in it item 3's outcome: fixed with its SHA, declined with evidence, or listed as pre-existing
     in the pull request body.
   - Post the outcomes with `pr_review.py comment <number> --repo <owner>/<repo> --fingerprint
     <fp> --body <outcomes>`. One from an owner, member, or collaborator clears the hold,
     `shapes=ACKNOWLEDGED`. New unplaced text on a new head owes a new acknowledgment.
   - The spelling is a data edit to the matching `scripts/reviewers/*.json` in the hub plus one
     fixture. Whoever acknowledged the part makes it in a pull request of its own, never an issue.
   - An item that reads as neither a finding nor not one goes to the maintainer with its raw text
     quoted, per "Escalate to the maintainer when" below.

   A part on an earlier round, or a label nested in a placed part, prints under `REVIEWER OUTPUT
   NOTED` and decides no exit code. Read that block rather than the `shapes=` field, which names a
   head hold first. Read each such part by hand, and give any finding in it item 3's outcome.

Verification

Every command below ran from the worktree on head 5be9db87, with the uv, mypy, ruff, and coverage caches under a scratch directory.

Command	Result
`uvx ruff@latest check .`	`All checks passed!`, exit 0
`uvx ruff@latest format --check .`	`265 files already formatted`, exit 0
`uvx mypy@latest`	`Success: no issues found in 71 source files`, exit 0
`uvx --with-requirements tests/requirements.txt coverage@latest run --source=scripts,spec,host-setup -m unittest discover -s tests`	`Ran 2761 tests`, `OK (skipped=8)`, exit 0
`uvx coverage@latest report --include='scripts/pr_review*.py'`	`pr_review_copilot.py` 97%, the two files together 96%
`python3 scripts/build_dist.py` then `python3 scripts/build_dist.py --check`	`Generated skill distributions are current.`, exit 0
`python3 scripts/prose_lint.py . --diff origin/develop`	`14 of 391 file(s) read, 465 changed line(s)`, no violation, exit 0
`python3 scripts/repo_gate.py`	`composite-actions`, `eol`, `eol-coverage`, and `sha-pin` each `0 issue(s)`, exit 0
`tests/test_pr_review_size.py` (inside the suite)	`pr_review.py` at 2,988 lines, under its ceiling of 3,000

The tests use constructed fixtures only, and none makes a live API call. They cover each case below:

A noted shape on the head holds, exit 50 and shapes=HELD, with its excerpt and fingerprint printed.
An acknowledgment by OWNER, MEMBER, or COLLABORATOR clears the hold, exit 0 and shapes=ACKNOWLEDGED.
An acknowledgment by CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, NONE, or no association does not clear it.
A marker with no outcome, a marker inside a fence, and a minimized acknowledgment do not clear it.
A new head with new unplaced text re-holds under a new fingerprint.
The same unplaced text on a new head at another effort level keeps its fingerprint and its acknowledgment.
comment --fingerprint posts the body with the marker the gate reads.

Each push carries a recorded adversarial local strict-review pass at the frontier tier, attested on this pull request. The first push spent its edit budget, so it listed the findings below. The second push fixed every one of them in c6896b72, rewrapped in 5be9db87, and the review threads that raised the same points are answered with that SHA. The final pass on 5be9db87 raised two style findings, each declined because it is a preference. One is a 26-word sentence under the opt-in sentence-length check, and the other is a 105-character docstring line, which ruff does not enforce.

Local review findings, fixed in `c6896b7`

scripts/pr_review_copilot.py placed(): a bold section-count line such as **0 open findings** is not a stop, so an unplaced label before it absorbs the count line and its list. The same unplaced text then takes a different fingerprint depending on that line.
scripts/pr_review_copilot.py stops: a top-level non-file table or a mid-body HTML comment ends a part, and the text after it lands in no part. That text is neither excerpted nor fingerprinted.
scripts/pr_review_copilot.py _read: a heading or label inside an unrecognized top-level <details> becomes a second part, which duplicates text the section's own part already carries.
.agents/skills/pr-review-conduct/SKILL.md and scripts/README.md: they say "a label nested in a placed part", but the code makes a label nested anywhere a note, a top-level list or blockquote included.
scripts/pr_review.py attestation docstring and the README attest paragraph: neither says that a minimized attestation no longer counts.
scripts/pr_review_copilot.py: the module docstring says every note carries text, and the comment on read's except still calls a reader failure a note.
docs/pr-reviewer-evaluation.md: it said a shape the adapter cannot place "stays a note in the digest rather than an exit".
tests/test_pr_review_copilot.py: TestUnplaced was inserted between two TestNotes methods, which moved test_a_pathological_body_reads_in_bounded_time under the wrong class.

Pre-existing findings, not filed

scripts/README.md:190 and scripts/pr_review.py:17 each hold a 31-word sentence that predates this change, on a line the diff touched (sentence-length, opt-in).
A top-level prose paragraph after the headline, carrying no label and no heading, becomes neither a note nor an unplaced part. A finding written there holds nothing, and that was already true before this change.
scripts/handoff.py keeps its own WRITERS set and its own trusted-and-shown comment filter, the same job trusted_comments now does here. Sharing one primitive in gh_graphql.py is a change of its own.

🤖 Generated with Claude Code

Decision (ptr727, 2026-10-23) on #3391 item 4 follow-up, unrecognized shapes hold until acknowledged. A Copilot body part the adapter cannot place on a round covering the head now holds the gate rather than printing a note: the digest prints `shapes=HELD`, each part's text as a bounded excerpt under `UNPLACED ON THE HEAD`, and a 12-hex fingerprint over the set, and `status` and `wait` exit 50. An owner, member, or collaborator clears the hold by posting the outcomes with `pr_review.py comment --fingerprint <fp>`, which appends `` and refuses with 69 where the head holds no set under that fingerprint. The digest then reads `shapes=ACKNOWLEDGED`. A part on an earlier round stays a note. pr-review-conduct Merge Gate item 4, its escalation list, and drive-pr's unrecognized-shape bullet are rewritten to match, and the mirrors are regenerated.

Read each unplaced part whole rather than its excerpt, end a heading or label part at the next placed block, skip labels nested in a list or a placed section, move the acknowledgment marker into the gate beside the attestation's and share one trusted-comment reader, and drop the fingerprint pre-check and its exit code, since a wrong fingerprint posts a comment that clears nothing and status keeps printing the held one.

A heading or label part now ends at the effort tail, a coverage statement, or a top-level HTML block too, so the same unplaced text on a new head keeps its fingerprint. A label nested in a list or a placed section stays a note rather than vanishing. A minimized trusted comment no longer counts, for the acknowledgment and the attestation alike, and the docs point at the NOTED block rather than the single shapes field.

…re/unrecognized-shape-ack

ptr727 · 2026-10-23T22:06:41Z

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

coderabbitai · 2026-10-23T22:06:43Z

Important

🔍 Trigger review

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

⚙️ Run configuration

Source mode: Auto
Repository: ptr727/ProjectTemplate
Branch: feature/unrecognized-shape-ack
File: .coderabbit.yaml
Commit: 5be9db8
Layers: Repository YAML
Review profile: CHILL
Plan: Advanced
Run ID: 7d27c4ea-0931-45ca-a4b3-a4f821d95df1

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

A heading or label part now ends only at a heading, a label, a counted or vetted section line, the effort tail, a coverage statement, the file table, the fleet-review marker, or a collapsed section, so another table or an HTML aside stays inside it and a count line does not. A heading or label inside an unrecognized collapsed section is no second part. The docs name nested labels and hidden attestations as the code reads them, and the evaluation doc states the hold.

Copilot

🟡 Changes recommended

Unresolved findings affect acknowledgment correctness, comment preservation, parser completeness, and guidance consistency.

9 open issues

Require prose acknowledgment beyond the marker · New
Preserve unplaced spans after raw separators · New
Claim unknown collapsed sections before scanning contents · New
Stop at recognized section-count paragraphs · New
Describe all nested labels as notes · New
Align review guidance with nested-label parser behavior · New
Document minimized-comment exclusion for attestations · New
Accurately document unplaced part fields · New
Update stale reader-failure behavior comment · New

What changed in this PR

This pull request adds a merge-gate hold for Copilot review text that the adapter cannot place, requiring a trusted acknowledgment with written outcomes before the hold clears.

Changes:

Tracks unplaced review parts with fingerprints, excerpts, and exit-code 50 handling.
Extends status, wait, and comment with acknowledgment support and trusted-comment filtering.
Updates tests, documentation, skills, and generated skill mirrors.

The review identified unresolved issues in acknowledgment validation, comment-body preservation, parser boundary handling, and related documentation.

File	Reviewed change
`tests/test_pr_review.py`	Tests merge-gate holds, acknowledgments, fingerprints, and CLI behavior.
`tests/test_pr_review_copilot.py`	Tests unplaced-part parsing, excerpts, fingerprints, and boundary cases.
`scripts/README.md`	Documents shape holds, acknowledgment flow, and exit behavior.
`scripts/pr_review.py`	Implements shape holds, trusted acknowledgments, digest output, and CLI support.
`scripts/pr_review_copilot.py`	Extracts unplaced review parts and generates fingerprints and excerpts.
`.github/skills/pr-review-conduct/SKILL.md`	Updates merge-gate guidance for unplaced review content.
`.github/skills/drive-pr/SKILL.md`	Updates PR-driving guidance for shape holds.
`.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md`	Regenerated Claude skill mirror for merge-gate guidance.
`.claude-plugin/fleet-skills/skills/drive-pr/SKILL.md`	Regenerated Claude skill mirror for PR-driving guidance.
`.claude-plugin/fleet-skills/.source-digests/pr-review-conduct`	Updates the generated conduct-skill source digest.
`.claude-plugin/fleet-skills/.source-digests/drive-pr`	Updates the generated drive-skill source digest.
`.agents/skills/pr-review-conduct/SKILL.md`	Updates canonical merge-gate guidance.
`.agents/skills/drive-pr/SKILL.md`	Updates canonical PR-driving guidance.

_{Review effort: Lite}

codecov · 2026-10-23T22:13:34Z

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff             @@
##             develop    #3415   +/-   ##
==========================================
  Coverage           ?   80.27%           
==========================================
  Files              ?       21           
  Lines              ?     9731           
  Branches           ?        0           
==========================================
  Hits               ?     7812           
  Misses             ?     1919           
  Partials           ?        0

Flag	Coverage Δ
python-3.13	`80.27% <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 · 2026-10-23T22:20:33Z

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

…#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)

ptr727 added 4 commits October 23, 2026 14:39

Merge remote-tracking branch 'refs/remotes/origin/develop' into featu…

74d0373

…re/unrecognized-shape-ack

Copilot AI lite review requested due to automatic review settings October 23, 2026 22:06

Copilot started reviewing on behalf of ptr727 October 23, 2026 22:07

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

Rewrap Two Lines and Name Every Trusted Author in the Evaluation Doc

5be9db8

ptr727 merged commit d95bdba into develop Oct 23, 2026
13 checks passed

ptr727 deleted the feature/unrecognized-shape-ack branch October 23, 2026 22:31

ptr727 mentioned this pull request Oct 24, 2026

Promote the Write Guard Rewrite, uv Scripts, and Parser Gates to Main #3417

Merged

ptr727 mentioned this pull request Oct 24, 2026

Session Handoff [default]: Two Burndown Rounds Released as 2.0.1313 and 2.0.1319, Round 3 Next #2361

Open

Navigation Menu

Repository navigation

Hold the Merge Gate on Unplaced Review Text Until Acknowledged - #3415

Hold the Merge Gate on Unplaced Review Text Until Acknowledged#3415
ptr727 merged 6 commits into
developfrom
feature/unrecognized-shape-ack

ptr727 commented Oct 23, 2026 •

edited

Loading

Uh oh!

ptr727 commented Oct 23, 2026

Uh oh!

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

ptr727 commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ptr727 commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Exit code

Digest wording

Merge Gate item 4 as written

Verification

Local review findings, fixed in c6896b7

Pre-existing findings, not filed

Uh oh!

ptr727 commented Oct 23, 2026

Uh oh!

coderabbitai Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ptr727 commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ptr727 commented Oct 23, 2026 •

edited

Loading

Local review findings, fixed in `c6896b7`

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

codecov Bot commented Oct 23, 2026 •

edited

Loading