Repository navigation
Read an Oversized Prose Coverage Count as Unknown in the Copilot Adapter - #3419
Merged
Merged
Conversation
The prose path truncated each count to COUNT_DIGITS before comparing, so a five-digit count read as full whenever its first four digits matched. Both the marker and prose paths now share count_state, which reads a count longer than COUNT_DIGITS as unknown.
Owner
Author
|
A recorded local strict-review pass covers head |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified, and all approval-readiness assessments support approval.
No open issues 🎉
What changed in this PR
This pull request fixes Copilot prose coverage parsing so oversized reviewed and changed-file counts are classified as unknown instead of being truncated and incorrectly accepted as complete coverage.
Changes:
- Adds shared coverage-count validation for marker and prose parsing.
- Rejects oversized counts and cases where reviewed files exceed changed files.
- Adds regression tests for oversized full and partial prose counts.
| File | Description |
|---|---|
tests/test_pr_review_copilot.py |
Adds regression cases verifying oversized prose counts produce unknown coverage and the expected note. |
scripts/pr_review_copilot.py |
Centralizes coverage-state calculation and applies it to both parsing paths. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3419 +/- ##
========================================
Coverage 80.78% 80.78%
========================================
Files 23 23
Lines 8848 8848
========================================
Hits 7148 7148
Misses 1700 1700
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:
|
This was referenced Oct 24, 2026
ptr727
added a commit
that referenced
this pull request
Oct 24, 2026
) ## Summary Fixes the two findings the fourth Copilot round on the develop -> main promotion #3417 raised. - **An oversized finding count reads unknown.** `stated()` in `scripts/pr_review_copilot.py` read a section count longer than `COUNT_DIGITS` as zero. A body saying `99999 open findings` therefore read as a clean round. - `believed(digits)` is the one length check, used by `count_state` and `stated`. That completes the class #3419 began. - `stated` returns `None` for such a count, and the reader notes "a finding count this reader cannot believe". - `Section.findings` is `int | None`. `total(counts)` is the one None-aware sum, used by `Reading.held`, by the open count in `_read`, and by `collected()` in `scripts/pr_review.py`. - An unbelievable open count reads `overview=?`, which leaves Merge Gate item 3 unsatisfied. A flagging round stating only such a count exits `50`. Any `suppressed=`, `previously_missed=`, or `cr_outside_diff=` tally holding one prints `?`. - The usage text and `scripts/README.md` describe the `?` reading. - **The gates-and-tests row in `docs/host-setup.md`** says `uv` installs `tests/requirements.txt` for the tests, rather than that there are no packages to install. ## Tests - `test_an_oversized_count_reads_as_none_and_is_noted` covers an open section labeled `99999 open findings`, the same count as a bold line, and a believable section beside an oversized line. Each expects `open is None` and the note. On develop's code all three fail, reading `0` or `1`. - `test_an_unbelievable_section_count_is_none_beside_a_believed_open_count` covers suppressed and previously-missed sections. - `test_an_unbelievable_previously_missed_count_tallies_as_unknown` asserts `previously_missed=? (on_head=0 earlier=?)` and exit `0`. - `test_an_unbelievable_open_count_reads_unknown_and_holds_the_round` asserts `overview=?/0`, the note, and exit `50`. ## Local review Four recorded passes at the `frontier` tier. - The first raised three, fixed in 8d4ad75. The suppressed and previously-missed tallies still read the count as zero. The length check lived in two places, and two docstrings were stale. - The second raised five. Three are fixed in 3f766ca. The usage text and README did not describe `?`. The None-aware sum lived twice, and the new digest test asserted no exit code. - Two from the second are declined. `counts["threaded"]` stays `self.open or 0`, since its one consumer reads 0 and None alike and fails closed. The `VERDICT WITH NO COUNTED FINDING` text reads "counts no finding" for a round stating only an unbelievable count. That fails closed too, with `overview=?` and the note printed beside it. - The third raised three, fixed in 743010e once the edit budget was spent. The open count kept its own None flag beside `total`. Two sentences the change extended ran past 25 words. - The fourth covers the head. Its findings are against text the previous fixes wrote, so editing stops here and each is listed below. ## Open findings from the last pass - `scripts/README.md:229` and `scripts/pr_review.py:90`, `introduced`: in "A count it cannot believe reads `?` too.", "it" can be read as the round rather than the reader. Declined as `style`. The paragraph defines `?` for "a count the reader cannot believe" earlier, and the field gates nothing. - `scripts/pr_review.py:91` and `scripts/pr_review_copilot.py:110`, `style`: the rewrap left one 131-column usage line and one short docstring line. Declined, since ruff does not select E501 here and the base already holds such lines. ## Pre-existing findings, not filed - `scripts/pr_review.py` digest notes, `pre-existing`: `cr_outside_diff=?` prints with no note naming the section, because the digest collects reader notes only from Copilot's rounds. The base read the same count as 0. ## Verification - `uv run --no-project --with-requirements tests/requirements.txt python -m unittest discover -s tests` -> `Ran 2864 tests`, `OK (skipped=7)`. - `uvx ruff@latest check`, `uvx ruff@latest format --check`, and `uvx mypy@latest` -> clean. - `test_an_oversized_count_reads_as_none_and_is_noted` against develop's adapter -> `FAILED (failures=3)`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
ptr727
added a commit
that referenced
this pull request
Oct 24, 2026
…#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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the finding the second Copilot round on the develop -> main promotion #3417 raised against
scripts/pr_review_copilot.py.The prose coverage path in
_readcut each captured count toCOUNT_DIGITScharacters before comparing. A body sayingCopilot reviewed 12345 out of 12345 changed files.therefore read asfull. So did12340 out of 12349, a partial review, which satisfied Merge Gate item 2 on a false reading. The marker path already read an over-long count asunknown.count_state(got, of)reads a reviewed-of-changed pair asfull,partial, orunknown. It returnsunknownwhere either count is longer thanCOUNT_DIGITSorgotexceedsof. The marker path and the prose path both call it. The prose path keeps its "cannot believe" note onunknown.test_a_count_stated_in_a_paragraph_of_its_own_is_readgains both five-digit rows, each expectingunknownwith the note. On develop's code both rows fail with'unknown' != 'full'.Local review
One recorded pass at the
frontiertier over the head found nothing. It checked thatint()on the\d+captures cannot raise, and that no other helper compares a reviewed count with a changed count.Verification
uv run --no-project --with-requirements tests/requirements.txt python -m unittest discover -s tests->Ran 2856 tests,OK (skipped=7).scripts/pr_review_copilot.py->FAILED (failures=2), both new rows readingfull.uvx ruff@latest check,uvx ruff@latest format --check, anduvx mypy@latest-> clean.🤖 Generated with Claude Code