Summary
Validate the structured Copilot review marker introduced by #793 against live GitHub reviews, then make pr_review.py enforce every field that proves useful.
Context
#793 instructs Copilot code review to end each review body with:
<!-- fleet-review: reviewed=N changed=N findings=N -->
scripts/pr_review.py reads reviewed and changed as a stable alternative to Copilot's changing coverage prose. It retains the legacy prose readers for reviews created before the marker ships. An absent coverage statement blocks with exit 45.
The implementation deliberately does not treat one local parser test as evidence that GitHub-hosted Copilot follows the instruction. It also parses the findings field as part of the marker shape but does not yet compare that value with inline or body-only findings.
Measurement
After #793 is available on a pull request head:
- Request Copilot review on at least one clean change and one change expected to produce a finding.
- Record whether the review body carries exactly one marker and whether GitHub preserves the HTML comment.
- Record whether
reviewed and changed match the pull request file set. - Record how
findings relates to inline threads, review-body findings, and any suppressed block. - Repeat after one fix push to verify the marker describes the current round rather than an earlier round.
- Capture any alternate output shape verbatim in fixtures before changing the parser.
Implementation Scope
- Add corpus-derived fixtures for every observed marker shape.
- Fail closed on duplicate, malformed, impossible, or conflicting markers.
- Decide whether
findings=N can be reconciled reliably with GitHub's review objects. - If it can, report the declared and observed counts in
status and block on disagreement. - If it cannot, remove
findings from the contract rather than retaining a field that appears enforced but is ignored. - Confirm that the no-suppression instruction prevents hidden findings. Keep the existing suppressed-block reader as backward-compatible defense.
- Update the
code-review skill, Copilot bootstrap, parser help, and tests together when the measured format differs.
Acceptance Criteria
- Evidence links identify the live review rounds used for validation.
- Tests reproduce the clean, finding, and post-fix rounds from sanitized review bodies.
- The documented marker exactly matches the parser's accepted shape.
- Every retained marker field affects a reported or blocking decision.
- Missing or malformed structured coverage cannot exit successfully.
- Legacy pre-marker coverage bodies remain readable until an explicit retirement decision is recorded.
Relationship to Existing Issues
#788 identified the unstated-coverage ambiguity. #793 introduces the structured marker and exit 45. This issue validates the external reviewer behavior that neither local unit tests nor CI can exercise.
Summary
Validate the structured Copilot review marker introduced by #793 against live GitHub reviews, then make
pr_review.pyenforce every field that proves useful.Context
#793 instructs Copilot code review to end each review body with:
scripts/pr_review.pyreadsreviewedandchangedas a stable alternative to Copilot's changing coverage prose. It retains the legacy prose readers for reviews created before the marker ships. An absent coverage statement blocks with exit 45.The implementation deliberately does not treat one local parser test as evidence that GitHub-hosted Copilot follows the instruction. It also parses the
findingsfield as part of the marker shape but does not yet compare that value with inline or body-only findings.Measurement
After #793 is available on a pull request head:
reviewedandchangedmatch the pull request file set.findingsrelates to inline threads, review-body findings, and any suppressed block.Implementation Scope
findings=Ncan be reconciled reliably with GitHub's review objects.statusand block on disagreement.findingsfrom the contract rather than retaining a field that appears enforced but is ignored.code-reviewskill, Copilot bootstrap, parser help, and tests together when the measured format differs.Acceptance Criteria
Relationship to Existing Issues
#788 identified the unstated-coverage ambiguity. #793 introduces the structured marker and exit 45. This issue validates the external reviewer behavior that neither local unit tests nor CI can exercise.