diff --git a/scripts/pr_review.py b/scripts/pr_review.py index a2d5ae1a..c79538ac 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -433,8 +433,12 @@ # The second overview format's own section for a finding against code the pull request did not change. # Anchored to the opening of its line, past whatever markup a heading wears, rather than matched anywhere in it. # The wording is ordinary English, so a body-wide match reads a sentence such as `restores a guard previously missed (2) rounds ago` as this section. -# `marker_blocks` accepts a line on a count alone, which is what lets that sentence through, so the anchor is what separates a heading from prose naming one. +# `marker_blocks` accepts a line that is not a heading on a `(N)` alone, which is what lets that sentence through. +# So the anchor is what separates a heading from prose naming one. # The opener admits heading markup and anything outside ASCII, and nothing else. +# One count may follow it, since a later revision opens the heading on its count, as `2 previously missed issues`. +# A count opening a line is how a sentence opens too, so `marker_blocks` reads such a line as prose unless it is a heading. +# `OPENING_COUNT` reads that count past this same opener, so whatever markup the anchor admits, the prose check strips. # An emoji is what this format already puts on its verdict headings, and a spelling that admitted no such character read a heading wearing one as no section at all. # That failure is silent rather than loud, the heading itself being vetted and `normal` dropping the emoji before the comparison, so the digest reports every shape read while the findings reach nobody. # Widened to any non-word character instead, ordinary punctuation opened the run and a sentence such as `- Previously missed (2) findings were re-raised` became the section, printing the prose around it as the finding. @@ -443,12 +447,14 @@ # Two alternatives able to consume the same character make the run ambiguous, and the parse then doubles per character of it on a line that does not go on to say this, which is a hang rather than a slow read. # A body carrying 30 non-breaking spaces in a line took minutes, and every alternative here now opens on a character the others cannot. # An emphasis marker is admitted only where no space follows it, which is what tells `**Previously missed (1)**` from the list item `* Previously missed (2) findings`. -# A bullet is the one prose opener that is also heading markup, and `marker_blocks` accepts a line on a count alone, so the list item read as the section and printed the prose around it as findings. +# A bullet is the one prose opener that is also heading markup, and `marker_blocks` accepts a line on a `(N)` alone. +# So the list item read as the section and printed the prose around it as findings. # One character per iteration rather than a run of them, since a repeated group that can split a run of stars more than one way is ambiguous and parses exponentially in it. # What this still admits is non-ASCII punctuation, an em dash or a typographic quote opening a sentence, which reads that sentence as the section. # It is narrower than the spelling before it, which admitted ASCII punctuation as well, and the remedy for the rest is a class of non-ASCII symbols rather than the whole of non-ASCII. +HEADING_OPENER = r"(?:]*>|[ \t>#]|[*_](?![ \t])|[^\x00-\x7f])*" PREVIOUSLY_MISSED = re.compile( - r"^(?:]*>|[ \t>#]|[*_](?![ \t])|[^\x00-\x7f])*Previously missed\b", + rf"^{HEADING_OPENER}(?:[0-9]+[ \t]+)?Previously missed\b", re.IGNORECASE, ) # CodeRabbit's own equivalent, collapsed into the review body like `SUPPRESSED` rather than raised as an inline comment. @@ -664,7 +670,6 @@ def strip_fences( # A body is read for these rather than trusted, because every reader below keys on one of them. # A heading this script has no spelling for is a section it will not find, reported as absent. # That is the shape of all three failures already on record here, each caught after it landed. -# The lists are small because the output is regular: 10 headings, 13 summaries and 4 labels. # Two overview formats are carried rather than one, the second arriving after that measurement. # Its own markers are listed beside the first's, since a round in either format can land next. # Counts are normalized to `(N)` and non-ASCII is dropped before comparing, and `unvetted` folds letter case at the comparison itself. @@ -693,19 +698,29 @@ def strip_fences( "Open (N)": OPEN, "(N) open findings": TOTAL, "(N) open finding": TOTAL, + "(N) open issues": TOTAL, + "(N) open issue": TOTAL, "Resolved since last review (N)": RESOLVED, "(N) resolved since last review": RESOLVED, } -VETTED_SUMMARIES = set(OVERVIEW_SECTIONS) | { - "Pull request overview", - NARRATIVE_SUMMARY, - "Show a summary per file", - "File summaries", - "Review details", - "Suppressed comments (N)", - "Comments suppressed due to low confidence (N)", +PREVIOUSLY_MISSED_SUMMARIES = { "Previously missed (N)", + "(N) previously missed issues", + "(N) previously missed issue", } +VETTED_SUMMARIES = ( + set(OVERVIEW_SECTIONS) + | { + "Pull request overview", + NARRATIVE_SUMMARY, + "Show a summary per file", + "File summaries", + "Review details", + "Suppressed comments (N)", + "Comments suppressed due to low confidence (N)", + } + | PREVIOUSLY_MISSED_SUMMARIES +) VETTED_LABELS = {"Files reviewed", "Comments generated", "Review effort level", "Review effort"} # Inline emphasis around a marker is presentation rather than identity. # The second overview format wraps every `` in `` where the first wrapped none. @@ -746,7 +761,8 @@ def strip_fences( # The format states a count per severity, so a round stating `2 . 3 ` raised five. CCR_FINDINGS = re.compile(r"^ {0,3}\*\*Findings:\*\*(.*)$", re.MULTILINE) CCR_OPEN_FINDINGS = re.compile( - r"^ {0,3}\*\*(\d+) open findings?\*\*[ \t]*\r?$", re.MULTILINE | re.IGNORECASE + r"^ {0,3}\*\*(\d+|No) open (?:findings?|issues?)\*\*[ \t]*(?:[^\x00-\x7f\s]+[ \t]*)?\r?$", + re.MULTILINE | re.IGNORECASE, ) # The markup the format writes a count beside, replaced by one sentinel before a count is read. # Masked rather than matched around, because a badge carries digits of its own in its attributes. @@ -817,9 +833,16 @@ def strip_fences( SUMMARY = re.compile(r"]*)?>(.*?)", re.DOTALL | re.IGNORECASE) TAGS = re.compile(r"", re.IGNORECASE) COUNT = re.compile(r"\((\d+)\)") +# A count opening a line past any opener `PREVIOUSLY_MISSED` admits, which is how `marker_blocks` tells prose. +OPENING_COUNT = re.compile(rf"{HEADING_OPENER}[0-9]+[ \t]") +# A count opening a heading's text, read once `heading_count` has reduced its badge and emphasis as `normal` does. +# No tag is skipped here, since a finding entry's summary opens on a severity badge rather than on heading markup. +# A badge read as markup made an entry titled `2 low confidence results` state a count, and bound a section there. +LEADING_COUNT = re.compile(r"(?:[ \t\r\n#*_]|[^\x00-\x7f])*([0-9]+)[ \t\r\n]") # What makes a line a heading rather than prose mentioning the phrase, in either markup. -# A line that is neither still qualifies where it carries a count. -# The section has appeared as a bare line too, and a count is what prose does not carry. +# A line that is neither still qualifies where it carries a `(N)` and does not open on a count. +# The section has appeared as a bare line too, and a closing `(N)` is what prose does not carry. +# A count opening the line is not that signal, since a sentence opens on one as readily as a heading does. HEADING = re.compile(r"\s*(?:#{1,6}\s| list[tuple[str, int | None, list[str]]]: role = roles.get(normal(summary).casefold()) if role is None: continue - digits = re.search(r"\d+", EMPHASIS.sub("", summary)) - count = counted(digits.group()) if digits else None + count = heading_count(summary) text = masked[start:end] own = "\n".join(text[a:b] for a, b in details_spans(tags[start:end])[1]) found.append((role, count, [ln for ln in own.splitlines() if ENTRY_LINE.match(ln)])) @@ -2417,8 +2439,9 @@ def stated_total(body: str, preamble_only: bool = False) -> int | None: `MARKUP_MASK`, for the reason `details_tags` gives, which keeps every offset the preamble needs. A later revision of the format drops the `**Findings:**` line and states its total as a bare - `**0 open findings**` line, read here the same way, since left unread it printed `?` and - suppressed the shortfall the total is compared for. + `**0 open findings**` line. That line is read here the same way, since left unread it printed + `?` and suppressed the shortfall the total is compared for. A later one still writes that zero as + `**No open issues**` and an emoji. That is the same stated zero. That revision writes the bare line only for a zero, measured over every body carrying it, and states any other total as the count opening its open-findings section, so where the preamble @@ -2430,7 +2453,10 @@ def stated_total(body: str, preamble_only: bool = False) -> int | None: opener = DETAILS_OPEN.search(CODE_SPAN.sub(MARKUP_MASK, stripped)) preamble = plain[: opener.start()] if opener else plain totals = [findings_on(m.group(1)) for m in CCR_FINDINGS.finditer(preamble)] - totals += [counted(m.group(1)) for m in CCR_OPEN_FINDINGS.finditer(preamble)] + totals += [ + 0 if m.group(1).casefold() == "no" else counted(m.group(1)) + for m in CCR_OPEN_FINDINGS.finditer(preamble) + ] found = [t for t in totals if t is not None] if not found and not preamble_only: found = [c for role, c, *_ in read_overview(body)[2] if role == TOTAL and c is not None] @@ -3647,6 +3673,10 @@ def unrecognized_in(body: str) -> list[str]: quoting a heading is not a review carrying one, and this script's own pull requests quote these lists in full. + A previously-missed summary that no reader finds as a section is reported too. Its spelling is + vetted wherever it appears, while the reader keys on how its line opens. A layout the reader + misses would otherwise hand its findings to nobody under `shapes=ok`. + A body carrying no heading at all is reported rather than passed, since every one of the 333 measured opens on a heading and a body with none is a format nothing here has seen. A refusal is the exemption, being a bare paragraph by design and already classified as one. @@ -3669,6 +3699,13 @@ def unrecognized_in(body: str) -> list[str]: for marker in (normal(s) for s in dict.fromkeys(SUMMARY.findall(plain))) if unvetted(marker, VETTED_SUMMARIES) and not finding_entry(marker) ] + missed = [ + marker + for marker in (normal(s) for s in SUMMARY.findall(plain)) + if not unvetted(marker, PREVIOUSLY_MISSED_SUMMARIES) + ] + if len(missed) > len(previously_missed_blocks(body)): + found.append(f"summary: {missed[0]}, vetted while no reader found its section") found += [ f"metadata label: {la}" for la in dict.fromkeys(labels) if unvetted(la, VETTED_LABELS) ] @@ -4266,6 +4303,25 @@ def heading_of(block: str) -> str: return m.group(1) if m and head.lower().startswith(" int | None: + """The count a heading states, closing it as `(N)` or opening it. + + One reader serves every caller, since the format moved its count to the front of a summary. + A caller reading only one of the two places misses the other. A section bounded on `(N)` alone + ran into a count-first sibling and printed that sibling's findings twice. + + It takes the heading itself rather than the block, and reads all of it. Cut to its first line, + a summary split as `Open` and then `(2)` lost the count its role was vetted with. + + The badge and the emphasis are reduced first, as `normal` reduces them before vetting. The + count is then read from the same text the role was vetted on. A badge reduces to its word, which is + what keeps an entry titled `2 low confidence results` from stating a count. + """ + text = EMPHASIS.sub("", SEVERITY_BADGE.sub(badge_text, heading)) + m = COUNT.search(text) or LEADING_COUNT.match(text) + return counted(m.group(1)) if m else None + + # `
(.*?)
` lazily pairs each open with the *next* close, which is the innermost one once a shape nests, silently losing everything the outer wrapper still carries after it. # CodeRabbit's outside-diff section does exactly that: a file wrapper nested inside the section heading, itself wrapping a per-finding "Prompt for AI Agents" block three levels deep. DETAILS_TAG = re.compile( @@ -4395,8 +4451,13 @@ def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = F """Return the review body's sections whose own heading matches `marker`, each sliced from that heading through to the end of its own region. - Shared by `suppressed_blocks` and `outside_diff_blocks`. `strip_blockquote` affects heading - detection only, never the returned content. + Shared by `suppressed_blocks`, `previously_missed_blocks`, and `outside_diff_blocks`. + `strip_blockquote` affects heading detection only, never the returned content. A summary split + across lines is folded onto its opening line in the returned content too, per `fold_summaries`. + + A line that is not a heading qualifies on a `(N)` it carries, and only where it does not open + on a count. A count opening a line is how a heading states it and also how a sentence opens, + so `2 previously missed (3) rounds ago` is prose, not a section. """ if not body: return [] @@ -4412,15 +4473,53 @@ def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = F # Scanned line by line rather than read from a wrapper's own summary. # A heading can sit in the wrapper's body instead, where reading only the summary would miss it. scan_lines = "".join(masked[a:b] for a, b in spans).splitlines() + scan_lines, raw_lines = fold_summaries(scan_lines, raw_lines) if strip_blockquote: scan_lines = [BLOCKQUOTE.sub("", ln) for ln in scan_lines] for i, line in enumerate(scan_lines): - if marker.search(line) and (HEADING.match(line) or COUNT.search(line)): + bare = COUNT.search(line) and not OPENING_COUNT.match(line) + if marker.search(line) and (HEADING.match(line) or bare): blocks.append("\n".join(raw_lines[i : sibling_heading(scan_lines, i, marker)])) break return blocks +FOLD_OPENER = re.compile(r"[>\s]*]|$)", re.IGNORECASE) +FOLD_STOP = re.compile(r"\s*<(?:details|summary)(?=[\s>/]|$)", re.IGNORECASE) + + +def fold_summaries(scan_lines: list[str], raw_lines: list[str]) -> tuple[list[str], list[str]]: + """Both line lists with each `` split across lines folded onto its opening line. + + A heading is found line by line, so a summary whose text sits on lines of its own read as no + heading at all. `unrecognized_in` vetted its spelling meanwhile and reported nothing. The groups + come from the scanned copy and apply to both, so the two keep addressing the same lines. + + Only a line opening on `" not in scan_lines[i].lower(): + for j in range(i + 1, len(scan_lines)): + bare = BLOCKQUOTE.sub("", scan_lines[j].strip()) + if FOLD_STOP.match(bare) or MARKDOWN_HEADING.match(bare): + break + if "" in bare.lower(): + end = j + break + for source, target in ((scan_lines, scan), (raw_lines, raw)): + parts = [BLOCKQUOTE.sub("", ln.strip()) for ln in source[i + 1 : end + 1]] + target.append(" ".join([source[i].rstrip(), *parts])) + i = end + 1 + return scan, raw + + def sibling_heading(scan_lines: list[str], start: int, marker: re.Pattern[str]) -> int: """Where this block ends, which is the next sibling section's heading or the region's end. @@ -4449,7 +4548,7 @@ def sibling_heading(scan_lines: list[str], start: int, marker: re.Pattern[str]) """ for j in range(start + 1, len(scan_lines)): line = scan_lines[j] - if not (HEADING.match(line) and COUNT.search(line)): + if not (HEADING.match(line) and heading_count(heading_of(line)) is not None): continue if any(o is not marker and o.search(line) for o in SECTION_MARKERS): return j @@ -4508,13 +4607,15 @@ def outside_diff_blocks(body: str) -> list[str]: def finding_count(block: str) -> int: - """The heading's `(N)`, floored at one, since a block reported as zero reads as a clean pass. + """The heading's stated count, floored at one, since a block reported as zero reads as clean. + + A heading stating its count first, `2 previously missed issues`, is read by `heading_count` too. + Left unread, it fell to the floor and reported one finding where the round raised two. A run longer than a count carries falls to that same floor rather than crashing the digest, the block existing being what says there is at least one finding in it. """ - m = COUNT.search(heading_of(block)) - stated = counted(m.group(1)) if m else None + stated = heading_count(heading_of(block)) return max(stated, 1) if stated is not None else 1 diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index ed1061fd..b06e1270 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -1337,8 +1337,8 @@ def test_no_exit_code_rides_on_it(self) -> None: self.assertEqual(0, code) def test_prose_naming_the_section_is_not_the_section(self) -> None: - """`marker_blocks` accepts a line on a count alone, so an unanchored marker reads an - ordinary English sentence carrying a number as a section this round raised. + """`marker_blocks` accepts a line that is not a heading on a `(N)` alone. An unanchored + marker then reads an ordinary English sentence carrying a number as a section this round raised. The observed wording is common enough to write by accident, and the result is a banner telling the reader a clean round raised a finding against code the branch never touched. @@ -1366,6 +1366,17 @@ def test_prose_naming_the_section_is_not_the_section(self) -> None: line = f"{opener}Previously missed (2) findings were re-raised this round." self.assertIsNone(pr_review.PREVIOUSLY_MISSED.match(line)) self.assertEqual([], pr_review.previously_missed_blocks(f"{OVERVIEW}\n{line}\n")) + counted = f"{opener}2 previously missed issues were re-raised this round." + self.assertIsNone(pr_review.PREVIOUSLY_MISSED.match(counted)) + for bare in ( + "2 previously missed issues were re-raised this round.", + "2 previously missed (3) rounds ago, the guard is back.", + "> 2 previously missed (3) rounds ago, the guard is back.", + "2 previously missed (3) rounds ago", + "2 previously missed (3) rounds ago, the guard is back.", + ): + with self.subTest(bare=bare): + self.assertEqual([], pr_review.previously_missed_blocks(f"{OVERVIEW}\n{bare}\n")) def test_the_heading_is_read_through_whatever_markup_it_wears(self) -> None: """What the anchor must admit, beside the prose above that it must not. @@ -1386,10 +1397,154 @@ def test_the_heading_is_read_through_whatever_markup_it_wears(self) -> None: # An emphasis marker is heading markup and a bullet is prose, and a space is what separates them, so both spellings are held here rather than only the one. ("italic, with no tags", "_Previously missed (1)_"), ("bold and italic", "***Previously missed (1)***"), + ( + "the count-first spelling", + "2 previously missed issues", + ), + ("a count-first markdown heading", "### 2 previously missed issues"), ): with self.subTest(case=label): self.assertIsNotNone(pr_review.PREVIOUSLY_MISSED.match(heading)) + def test_a_count_first_heading_is_the_section_and_states_its_count(self) -> None: + """A later revision opens the heading on its count, `2 previously missed issues`. + + Unread, the round exited 43 on an unvetted summary. Its findings open no thread, so they + reached the agent only through the raw body. Read without its count, the block fell to the + floor of one. It then reported one finding where the round raised two. + """ + body = counted_missed(2) + self.assertEqual([], pr_review.unrecognized_in(body)) + blocks = pr_review.previously_missed_blocks(body) + self.assertEqual(1, len(blocks)) + self.assertEqual(2, pr_review.finding_count(blocks[0])) + self.assertEqual([], pr_review.unrecognized_in(counted_missed(1))) + self.assertEqual( + 3, pr_review.finding_count(pr_review.previously_missed_blocks(counted_missed(3))[0]) + ) + long_run = counted_missed(1).replace( + "1 previously missed issue", "12345 previously missed issue" + ) + long_blocks = pr_review.previously_missed_blocks(long_run) + self.assertEqual(1, len(long_blocks)) + self.assertEqual(1, pr_review.finding_count(long_blocks[0])) + self.answer(payload([review(body=body)])) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("previously_missed=2 (on_head=2 earlier=0)", out) + self.assertIn("shapes=ok", out) + self.assertIn("Low severity Constructed missed finding 2", out) + + def test_a_summary_split_across_lines_is_still_the_section(self) -> None: + """A summary whose text sits on lines of its own is vetted by its spelling. A heading + found only within one line read it as no section, while `shapes=ok` reported nothing.""" + for label, summary in ( + ( + "its text on a line of its own", + "\n2 previously missed issues\n", + ), + ( + "its text set off by blank lines", + "\n\n2 previously missed issues\n\n", + ), + ( + "its emphasis in capitals", + "2 previously missed issues", + ), + ( + "its wording broken across lines", + "2 previously\nmissed issues", + ), + ): + with self.subTest(case=label): + body = counted_missed(2).replace( + "2 previously missed issues", summary + ) + self.assertEqual([], pr_review.unrecognized_in(body)) + blocks = pr_review.previously_missed_blocks(body) + self.assertEqual(1, len(blocks)) + self.assertEqual(2, pr_review.finding_count(blocks[0])) + self.assertIn("Constructed missed finding 2", blocks[0]) + + def test_a_vetted_summary_no_reader_finds_is_reported(self) -> None: + """The spelling is vetted wherever it appears, while the reader keys on how its line opens. + + A layout the reader misses would hand its findings to nobody under `shapes=ok`. So the + shortfall between the two is reported as an unread shape instead. + """ + body = counted_missed(2).replace( + "2 previously missed issues", + "2 previously missed issues", + ) + self.assertEqual([], pr_review.previously_missed_blocks(body)) + found = pr_review.unrecognized_in(body) + self.assertEqual(1, len(found)) + self.assertIn("no reader found its section", found[0]) + + def test_an_entry_title_opening_on_a_number_bounds_no_section(self) -> None: + """An entry's summary opens on its severity badge, which is not heading markup. + + Read as markup, an entry titled `2 low confidence results are dropped` stated a count. That + entry then bounded the section, and the entries after it were dropped from it. + """ + body = counted_missed(3).replace( + "Constructed missed finding 2", "2 low confidence results are dropped" + ) + blocks = pr_review.previously_missed_blocks(body) + self.assertEqual(1, len(blocks)) + self.assertIn("Constructed missed finding 3", blocks[0]) + self.assertEqual(3, pr_review.finding_count(blocks[0])) + + def test_a_tag_named_in_prose_folds_nothing(self) -> None: + """Only a line opening on `\nReview details\n\n" + "### Suppressed comments (1)\n\n" + "The SUMMARY pattern misses a opener.\n\n" + "2 previously missed issues\n\n" + "The previously missed finding.\n\n" + ) + missed = pr_review.previously_missed_blocks(shared) + self.assertEqual(1, len(missed)) + self.assertIn("The previously missed finding.", missed[0]) + suppressed = pr_review.suppressed_blocks(shared) + self.assertEqual(1, len(suppressed)) + self.assertNotIn("The previously missed finding.", suppressed[0]) + + def test_a_section_ends_where_its_count_first_sibling_starts(self) -> None: + """A sibling bounds a section only where its heading states a count. + + A bound reading only `(N)` ran a suppressed block into a count-first previously-missed + sibling. The digest then printed that finding twice, once under the suppressed banner. + """ + shared = ( + f"{OVERVIEW}\n
\nReview details\n\n" + "### Suppressed comments (1)\n\nThe suppressed finding.\n\n" + "
\n2 previously missed issues\n\n" + "The previously missed finding.\n
\n
\n" + ) + suppressed = pr_review.suppressed_blocks(shared) + self.assertEqual(1, len(suppressed)) + self.assertIn("The suppressed finding.", suppressed[0]) + self.assertNotIn("The previously missed finding.", suppressed[0]) + self.answer(payload([review(body=shared)])) + out, _ = pr_review.digest("o", "r", 7) + self.assertEqual(1, out.count("The previously missed finding.")) + + def test_a_count_first_section_cancels_the_flagging_verdict_reading(self) -> None: + """The round flags a closer look and states no open issue. + + Its findings are the previously missed ones, which the digest now counts. So the round is + not a verdict naming nothing. + """ + self.answer(payload([review(body=counted_missed(2))])) + self.assertIsNone(pr_review.uncounted_verdict(payload([review(body=counted_missed(2))]))) + with contextlib.redirect_stdout(io.StringIO()) as printed: + code = pr_review.main(["status", "7", "--repo", "o/r"]) + self.assertNotIn("VERDICT WITH NO COUNTED FINDING", printed.getvalue()) + self.assertNotIn("UNRECOGNIZED", printed.getvalue()) + self.assertEqual(0, code) + def test_a_quoted_heading_is_not_a_section_this_round_raised(self) -> None: """Every other reader in this file strips a quotation before looking, and this one did not. @@ -1879,6 +2034,29 @@ def previously_missed( ) +def counted_missed(entries: int) -> str: + """The later revision's previously-missed section, its summary opening on the count. + + The round flags a closer look and states its zero as `**No open issues**`. The section nests + one badged entry per finding, each a `FILE:LINE` anchor and a priority-tagged paragraph. + That nesting is the observed one, and every title, path, and sentence is constructed, per + GOVERNANCE.md "Representative Data in Agent-Authored Text". + """ + noun = "issue" if entries == 1 else "issues" + found = "".join( + f"
\n{BADGE} Constructed missed finding {i}\n\n" + f"`a.py:{10 * i}`\n\n[P1] A constructed paragraph about the finding.\n
\n" + for i in range(1, entries + 1) + ) + block = ( + f"
\n{entries} previously missed {noun}\n\n" + f"In code that hasn't changed since last review\n\n{found}
\n\n
" + ) + return revised_with( + "**No open issues** \U0001f389", "### \U0001f535 Needs a closer look" + ).replace("
", block, 1) + + class TestCoverage(GqlCase): """The round that covered the head and read part of the diff, which is a clean pass elsewhere. @@ -4109,6 +4287,22 @@ def test_the_revision_s_open_findings_line_is_its_stated_total(self) -> None: ) self.assertIsNone(pr_review.stated_total(inside)) + def test_the_revision_s_no_open_issues_line_is_a_stated_zero(self) -> None: + """A later revision writes its zero as `**No open issues**` closed by an emoji, and unread + it printed `?` over a round stating nothing open. The line anchor still holds, so prose + naming the words, or a line going on past the emoji, states nothing.""" + for line, total in ( + ("**No open issues** \U0001f389", 0), + ("**No open issues**", 0), + ("**1 open issue**", 1), + ("**2 open issues**", 2), + ): + with self.subTest(line=line): + self.assertEqual(total, pr_review.stated_total(overview_v2_revised(line))) + for line in ("It had **No open issues** before.", "**No open issues** \U0001f389 yet"): + with self.subTest(line=line): + self.assertIsNone(pr_review.stated_total(overview_v2_revised(line))) + def test_a_total_split_across_severities_is_summed(self) -> None: """The format states a count per severity, so the first number alone undercounts. @@ -4651,23 +4845,6 @@ def test_a_phrase_rewrapped_across_comment_lines_reads_as_one_sentence(self) -> comment_flowed_text(wrapped), ) - def test_the_vetted_lists_hold_what_the_comment_beside_them_counts(self) -> None: - """The comment states the sizes, and adding an entry without it is how it goes stale.""" - source = comment_flowed_text(Path(pr_review.__file__).read_text(encoding="utf-8")) - stated = re.search( - r"the output is regular: (\d+) headings, (\d+) summaries and (\d+) labels", source - ) - if stated is None: - self.fail("the comment stating the vetted list sizes is no longer in the source") - self.assertEqual( - ( - len(pr_review.VETTED_HEADINGS), - len(pr_review.VETTED_SUMMARIES), - len(pr_review.VETTED_LABELS), - ), - tuple(int(g) for g in stated.groups()), - ) - COVERS_ONE = "" @@ -4728,6 +4905,42 @@ def test_the_open_findings_section_states_the_round_s_total(self) -> None: body = revised_with(open_section(("6000000001", "6000000002", "6000000003"))) self.assertEqual(3, pr_review.stated_total(body)) + def test_the_open_issues_spelling_is_the_same_section(self) -> None: + """A later revision writes `1 open issue` where it wrote `1 open finding`, and exited 43. + + Its entries link the round's own threads, as the earlier spelling's do. So it is read as the + same section: vetted, stating the total, and each entry checked against the threads. + """ + for ids in (("6000000001",), ("6000000001", "6000000002")): + noun = "issue" if len(ids) == 1 else "issues" + with self.subTest(count=len(ids)): + body = revised_with(open_section(ids, noun=noun)) + self.assertEqual([], pr_review.unrecognized_in(body)) + self.assertEqual(len(ids), pr_review.stated_total(body)) + rd = review(body=body, rid="PRR_head") + threads = [thread(f"T{i}", rid="PRR_head", cid=c) for i, c in enumerate(ids)] + self.assertEqual(0, pr_review.unthreaded_entries(payload([rd], threads))) + lone = revised_with(open_section(("6000000009",), noun="issue")) + self.assertEqual(1, pr_review.unthreaded_entries(payload([review(body=lone)]))) + + def test_a_summary_split_across_lines_keeps_its_count(self) -> None: + """The role is vetted on the whole summary, so its count is read from the whole summary + too. Read from its first line alone, `Open` then `(2)` stated no count.""" + ids = ("6000000001", "6000000002", "6000000003") + split = revised_with(open_section(ids)).replace( + "3 open findings", "\n3 open findings\n" + ) + self.assertEqual([3], [c for r, c, _ in pr_review.section_entries(split) if r == "total"]) + self.assertEqual(3, pr_review.stated_total(split)) + for spelled in ("3 open findings", "3 open findings"): + with self.subTest(spelled=spelled): + body = revised_with(open_section(ids)).replace( + "3 open findings", spelled + ) + self.assertEqual(3, pr_review.stated_total(body)) + closing = overview_v2().replace("Open (2)", "Open\n(2)") + self.assertEqual([2], [c for r, c, _ in pr_review.section_entries(closing) if r == "open"]) + def test_a_count_first_summary_outside_the_table_is_still_unvetted(self) -> None: """The table vets spellings, not any summary that opens on a count.""" body = revised_with(open_section(("6000000001",), noun="questions")) @@ -5015,7 +5228,11 @@ def test_fifty_outranks_a_stuck_check(self) -> None: def test_a_collapsed_finding_cancels_the_reading(self) -> None: """A suppressed or previously-missed block is a counted finding the digest already prints.""" - for summary in ("Previously missed (1)", "Suppressed comments (1)"): + for summary in ( + "Previously missed (1)", + "2 previously missed issues", + "Suppressed comments (1)", + ): with self.subTest(summary=summary): block = ( f"
\n{summary}\n\n"