Read a Section Heading Through One Reduction in pr_review.py

scripts/pr_review.py

@@ -380,6 +380,7 @@
 import time
 import unicodedata
 import urllib.parse
+from collections import Counter
 from collections.abc import Callable, Iterable, Iterator
 from datetime import UTC, datetime
 from pathlib import Path
@@ -835,10 +836,7 @@ def strip_fences(
 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]")
+LEADING_COUNT = re.compile(r"^([#*_ ]*)([0-9]+)(?= )")
 # 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 `(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.
@@ -3616,6 +3614,26 @@ def counted(digits: str) -> int | None:
     return int(digits) if len(digits) <= COUNT_DIGITS else None
 
 
+def reduced(text: str) -> str:
+    """The text with each severity badge reduced to its word and its emphasis markup dropped.
+
+    The one reduction `normal`, `unwrap`, and `heading_count` share, so a heading is vetted,
+    printed, and counted from the same text.
+    """
+    return EMPHASIS.sub("", SEVERITY_BADGE.sub(badge_text, text))
+
+
+def spaced(text: str) -> str:
+    """The text `reduced`, then cut to ASCII and single spaces, which `LEADING_COUNT` reads.
+
+    `LEADING_COUNT` is the one leading-count rule `normal` and `heading_count` share. It reads a
+    count past a Markdown heading or emphasis marker, and `normal` keeps that marker in place. It
+    skips no tag, since a finding entry 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.
+    """
+    return re.sub(r"\s+", " ", "".join(c for c in reduced(text) if ord(c) < 128)).strip()
+
+
 def normal(text: str) -> str:
     """A marker reduced toward what a vetted list compares: emphasis markup dropped, ASCII,
     single spaces, counts as `(N)`.
@@ -3632,11 +3650,9 @@ def normal(text: str) -> str:
 
     A count opening the marker is rewritten the same way, since a later revision of the second
     format writes `3 resolved since last review` where it once closed the summary on `(3)`.
+    `heading_count` reads that count by the same `LEADING_COUNT`.
     """
-    unbadged = SEVERITY_BADGE.sub(badge_text, text)
-    ascii_only = "".join(c for c in EMPHASIS.sub("", unbadged) if ord(c) < 128)
-    spaced = re.sub(r"\s+", " ", re.sub(r"\(\d+\)", "(N)", ascii_only)).strip()
-    return re.sub(r"^\d+(?= )", "(N)", spaced)
+    return LEADING_COUNT.sub(r"\1(N)", re.sub(r"\([0-9]+\)", "(N)", spaced(text)))
 
 
 def finding_entry(marker: str) -> bool:
@@ -3675,7 +3691,9 @@ def unrecognized_in(body: str) -> list[str]:
 
     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`.
+    misses would otherwise hand its findings to nobody under `shapes=ok`. The check compares which
+    summaries the blocks hold rather than how many blocks there are. Compared by count, a bare
+    `(N)` line read in one region stood in for a summary missed in another.
 
     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
@@ -3699,13 +3717,19 @@ 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 = [
+    missed = Counter(
         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")
+    )
+    read = Counter(
+        normal(s)
+        for block in previously_missed_blocks(body)
+        for s in SUMMARY.findall(CODE_SPAN.sub(" ", strip_fences(block)))
+    )
+    unread = missed - read
+    if unread:
+        found.append(f"summary: {next(iter(unread))}, vetted while no reader found its section")
     found += [
         f"metadata label: {la}" for la in dict.fromkeys(labels) if unvetted(la, VETTED_LABELS)
     ]
@@ -4313,13 +4337,15 @@ def heading_count(heading: str) -> int | None:
     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.
+    The count is read from `spaced`, the text `normal` vets the role on, through the same
+    `LEADING_COUNT`. 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
+    text = spaced(heading)
+    closing = COUNT.search(text)
+    opening = LEADING_COUNT.match(text)
+    digits = closing.group(1) if closing else opening.group(2) if opening else None
+    return counted(digits) if digits is not None else None
 
 
 # `<details>(.*?)</details>` 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.
@@ -4447,9 +4473,23 @@ def mask_quotations(body: str, span: str = " ") -> str:
 SECTION_MARKERS = (SUPPRESSED, PREVIOUSLY_MISSED)
 
 
+def entry_heading(line: str) -> bool:
+    """Whether this line is a finding entry's own heading, which opens on a severity badge.
+
+    A finding entry is never a section heading, whatever words its title carries. `SUPPRESSED`
+    matches anywhere in a line, and `PREVIOUSLY_MISSED` reads past the badge as heading markup.
+    Read as a heading, an entry titled `2 low confidence results are dropped` opened a section.
+    A summary opening on a severity and stating a count is still reported by `unrecognized_in`,
+    as an unvetted spelling.
+    """
+    return bool(SEVERITY_TEXT.match(spaced(TAGS.sub("", heading_of(line)))))
+
+
 def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = False) -> list[str]:
-    """Return the review body's sections whose own heading matches `marker`, each sliced from
-    that heading through to the end of its own region.
+    """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.
 
     Shared by `suppressed_blocks`, `previously_missed_blocks`, and `outside_diff_blocks`.
     `strip_blockquote` affects heading detection only, never the returned content. A summary split
@@ -4478,8 +4518,12 @@ def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = F
             scan_lines = [BLOCKQUOTE.sub("", ln) for ln in scan_lines]
         for i, line in enumerate(scan_lines):
             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)]))
+            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]):
+                    end = sibling_heading(scan_lines, start)
+                    blocks.append("\n".join(raw_lines[start:end]))
+                    start = end
                 break
     return blocks
 
@@ -4520,7 +4564,7 @@ def fold_summaries(scan_lines: list[str], raw_lines: list[str]) -> tuple[list[st
     return scan, raw
 
 
-def sibling_heading(scan_lines: list[str], start: int, marker: re.Pattern[str]) -> int:
+def sibling_heading(scan_lines: list[str], start: int) -> int:
     """Where this block ends, which is the next sibling section's heading or the region's end.
 
     Two sections sharing one wrapper is a shape the corpus carries, a Markdown heading each inside
@@ -4536,9 +4580,12 @@ def sibling_heading(scan_lines: list[str], start: int, marker: re.Pattern[str])
     opposite triage about them, while both counts come out wrong.
 
     A stated count is what separates them, the same signal `finding_entry` reads: every section
-    heading here states one and a finding's own title does not. Requiring it also keeps this off
-    the CodeRabbit blocks, whose own marker is in neither of the two and which were never bounded
-    before this.
+    heading here states one and a finding's own title does not. A title can still state one, so
+    an entry opening on its severity badge never bounds, per `entry_heading`. Requiring a count
+    also narrows the bound on a CodeRabbit block, whose own marker is in neither of the two.
+
+    A section of the same kind that follows directly bounds too, so each is its own block with
+    its own count. Read as one block, the digest counted only the first heading's findings.
 
     The cost is named rather than hidden. A sibling heading stating no count does not bound, so a
     block runs into it, which is where this started before the bound existed at all. It is strictly
@@ -4550,7 +4597,9 @@ def sibling_heading(scan_lines: list[str], start: int, marker: re.Pattern[str])
         line = scan_lines[j]
         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):
+        if entry_heading(line):
+            continue
+        if any(o.search(line) for o in SECTION_MARKERS):
             return j
     return len(scan_lines)
 
@@ -4571,7 +4620,7 @@ def unwrap(block: str) -> str:
     nothing, and a severity badge says one word wrapped in two versioned icon URLs that bury the
     title following them. The word is kept and the markup is not.
     """
-    return TAGS.sub("", EMPHASIS.sub("", SEVERITY_BADGE.sub(badge_text, block)))
+    return TAGS.sub("", reduced(block))
 
 
 def previously_missed_blocks(body: str) -> list[str]:

tests/test_pr_review.py

@@ -1531,6 +1531,23 @@ def test_a_section_ends_where_its_count_first_sibling_starts(self) -> None:
         out, _ = pr_review.digest("o", "r", 7)
         self.assertEqual(1, out.count("The previously missed finding."))
 
+    def test_prose_past_a_bound_opens_no_second_section(self) -> None:
+        """Only the sibling heading a section ends at can open another section in its region.
+
+        Scanned on past the bound, prose naming the marker beside a `(N)` opened a second
+        suppressed block. The previously missed finding then printed under both banners.
+        """
+        shared = (
+            f"{OVERVIEW}\n<details>\n<summary>Review details</summary>\n\n"
+            "### Suppressed comments (1)\n\nThe suppressed finding.\n\n"
+            "<details>\n<summary><strong>2 previously missed issues</strong></summary>\n\n"
+            "A fallback treats low confidence (2) results as clean.\n</details>\n</details>\n"
+        )
+        suppressed = pr_review.suppressed_blocks(shared)
+        self.assertEqual(1, len(suppressed))
+        self.assertNotIn("A fallback treats", suppressed[0])
+        self.assertEqual(1, len(pr_review.previously_missed_blocks(shared)))
+
     def test_a_count_first_section_cancels_the_flagging_verdict_reading(self) -> None:
         """The round flags a closer look and states no open issue.
 
@@ -1584,6 +1601,66 @@ 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_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.
+
+        Read as a heading, an entry opened the other section at that entry. Its finding then
+        printed under both banners, which assert opposite triage, under `shapes=ok`.
+        """
+        body = f"{OVERVIEW}\n{COVERED}\n\n" + entries_section(
+            "Previously missed (2)", "Constructed finding 1", "2 low confidence results are dropped"
+        )
+        self.assertEqual([], pr_review.suppressed_blocks(body))
+        self.assertEqual(2, pr_review.previously_missed_blocks(body)[0].count("paragraph"))
+        self.assertEqual([], pr_review.unrecognized_in(body))
+        body = f"{OVERVIEW}\n{COVERED}\n\n" + entries_section(
+            "Comments suppressed due to low confidence (2)",
+            "Constructed finding 1",
+            "2 previously missed guards are re-added",
+        )
+        self.assertEqual([], pr_review.previously_missed_blocks(body))
+        self.assertEqual(2, pr_review.suppressed_blocks(body)[0].count("paragraph"))
+
+    def test_a_finding_entry_stating_a_count_does_not_end_its_section(self) -> None:
+        """A title can state a count, which is what bounds a section on its sibling's heading.
+
+        Bounded there, the entries after it left the section they belong to.
+        """
+        body = f"{OVERVIEW}\n{COVERED}\n\n" + entries_section(
+            "Previously missed (3)",
+            "Constructed finding 1",
+            "Suppressed comments (3) are dropped",
+            "Constructed finding 3",
+        )
+        self.assertEqual([], pr_review.suppressed_blocks(body))
+        self.assertEqual(3, pr_review.previously_missed_blocks(body)[0].count("paragraph"))
+
+    def test_the_vetted_but_unread_check_compares_which_summaries_were_read(self) -> None:
+        """Compared by how many, a count of blocks stood in for which summaries they hold.
+
+        Two sections in one wrapper read as one block holding both, counted by the first heading.
+        A bare line read in one region matched the count of a summary no reader found in another.
+        """
+        nested_pair = (
+            f"{OVERVIEW}\n{COVERED}\n\n<details>\n<summary>Review details</summary>\n\n"
+            + entries_section("Previously missed (1)", "Constructed finding 1")
+            + entries_section("Previously missed (2)", "Constructed finding 2", "Constructed 3")
+            + "</details>\n"
+        )
+        blocks = pr_review.previously_missed_blocks(nested_pair)
+        self.assertEqual([1, 2], [pr_review.finding_count(b) for b in blocks])
+        self.assertEqual([1, 2], [b.count("paragraph") for b in blocks])
+        self.assertEqual([], pr_review.unrecognized_in(nested_pair))
+        stand_in = (
+            f"{OVERVIEW}\n{COVERED}\n\nPreviously missed (2)\n\n<details>\n"
+            "<summary>Review details</summary>\n\n"
+            "Note: <summary>Previously missed (1)</summary>\n\n</details>\n"
+        )
+        self.assertEqual(
+            ["summary: Previously missed (N), vetted while no reader found its section"],
+            pr_review.unrecognized_in(stand_in),
+        )
+
     def test_every_no_thread_render_drops_the_same_presentation_markup(self) -> None:
         """All of these sections come from one format, so a badge in a suppressed block buries its
         title exactly as one in a previously-missed block does.
@@ -2034,6 +2111,23 @@ def previously_missed(
     )
 
 
+def entries_section(
+    heading: str, *titles: str, preamble: str = "", paragraph: str = "A constructed paragraph."
+) -> str:
+    """A collapsed section under `heading`, nesting one badged entry per title.
+
+    The nesting is the observed one, and every title, path, and sentence is constructed.
+    """
+    found = "".join(
+        f"<details>\n<summary>{BADGE} {title}</summary>\n\n"
+        f"`a.py:{10 * i}`\n\n{paragraph}\n</details>\n"
+        for i, title in enumerate(titles, 1)
+    )
+    return (
+        f"<details>\n<summary><strong>{heading}</strong></summary>\n\n{preamble}{found}</details>\n"
+    )
+
+
 def counted_missed(entries: int) -> str:
     """The later revision's previously-missed section, its summary opening on the count.
 
@@ -2043,15 +2137,13 @@ def counted_missed(entries: int) -> str:
     GOVERNANCE.md "Representative Data in Agent-Authored Text".
     """
     noun = "issue" if entries == 1 else "issues"
-    found = "".join(
-        f"<details>\n<summary>{BADGE} Constructed missed finding {i}</summary>\n\n"
-        f"`a.py:{10 * i}`\n\n[P1] A constructed paragraph about the finding.\n</details>\n"
-        for i in range(1, entries + 1)
-    )
-    block = (
-        f"<details>\n<summary><strong>{entries} previously missed {noun}</strong></summary>\n\n"
-        f"In code that hasn't changed since last review\n\n{found}</details>\n\n<details>"
+    block = entries_section(
+        f"{entries} previously missed {noun}",
+        *(f"Constructed missed finding {i}" for i in range(1, entries + 1)),
+        preamble="In code that hasn't changed since last review\n\n",
+        paragraph="[P1] A constructed paragraph about the finding.",
     )
+    block += "\n<details>"
     return revised_with(
         "**No open issues** \U0001f389", "### \U0001f535 Needs a closer look"
     ).replace("<details>", block, 1)
@@ -4095,6 +4187,16 @@ def test_emphasis_markup_around_a_marker_is_dropped_rather_than_vetted_with_it(s
         self.assertEqual("Open (N)", pr_review.normal("Open (2)"))
         self.assertEqual("Review details", pr_review.normal("Review details"))
 
+    def test_the_role_a_summary_is_vetted_with_and_its_count_come_from_one_rule(self) -> None:
+        """Defined once for vetting and once for counting, the leading count disagreed.
+
+        A form feed after the count vetted the open-findings section while reading no count.
+        """
+        summary = "<strong>2\fopen findings</strong>"
+        self.assertEqual("(N) open findings", pr_review.normal(summary))
+        self.assertEqual(2, pr_review.heading_count(summary))
+        self.assertEqual(2, pr_review.heading_count("### 2 previously missed issues"))
+
     def test_the_effort_line_reads_in_either_format_s_own_spelling(self) -> None:
         """The first states it as a `Review details` bullet and the second as a bare bold line.