Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions scripts/pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -4534,7 +4534,7 @@ 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 is sliced from its heading to the next sibling heading or the end of its own region.
Past a region's first section, only the sibling heading it ends at can open another.
Past a region's first section, only a sibling heading some section ends at can open another.

Shared by `suppressed_blocks`, `previously_missed_blocks`, and `outside_diff_blocks`.
`strip_blockquote` affects heading detection only, never the returned content. A summary split
Expand Down Expand Up @@ -4565,9 +4565,10 @@ def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = F
bare = COUNT.search(line) and not OPENING_COUNT.match(line)
if marker.search(line) and (HEADING.match(line) or bare) and not entry_heading(line):
start = i
while start < len(scan_lines) and marker.search(scan_lines[start]):
while start < len(scan_lines):
end = sibling_heading(scan_lines, start)
blocks.append("\n".join(raw_lines[start:end]))
if marker.search(scan_lines[start]):
blocks.append("\n".join(raw_lines[start:end]))
start = end
break
return blocks
Expand Down
42 changes: 42 additions & 0 deletions tests/test_pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,48 @@ def test_a_section_ends_where_its_sibling_s_heading_starts(self) -> None:
self.assertEqual(1, out.count("The suppressed finding."))
self.assertEqual(1, out.count("The previously missed finding."))

def test_sections_alternating_in_one_wrapper_are_each_read(self) -> None:
"""The chain of bounds runs through a section of the other kind rather than stopping there.

Stopped at the first sibling of the other kind, the second suppressed section reached no
block. The digest counted one suppressed finding of three, under `shapes=ok`.
"""
shared = (
f"{OVERVIEW}\n<details>\n<summary>Review details</summary>\n\n"
"<summary>Suppressed comments (1)</summary>\n\nThe first suppressed finding.\n\n"
"<summary>Previously missed (1)</summary>\n\nThe previously missed finding.\n\n"
"<summary>Suppressed comments (2)</summary>\n\nThe second suppressed finding.\n\n"
"The third suppressed finding.\n</details>\n"
)
suppressed = pr_review.suppressed_blocks(shared)
self.assertEqual(2, len(suppressed))
self.assertIn("The first suppressed finding.", suppressed[0])
self.assertIn("The second suppressed finding.", suppressed[1])
self.assertIn("The third suppressed finding.", suppressed[1])
self.assertNotIn("The previously missed finding.", "".join(suppressed))
missed = pr_review.previously_missed_blocks(shared)
self.assertEqual(1, len(missed))
self.assertNotIn("suppressed finding.", missed[0])
self.answer(payload([review(body=shared)]))
out, _ = pr_review.digest("o", "r", 7)
self.assertIn("suppressed=3", out)
self.assertIn("shapes=ok", out)
for finding in (
"first suppressed",
"previously missed",
"second suppressed",
"third suppressed",
):
self.assertEqual(1, out.count(f"The {finding} finding."))
Comment thread
Copilot marked this conversation as resolved.
swapped = (
f"{OVERVIEW}\n<details>\n<summary>Review details</summary>\n\n"
"<summary><strong>Previously missed (1)</strong></summary>\n\nThe first.\n\n"
"<summary><strong>Suppressed comments (1)</strong></summary>\n\nThe second.\n\n"
"<summary><strong>Previously missed (1)</strong></summary>\n\nThe third.\n</details>\n"
)
self.assertEqual(2, len(pr_review.previously_missed_blocks(swapped)))
self.assertEqual([], pr_review.unrecognized_in(swapped))

def test_a_finding_entry_is_no_section_whatever_its_title_says(self) -> None:
"""An entry's title is free text, and either section's marker words can appear in it.

Expand Down
Loading