diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ad2d4323..6ae5c397 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -168,7 +168,7 @@ Copilot reviewed 2 out of 3 changed files in this pull request and generated no The sentence tail after the first spelling reports how many comments the round raised and appears in four wordings. It is not coverage, so it is not part of what has to be recognized, and the counts are. Read them into three verdicts and two exemptions: - **Counts equal** - the round read the whole diff. This is the clean pass. -- **Counts unequal** - files in the diff have no review at all. Do **not** treat a re-request as the remedy: measured over four pull requests and seven rounds on this repository, every partial round stayed partial at the identical ratio and no round ever recovered, so re-requesting spends a round and changes nothing. Splitting works where it applies and does not apply to a promotion, whose head is `develop`. The reviewer names no file list in these rounds, so which file went unread cannot be read from the API either. Report the state and hand the merge decision to the maintainer. +- **Counts unequal** - files in the diff have no review at all. Do **not** treat a re-request as the remedy: measured over four pull requests and seven rounds on this repository, every partial round stayed partial at the identical ratio and no round ever recovered, so re-requesting spends a round and changes nothing. Splitting works where it applies and does not apply to a promotion, whose head is `develop`. **The file table in the body does not tell you which file went unread**, and it looks as though it should, which is why it is written down here: measured over 348 review bodies on this repository and 121 on another in this fleet, that table names the whole changed set on partial and fully covered rounds alike, so a table naming every changed file is what a full round carries too and contradicts nothing. One round of the seven is the exception, stating 16 of 17 and naming 16, omitting `GOVERNANCE.md`, and `status` names an omitted file only in that shape, where the table is short by exactly what the counts leave unread and names nothing outside the diff. Treat that as a lead to check rather than a verdict, one round here naming `GOVENANCE.md`, a path no diff carries. Report the state and hand the merge decision to the maintainer. - **Coverage-shaped and unreadable** - the remedy is to fix the reader, not to read past it. The vetted spellings live in `scripts/pr_review.py` and here, and they stay in step because a case reads them out of this file. - **Exempt: a body stating no coverage at all.** 28 of those 332 bodies are an overview and a change list and nothing more. That shape is current, interleaves with the counted one throughout, and one pull request carries both across its two rounds, so treating it as a failure cries wolf on about one review in twelve and teaches an agent to work around the gate. It reads as `coverage=unstated`, never as a pass and never as a failure. - **Exempt: a refusal.** It carries no coverage line by design, and the refusal rule above has already classified it. Read it here as well and every refusal grows a spurious second failure on top of the one that names its remedy. diff --git a/TODO.md b/TODO.md index 76ab186d..992b904b 100644 --- a/TODO.md +++ b/TODO.md @@ -313,10 +313,13 @@ One pull request, after a survey, deciding whether anything stands between this - **Find out which file a partial round skips, and why re-requesting never clears it.** The coverage reading shipped in #608 blocks on a partial round, and the record says the state is durable rather than transient. - **Blocked by** - Nothing, though it is research rather than a change, and the reader already reports the state correctly. - - **Issue** - None filed. The reading that surfaces it shipped under [#607][issue-607]. - - **Checked** - `develop` at `fa1ebf1` on 2026-08-08, measured over the 332 Copilot review bodies on the newest 120 pull requests, read with `gh pr list --json number,changedFiles,additions,deletions` beside them. - - **Open** - Which file is skipped. The reviewer names no file list in these rounds, so it cannot be recovered from the API, and the GitHub pull request page is the only place it may appear. - - **Open** - Whether a partial round is worth escalating to GitHub at all, which needs the file first. + - **Issue** - [#623][issue-623], filed from a downstream repository against the `PARTIAL` caveat's claim that the reviewer names no file list. The reading that surfaces the state shipped under [#607][issue-607]. + - **Checked** - `develop` at `674a27a` on 2026-08-08, measured over 348 Copilot review bodies on the newest 120 pull requests here and 121 on the fleet's Blog repository, each read against the pull request's own changed-file list rather than against its counts alone. + - **Settled** - The reviewer does name a file list, and the caveat saying otherwise was wrong. It is a `| File | Description |` table carried by 91 of the 348 bodies, and every table row in the corpus belongs to one of those tables. + - **Settled** - The table names the unread file on exactly one round of the seven, which states 16 of 17 and names 16, omitting `GOVERNANCE.md`. That round is also the only evidence on record that the unread file is a real file rather than an artifact of counting. + - **Settled** - It cannot be read as coverage anywhere else. It names the whole changed set on partial and fully covered rounds alike, including all seven partials on Blog, while one round here states 61 of 62 and names 50, another states 33 of 33 and names 32, and a third names `GOVENANCE.md`, a path no diff carries. A reading identical under both outcomes discriminates neither. + - **Settled** - Three of the four partials here carry their table on the round before a push, describing the diff that push replaced, so the comparison is head-scoped like the counts and reports no table rather than a stale list of unreviewed files. + - **Open** - Whether a partial round is worth escalating to GitHub at all. One named file on one round is a starting point rather than the pattern an escalation needs. - **Settled** - It is durable rather than flaky. Four pull requests and seven rounds (#476, #479, #592, and the #609 promotion), and **every later round repeated the identical ratio**. A re-request has never cleared one, so the remedy the digest first stated was wrong and now says so. - **Settled** - Size does not predict it. The partials changed 502, 629 and 961 lines, while fully covered pull requests here reach 33 files and 2,219 lines. - **Settled** - The reviewer counts the file and does not read it, rather than losing it earlier. The stated denominator equals the API's own `changedFiles` on **103 of 104** pull requests, the exception being one whose branch shrank between rounds. @@ -476,6 +479,7 @@ Each was checked against the tree and has nothing left to do anywhere. Closing i [issue-585]: https://github.com/ptr727/ProjectTemplate/issues/585 [issue-597]: https://github.com/ptr727/ProjectTemplate/issues/597 [issue-607]: https://github.com/ptr727/ProjectTemplate/issues/607 +[issue-623]: https://github.com/ptr727/ProjectTemplate/issues/623 diff --git a/scripts/README.md b/scripts/README.md index b0315e89..aa4a62b2 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -124,7 +124,7 @@ The reading fails closed, so a coverage-shaped line that parses to no counts is The line is matched at its **start** rather than anywhere in the body, since both spellings are structural: across those bodies every coverage statement opens its line, 272 with the reviewer's own name and 32 as the `Review details` bullet, and none sits mid-sentence. A body-wide match reports the pull request that adds this check as a partial round, which is the false positive the suppressed matcher and the refusal matcher have each had once already, and fenced blocks are dropped for the same reason, 131 of the bodies carrying one and this change putting both spellings into the diff a review of it quotes. The cost is named rather than hidden: a wording that moves the statement off the line start reads as no statement rather than as one this cannot parse. The reading is **head-scoped**, unlike a suppressed finding and like a refusal, because a partial round describes one commit's diff and the push that changes that diff raises a round reading the whole of the new one, and where one head carries two rounds through a re-request the worst of them reports, since the one naming files it did not read is the one to answer. A case reads the vetted spellings out of the runbook and hands them to this script's own parser, so the pair stays in step in both directions. -A partial round is reported and handed over rather than retried into. Measured over four pull requests and seven rounds, **every partial stayed partial at the identical ratio and no round ever recovered**, and the diff size does not predict it: fully covered pull requests here run to 33 files and 2,219 changed lines, well past every partial. The stated denominator matches the API's own changed-file count on 103 of 104 pull requests, so the reviewer counts the file and does not read it, and it names no file list in these rounds, so which file went unread cannot be recovered from the API. `wait` and `status` therefore report `42` as a state for the maintainer rather than one to re-request into. +A partial round is reported and handed over rather than retried into. Measured over four pull requests and seven rounds, **every partial stayed partial at the identical ratio and no round ever recovered**, and the diff size does not predict it: fully covered pull requests here run to 33 files and 2,219 changed lines, well past every partial. The stated denominator matches the API's own changed-file count on 103 of 104 pull requests, so the reviewer counts the file and does not read it. Which file that is usually cannot be recovered, and the file table many bodies carry is why it is worth saying "usually" rather than "never": measured over 348 review bodies here and 121 on another repository in this fleet, that table names the whole changed set on partial and fully covered rounds alike, so a table naming every file is exactly what a fully covered round carries too and separates a miscount from a file nobody read exactly never. **One round of the seven is the exception**, stating 16 of 17 and naming 16, omitting `GOVERNANCE.md`, and it is the only evidence on record that the unread file is a real file rather than an artifact of counting. `status` therefore prints what the table says beside the counts and takes no verdict from it. It names the omitted file only where the table is short by exactly what the counts leave unread **and** names nothing outside the diff, one round here naming `GOVENANCE.md`, a path no diff carries, which is enough of a reminder that the table is prose the reviewer writes rather than a list from the API. The comparison is head-scoped like the counts are, so a table from before a push describes the diff that push replaced and reports as no table rather than as a list of unreviewed files, which is the shape three of the four partials here carry. `wait` and `status` therefore report `42` as a state for the maintainer rather than one to re-request into. `status` and `wait` both exit `43` where the reviewer sent a **shape this script has no reader for**, and that outcome outranks every other reading here, because a reader that does not understand the output cannot be believed about what it found in it. Every rule in this script keys on a structural marker, so a marker that changes spelling is a section the reader stops finding and reports as absent. All three failures on record have that exact shape: the suppressed heading reworded and the count went to zero, the suppressed section moved inside another wrapper and the count went to zero again, and the coverage line was never parsed at all. Each reported a clean pass over a review it had misread, and each was found by the maintainer afterwards rather than by the gate. The digest carries `shapes=UNRECOGNIZED`, lists each shape it could not place, and names the remedy in two parts: **file an issue on the hub repository that hosts this script**, quoting the body the shape came from, and leave the merge decision to the maintainer, since an unrecognized shape does not say the pull request is bad, only that nothing here can vouch for the review of it. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 1c1d5ece..aad86bb7 100644 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -140,6 +140,15 @@ # This change puts both spellings into the source and the runbook, so a review of it quotes them. # A quoted count read as this round's own is a coverage figure nobody stated. FENCE = re.compile(r'^ {0,3}```.*?^ {0,3}```[^\n]*', re.DOTALL | re.MULTILINE) +# The round's own file summary table, whose first column is a path and whose second is prose. +# The header row is the marker rather than the `
` around it. +# One measured round carries the table with no ``, and two summary spellings wrap it. +# Over the same 348 bodies, 91 carry a table and every table row in the corpus belongs to one. +# A row outside this shape is a shape nothing has seen rather than a table read wrongly. +TABLE_HEADER = re.compile(r'\s*\|\s*File\s*\|', re.IGNORECASE) +# The alignment row under the header, which is punctuation rather than a file. +TABLE_RULE = re.compile(r'\s*\|[\s:|-]+\|\s*$') +TABLE_ROW = re.compile(r'\s*\|([^|]*)\|') # The readings a round's coverage carries, worst first. # A head carries more than one round only through a re-request. # Where two disagree, the one naming files it did not read is the one to answer. @@ -198,6 +207,13 @@ # A test holds this equal to the number the queries carry, since a drift between them reads clean. WINDOW = 100 +# How many of the pull request's own changed files the full query asks for. +# 100 is the connection's own ceiling, so a longer diff is truncated rather than paged. +# A truncated list is reported as one and compared against nothing. +# A path missing from a short window reads exactly like a path the reviewer left out. +# The record holds a pull request of 301 changed files, so the truncation is reachable here. +FILES_WINDOW = 100 + # How many rollup contexts the full query asks for, which is not the window above. # The query is built from this and the truncation line quotes it, so the two cannot drift. # Naming it beside a hard-coded literal only documented the literal, which a case then held. @@ -243,6 +259,7 @@ comments(first:1){ nodes{ author{login} path line body } } }} comments(last:100){ nodes{ author{login} createdAt body } pageInfo{ hasPreviousPage } } reviewRequests(first:10){ nodes{ requestedReviewer{ __typename ... on Bot{login} ... on User{login} } } } + files(first:__FILES_WINDOW__){ pageInfo{ hasNextPage } nodes{ path } } commits(last:1){ nodes{ commit{ oid statusCheckRollup{ state contexts(first:__CHECKS_WINDOW__){ pageInfo{ hasNextPage } nodes{ __typename @@ -250,7 +267,7 @@ ... on StatusContext{ context state createdAt } }}}}}} }}} -""".replace('__CHECKS_WINDOW__', str(CHECKS_WINDOW)) +""".replace('__CHECKS_WINDOW__', str(CHECKS_WINDOW)).replace('__FILES_WINDOW__', str(FILES_WINDOW)) # Substituted rather than interpolated, because GraphQL is braces from end to end. # An f-string would need every one of them doubled, which is unreadable against the schema. @@ -577,6 +594,108 @@ def head_coverage(pr: dict) -> tuple[str, str]: return worst, detail +def file_table(body: str) -> list[str]: + """The paths the round's own file summary table names, in the order it names them. + + Quotations are dropped for the reason a coverage line's are: this change puts a table into + the diff and a review of it quotes one, and a quoted table read as this round's own names + files nobody reviewed. + + The header is what opens the table and any line that is not a row closes it, so a second + table later in the body is read as a second table rather than as more of the first. + """ + paths, reading = [], False + for line in FENCE.sub('', body or '').splitlines(): + if TABLE_HEADER.match(line): + reading = True + elif not TABLE_ROW.match(line): + reading = False + elif reading and not TABLE_RULE.match(line): + cell = TABLE_ROW.match(line).group(1) + paths.append(cell.strip().strip('`').strip()) + return [p for p in paths if p] + + +def changed_paths(pr: dict) -> tuple[list[str], bool]: + """The paths this pull request changes, and whether the query's window cut the list short. + + Truncation is carried rather than hidden, since a path outside a short window reads exactly + like a path the reviewer left out of its table, and the two take opposite readings. + """ + files = pr.get('files') or {} + paths = [n.get('path') or '' for n in (files.get('nodes') or [])] + return [p for p in paths if p], bool((files.get('pageInfo') or {}).get('hasNextPage')) + + +def head_table(pr: dict) -> list[str]: + """The file table the rounds covering the current head carry, from the first that has one. + + Any round on the head rather than the one whose counts decided the verdict, since rounds on + one commit describe one diff, and a re-requested round states the counts again and carries no + table at all. Thirteen commits here carry more than one round and one of those pairs is that + shape exactly, so which of the two the verdict happens to read must not decide whether a + table is found. + + Head-scoped for the reason the coverage line is, and this is the tighter half of the reading: + three of this repository's four partial pull requests carry their table on the round before a + push, describing a diff that is not the one being merged, and comparing that table against + the current changed files would name files as unreviewed on the strength of a stale list. + Those report as no table rather than as a comparison, which is the honest answer. + """ + for node in head_reviews(pr): + named = file_table(node.get('body') or '') + if named: + return named + return [] + + +def table_against_diff(pr: dict, counts: tuple[int, int] | None) -> str: + """What the reviewer's own file table says about the files the counts leave unread. + + This reports and decides nothing. The exit code stays with the counts, because the table + tracks them nowhere near well enough to overturn one, and the measurement is what says so. + Over 348 review bodies on this repository and 121 on ptr727/Blog, the table names the whole + changed set on partial and fully covered rounds alike: all seven partials on Blog name every + changed file, as do two of the four here that carry a table. A reading identical under both + outcomes discriminates neither, which is why a full table is reported as corroborating + nothing rather than as the count being a miscount. + + The one arm that locates anything is a table shorter by exactly what the counts say went + unread. #479 states 16 of 17 and names 16, omitting `GOVERNANCE.md`, and it is the only + evidence on record that the unread file is a real file rather than an artifact of counting. + It stays a lead rather than a verdict, since the table is prose the reviewer writes: #609 + states 61 of 62 and names 50, and #606 names `GOVENANCE.md`, a path no diff here carries. + + A path named that the diff does not carry is what disqualifies the naming arm, that typo + being enough to drop a real file into the omissions and read it as the one nobody reviewed. + """ + named = head_table(pr) + if not named: + return 'no round covering this head carries a file table, so none of them locates the file' + changed, truncated = changed_paths(pr) + if truncated or not changed: + return (f'the reviewer names {len(named)} files in its own table and the diff could not ' + f'be read back to compare them, the changed-file list being ' + f'{"longer than the window this reads" if truncated else "absent from the query"}') + omitted = [p for p in changed if p not in named] + invented = [p for p in named if p not in changed] + short = 0 if counts is None else counts[1] - counts[0] + if not omitted: + return (f'the reviewer\'s own file table names all {len(changed)} changed files, which it ' + f'also does on rounds stating full coverage, so it corroborates nothing and ' + f'names no unread file') + if len(omitted) == short and not invented: + return (f'the reviewer\'s own file table omits exactly the {short} file' + f'{"" if short == 1 else "s"} the counts leave unread, naming ' + f'{", ".join(omitted)}. The table is prose the reviewer writes rather than a ' + f'list from the API, so that is a lead to check rather than a verdict') + return (f'the reviewer\'s own file table names {len(named)} of the {len(changed)} changed ' + f'files, omitting {len(omitted)} where the counts leave {short} unread' + + (f' and naming {", ".join(invented)}, which the diff does not carry' if invented + else '') + + ', so it tracks the counts nowhere and names no unread file') + + def normal(text: str) -> str: """A marker reduced to what a vetted list compares: ASCII, single spaces, counts as `(N)`. @@ -678,10 +797,10 @@ def report_verdict(pr: dict) -> int: print(f'status=COVERAGE_IS_PARTIAL the review covering the head read fewer files than the ' f'pull request changed, so part of the diff has no review at all. Every partial on ' f'record stayed partial at the identical ratio across every later round, so a ' - f're-request is not the remedy it reads as, and the reviewer names no file list in ' - f'these rounds, so which file went unread cannot be read from the API. Splitting is ' + f're-request is not the remedy it reads as. Splitting is ' f'the remedy where it applies, and it does not apply to a promotion. Otherwise this ' - f'is the maintainer\'s call, taken knowing {gap}') + f'is the maintainer\'s call, taken knowing {gap}, and knowing that ' + f'{table_against_diff(pr, counts)}') return 42 return 0 @@ -1033,6 +1152,11 @@ def digest(owner: str, repo: str, num: int, seen: set[str] | None = None, 're-request has never cleared this on record, so report it rather than ' 'retrying into it, and let the maintainer take the merge decision') lines.append(f' {cover_line}') + # The counts decide the state and this line says what the reviewer's file table adds. + # Printed under the same marker, since a maintainer taking the decision wants both. + # It names no verdict of its own, having been measured against the counts it sits under. + # The table tracks them nowhere, which is why it is reported rather than read. + lines.append(f' {table_against_diff(pr, read_coverage(cover_line))}') for node, shape in stuck: # Each shape carries its own remedy, which is the whole point of telling them apart. # A reader handed one word for all three retries the wrong thing, or waits on a queue. diff --git a/scripts/test_pr_review.py b/scripts/test_pr_review.py index 62b049ec..1c5217a0 100644 --- a/scripts/test_pr_review.py +++ b/scripts/test_pr_review.py @@ -81,6 +81,17 @@ def nested(heading: str = '### Suppressed comments (2)', '- **Review effort level:** Lite\n
\n') +def summarized(paths: list[str], covers: str = COVERED) -> str: + """A round carrying its coverage line and the file table naming `paths`. + + The table is quoted from the corpus in shape: a header, an alignment row, and one row per + file whose second cell is prose about the change. + """ + rows = '\n'.join(f'| {p} | Prose about the change. |' for p in paths) + return (f'{OVERVIEW}\n{covers}\n\n
\nShow a summary per file\n\n' + f'| File | Description |\n| ---- | ----------- |\n{rows}\n\n
\n') + + REFUSED = ("Copilot wasn't able to review this pull request because it exceeds the maximum " 'number of files (300). Try reducing the number of changed files and requesting a ' 'review from Copilot again.') @@ -136,10 +147,15 @@ def status_context(context: str = 'ci/external', state: str = 'SUCCESS', def payload(reviews: list[dict], threads: list[dict] | None = None, merge: str = 'CLEAN', comments: list[dict] | None = None, older: bool = False, older_reviews: bool = False, pending: bool = False, - checks: list[dict] | None = None, rollup_oid: str | None = None) -> dict: + checks: list[dict] | None = None, rollup_oid: str | None = None, + files: list[str] | None = None, more_files: bool = False) -> dict: requested = ([{'requestedReviewer': {'__typename': 'Bot', 'login': pr_review.REVIEWER}}] if pending else []) return {'headRefOid': HEAD, 'mergeable': 'MERGEABLE', 'mergeStateStatus': merge, + # The diff's own file list, which the round's file table is compared against. + # A case naming none gets the empty connection a pull request of no files carries. + 'files': {'nodes': [{'path': p} for p in files or []], + 'pageInfo': {'hasNextPage': more_files}}, 'reviews': {'nodes': reviews, 'pageInfo': {'hasPreviousPage': older_reviews}}, 'reviewThreads': {'nodes': threads or []}, 'comments': {'nodes': comments or [], 'pageInfo': {'hasPreviousPage': older}}, @@ -1025,6 +1041,134 @@ def test_a_refusal_still_exits_forty_one_rather_than_on_a_coverage_code(self) -> self.assertEqual(41, pr_review.main(['wait', '7', '--repo', 'o/r', '--timeout', '0'])) +class TestTheRoundsOwnFileTable(GqlCase): + """The file table a partial round carries, read against the diff it claims to describe. + + The reading reports and decides nothing, because the measurement says it cannot decide + anything. Over 348 review bodies here and 121 on ptr727/Blog, the table names the whole + changed set on partial and fully covered rounds alike, and every case below is one of the + shapes that corpus carries rather than one invented for the reader. + """ + + PART = ('Copilot reviewed 2 out of 3 changed files in this pull request and generated ' + 'no comments.') + + def reading(self, body: str, files: list[str], more: bool = False) -> str: + counts = pr_review.read_coverage(self.PART) if self.PART in body else None + return pr_review.table_against_diff( + payload([review(body=body)], files=files, more_files=more), counts) + + def test_the_table_reads_as_its_paths_rather_than_as_its_rows(self) -> None: + """The header and the alignment row are punctuation, and the second cell is prose.""" + self.assertEqual(['a.py', 'docs/b.md'], + pr_review.file_table(summarized(['a.py', 'docs/b.md']))) + + def test_a_quoted_table_is_not_this_rounds_own(self) -> None: + """The reason a quoted coverage line is not: this change puts a table in the diff. + + A review of it quotes one, and a quoted table read as the round's own names files + nobody reviewed. + """ + quoted = f'{OVERVIEW}\n```\n| File | Description |\n| ---- | ---- |\n| a.py | Prose. |\n```' + self.assertEqual([], pr_review.file_table(quoted)) + + def test_a_table_naming_every_changed_file_corroborates_nothing(self) -> None: + """The reporter's case, and the one the measurement answers. + + All seven partial rounds on ptr727/Blog name every changed file, as do #476 and #592 + here. So does a round stating full coverage, which is why a full table cannot separate a + miscount from a file that went unread. + """ + body = summarized(['a.py', 'b.md', 'c.yml'], covers=self.PART) + self.assertIn('corroborates nothing', + self.reading(body, ['a.py', 'b.md', 'c.yml'])) + + def test_a_table_short_by_what_the_counts_leave_unread_names_the_file(self) -> None: + """#479 states 16 of 17 and names 16, omitting `GOVERNANCE.md`. + + It is the only round on record whose table locates the unread file, and it is reported + as a lead rather than as a verdict, the table being prose the reviewer writes. + """ + out = self.reading(summarized(['a.py', 'b.md'], covers=self.PART), ['a.py', 'b.md', 'c.yml']) + self.assertIn('omits exactly the 1 file', out) + self.assertIn('c.yml', out) + self.assertIn('lead to check rather than a verdict', out) + + def test_a_path_the_diff_does_not_carry_disqualifies_the_naming(self) -> None: + """#606 names `GOVENANCE.md`, which no diff here carries. + + A misspelled path drops the real file into the omissions, where the arm above would read + it as the one nobody reviewed, so a table naming anything outside the diff names nothing. + """ + out = self.reading(summarized(['a.py', 'b.mb'], covers=self.PART), ['a.py', 'b.md', 'c.yml']) + self.assertIn('b.mb', out) + self.assertIn('names no unread file', out) + self.assertNotIn('omits exactly', out) + + def test_a_table_short_by_more_than_the_counts_tracks_neither(self) -> None: + """#609 states 61 of 62 and names 50, so the shortfalls disagree by eleven files.""" + out = self.reading(summarized(['a.py'], covers=self.PART), ['a.py', 'b.md', 'c.yml']) + self.assertIn('names 1 of the 3 changed files', out) + self.assertIn('names no unread file', out) + + def test_a_changed_file_list_the_window_cut_short_is_not_compared(self) -> None: + """A path outside the window reads exactly like a path the reviewer left out. + + The record holds a pull request of 301 changed files, so this is reachable rather than + theoretical, and reading it would name every file past the window as unreviewed. + """ + out = self.reading(summarized(['a.py'], covers=self.PART), ['a.py'], more=True) + self.assertIn('longer than the window this reads', out) + self.assertNotIn('corroborates nothing', out) + + def test_a_head_carrying_no_table_at_all_says_so_rather_than_staying_silent(self) -> None: + """A silent field reads as a comparison that ran and found nothing to report.""" + self.assertIn('no round covering this head carries a file table', + self.reading(OVERVIEW + '\n' + self.PART, ['a.py', 'b.md', 'c.yml'])) + + def test_the_table_is_read_from_any_round_on_the_head_rather_than_the_deciding_one(self) -> None: + """A re-request restates the counts and carries no table, and #474 is that pair. + + Thirteen commits here carry more than one round, and on one of them a round with a table + sits beside a round without, so which of the two the verdict reads must not decide + whether a table is found. Both reviewed the same commit, so both describe the same diff. + """ + again = ('Copilot reviewed 2 out of 3 changed files in this pull request and generated ' + 'no new comments.') + pr = payload([review(body=summarized(['a.py', 'b.md', 'c.yml'], covers=self.PART), + at=EARLY), + review(body=OVERVIEW + '\n' + again, at=LATE)], + files=['a.py', 'b.md', 'c.yml']) + self.assertIn('corroborates nothing', + pr_review.table_against_diff(pr, pr_review.read_coverage(again))) + + def test_a_table_from_before_a_push_describes_another_diff_and_is_not_read(self) -> None: + """The tighter half, and the shape three of this repository's four partials carry. + + The round naming files sits before a push and describes the diff that push replaced, so + comparing it against the current changed files names a file as unreviewed on a stale + list. Reporting no table is the honest answer where the only table is that one. + """ + again = ('Copilot reviewed 2 out of 3 changed files in this pull request and generated ' + 'no new comments.') + pr = payload([review(oid=OLD, body=summarized(['a.py'], covers=self.PART)), + review(body=OVERVIEW + '\n' + again)], + files=['a.py', 'b.md', 'c.yml']) + self.assertIn('no round covering this head carries a file table', + pr_review.table_against_diff(pr, pr_review.read_coverage(again))) + + def test_the_table_moves_no_exit_code_and_prints_under_the_partial_marker(self) -> None: + """The counts decide the state, and the table is what the maintainer decides beside it.""" + self.answer(payload([review(body=summarized(['a.py', 'b.md', 'c.yml'], covers=self.PART))], + files=['a.py', 'b.md', 'c.yml'])) + with contextlib.redirect_stdout(io.StringIO()) as out: + self.assertEqual(42, pr_review.main(['status', '7', '--repo', 'o/r'])) + printed = out.getvalue() + self.assertIn('COVERAGE IS PARTIAL', printed) + self.assertIn('corroborates nothing', printed) + self.assertIn('status=COVERAGE_IS_PARTIAL', printed) + + class TestDigestReportsTheAnswer(GqlCase): def test_the_comment_prints_whole_under_a_marker_naming_it_terminal(self) -> None: """Its wording is what separates a refusal from a remark, so it is not truncated."""