From 88f11dfa0c7f48992711ebcb83ed8aa443643f6d Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 11:40:27 -0700 Subject: [PATCH 1/9] Count an Open (N) Entry Carrying No Thread Link as Unthreaded An `Open (N)` section is not read as a total, so an entry it counts that carries no `#discussion_r` link, or one spelled in a way the reader does not recognize, dropped out of every count and the digest passed it. The section's count beyond the links it carries is now added to the unthreaded entries, so `FINDINGS WITH NO THREAD` names it. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 17 ++++++++++++++++- tests/test_pr_review.py | 12 ++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 42ca67176..6e468a5d4 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. Where that section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one, while a finding the count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. An `Open (N)` section counting more entries than it links counts the difference the same way, since an entry carrying no link, or one spelled in a way this reader does not recognize, links no thread either. Where that section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one, while a finding the count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 82c0453ef..c5c1e8bce 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -2111,13 +2111,28 @@ def unthreaded_entries(pr: dict) -> int: a finding named only in the review body, which no thread poll reaches. Distinct ids are counted, and a resolved section's entries are not, being threads an earlier round raised. A thread past the hundred the query reads counts here too, overstating rather than hiding. + An `Open (N)` section counting more entries than it links adds the difference, since an entry + carrying no link, or one this reader cannot recognize, links no thread either. """ known = { str(first_comment(thread)["fullDatabaseId"]) for thread in ((pr.get("reviewThreads") or {}).get("nodes") or []) if first_comment(thread).get("fullDatabaseId") is not None } - return len(open_entry_ids(pr) - known) + return len(open_entry_ids(pr) - known) + unlinked_open_entries(pr) + + +def unlinked_open_entries(pr: dict) -> int: + """How many entries the head round's `Open (N)` sections count beyond the links they carry.""" + newest = second_format_head(pr) + if newest is None: + return 0 + sections = read_overview(newest.get("body") or "")[2] + return sum( + max(count - len(ids), 0) + for role, count, ids in sections + if role == OPEN and count is not None + ) def open_entry_ids(pr: dict) -> set[str]: diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index ad0098952..b8b858b6b 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -4223,6 +4223,18 @@ def test_an_unthreaded_entry_under_no_stated_total_says_so(self) -> None: out, ) + def test_an_open_list_entry_carrying_no_link_is_counted(self) -> None: + """`Open (N)` is not a total, so an entry it counts with no thread link is counted here.""" + linked = "- [Finding 1](#discussion_r4000000001) New" + body = overview_v2(findings="", entries=1) + self.assertIn(linked, body) + self.assertIn("Open (1)", body) + body = body.replace("Open (1)", "Open (2)").replace(linked, f"{linked}\n- Finding 2 New") + self.answer(payload([review(body=body)], [thread("T1", cid="4000000001")])) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("FINDINGS WITH NO THREAD (1)", out) + self.assertIn("and 1 of the entries its open section lists links no thread", out) + def test_an_entry_linking_no_thread_is_counted_where_the_totals_balance(self) -> None: """The round opened as many threads as it states, and one entry links none of them, so the totals alone pass a finding named only in the body.""" From a6da3325f6296cb1e806d9d462ab230f6fe42fc6 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 11:44:03 -0700 Subject: [PATCH 2/9] Count Linked Lines Rather Than Links in an Open (N) Section An entry can link a thread beside its own, and two links on one entry cancelled an entry carrying none. The section's linked entries are now the lines in it carrying a link. The module docstring states the new count beside the README, and the paragraph's sentences are split under the length gate. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 30 +++++++++++++++++------------- tests/test_pr_review.py | 13 +++++++++++++ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 6e468a5d4..489150b34 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. An `Open (N)` section counting more entries than it links counts the difference the same way, since an entry carrying no link, or one spelled in a way this reader does not recognize, links no thread either. Where that section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one, while a finding the count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond the lines in it carrying a link. Such an entry carries no link, or one spelled in a way this reader does not recognize. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding the count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index c5c1e8bce..1c17cf92f 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -161,8 +161,9 @@ Where the preamble states no total, the count opening the open-findings section is `T`, since that format's later revision writes the bare zero line only for a zero. Each entry an open section lists links its thread, and an entry linking no thread on - the pull request is counted in that same block whatever the totals say. Where that - section supplied `T`, the block is measured against its entries rather than against + the pull request is counted in that same block whatever the totals say. So is each + entry an `Open (N)` section counts beyond the lines in it carrying a link. Where the + open-findings section supplied `T`, the block is measured against its entries rather than against `M`, so an entry linking an earlier round's thread is no finding without one, while a finding the count names and no entry links is still counted. Ten bodies in that format have been read here, so what follows describes those rather @@ -1946,7 +1947,7 @@ def findings_on(tail: str) -> int | None: DISCUSSION_LINK = re.compile(r"\]\(#discussion_r(\d+)\)") -OverviewSection = tuple[str, int | None, list[str]] +OverviewSection = tuple[str, int | None, list[str], int] def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: @@ -1961,9 +1962,10 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: is the prose under it up to the next heading or bold line, taken from the body unmasked so a code span the headline names survives. Each section is a top-level `
` whose own summary is a row of `OVERVIEW_SECTIONS`, as its role, the count that - summary states, and the thread id each of its entries links, which is the database id of that - thread's first comment. Quotations are masked first, so a body quoting a section is not read - as carrying one. + summary states, the thread id each of its entries links, which is the database id of that + thread's first comment, and how many of its lines carry a link. That last is the linked entry + count, since an entry is one line and can link a thread beside its own. Quotations are masked + first, so a body quoting a section is not read as carrying one. An open section's entries are findings, each checked against the threads, and a resolved section's are threads an earlier round raised. The count-first open section also states the @@ -1994,7 +1996,9 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: continue digits = re.search(r"\d+", EMPHASIS.sub("", summary)) count = counted(digits.group()) if digits else None - sections.append((role, count, DISCUSSION_LINK.findall(masked[start:end]))) + text = masked[start:end] + linked = sum(1 for line in text.splitlines() if DISCUSSION_LINK.search(line)) + sections.append((role, count, DISCUSSION_LINK.findall(text), linked)) return verdict, " ".join(headline), sections @@ -2028,7 +2032,7 @@ def stated_total(body: str, preamble_only: bool = False) -> int | None: totals += [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] + found = [c for role, c, *_ in read_overview(body)[2] if role == TOTAL and c is not None] return max(found, default=None) @@ -2123,14 +2127,14 @@ def unthreaded_entries(pr: dict) -> int: def unlinked_open_entries(pr: dict) -> int: - """How many entries the head round's `Open (N)` sections count beyond the links they carry.""" + """How many entries the head round's `Open (N)` sections count beyond their linked lines.""" newest = second_format_head(pr) if newest is None: return 0 sections = read_overview(newest.get("body") or "")[2] return sum( - max(count - len(ids), 0) - for role, count, ids in sections + max(count - linked, 0) + for role, count, _, linked in sections if role == OPEN and count is not None ) @@ -2141,7 +2145,7 @@ def open_entry_ids(pr: dict) -> set[str]: if newest is None: return set() sections = read_overview(newest.get("body") or "")[2] - return {i for role, _, ids in sections if role != RESOLVED for i in ids} + return {i for role, _, ids, _ in sections if role != RESOLVED for i in ids} def uncounted_verdict(pr: dict) -> tuple[str, str] | None: @@ -2164,7 +2168,7 @@ def uncounted_verdict(pr: dict) -> tuple[str, str] | None: verdict, headline, sections = read_overview(body) if not verdict or unvetted(verdict, FLAGGING_VERDICTS): return None - listed = any(role != RESOLVED and (count or ids) for role, count, ids in sections) + listed = any(role != RESOLVED and (count or ids) for role, count, ids, _ in sections) if listed or stated_total(body) or round_threads(pr, newest): return None if suppressed_blocks(body) or previously_missed_blocks(body): diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index b8b858b6b..574b506a3 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -4235,6 +4235,19 @@ def test_an_open_list_entry_carrying_no_link_is_counted(self) -> None: self.assertIn("FINDINGS WITH NO THREAD (1)", out) self.assertIn("and 1 of the entries its open section lists links no thread", out) + def test_a_second_link_on_one_entry_does_not_cover_an_unlinked_entry(self) -> None: + """An entry linking a thread beside its own is still one entry, so the unlinked one counts.""" + linked = "- [Finding 1](#discussion_r4000000001) New" + body = overview_v2(findings="", entries=1) + self.assertIn(linked, body) + body = body.replace("Open (1)", "Open (2)").replace( + linked, f"{linked}, see [earlier](#discussion_r4000000007)\n- Finding 2 New" + ) + threads = [thread("T1", cid="4000000001"), thread("T7", cid="4000000007")] + self.answer(payload([review(body=body)], threads)) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("FINDINGS WITH NO THREAD (1)", out) + def test_an_entry_linking_no_thread_is_counted_where_the_totals_balance(self) -> None: """The round opened as many threads as it states, and one entry links none of them, so the totals alone pass a finding named only in the body.""" From ca2cb940a8b3f8c9ca2e116e16ca581e02397c0a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 11:47:59 -0700 Subject: [PATCH 3/9] Count Only Margin List Items as Linked Open (N) Entries A back-reference on its own indented line under an entry is not another entry, and counting every linked line let one cancel an unlinked entry. The digest names the entries its open sections count rather than list, since an entry the count names may not be listed, and the docstrings and README now describe the entry count rather than the link count. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 28 +++++++++++++++++----------- tests/test_pr_review.py | 19 ++++++++++++++++--- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 489150b34..42d0c9f7f 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond the lines in it carrying a link. Such an entry carries no link, or one spelled in a way this reader does not recognize. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding the count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. Such an entry carries no link, or one spelled in a way this reader does not recognize. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 1c17cf92f..29c428337 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -162,10 +162,10 @@ `T`, since that format's later revision writes the bare zero line only for a zero. Each entry an open section lists links its thread, and an entry linking no thread on the pull request is counted in that same block whatever the totals say. So is each - entry an `Open (N)` section counts beyond the lines in it carrying a link. Where the - open-findings section supplied `T`, the block is measured against its entries rather than against - `M`, so an entry linking an earlier round's thread is no finding without one, while a - finding the count names and no entry links is still counted. + entry an `Open (N)` section counts beyond those it lists with a link. Where the + open-findings section supplied `T`, the block is measured against its entries rather + than against `M`, so an entry linking an earlier round's thread is no finding without + one, while a finding that section's count names and no entry links is still counted. Ten bodies in that format have been read here, so what follows describes those rather than the format in general. None carried a `Suppressed comments` heading, which is why this field rather than `suppressed=` is what finds a withheld finding in one. @@ -1947,6 +1947,7 @@ def findings_on(tail: str) -> int | None: DISCUSSION_LINK = re.compile(r"\]\(#discussion_r(\d+)\)") +ENTRY_LINE = re.compile(r"[-*+]\s") OverviewSection = tuple[str, int | None, list[str], int] @@ -1963,9 +1964,10 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: code span the headline names survives. Each section is a top-level `
` whose own summary is a row of `OVERVIEW_SECTIONS`, as its role, the count that summary states, the thread id each of its entries links, which is the database id of that - thread's first comment, and how many of its lines carry a link. That last is the linked entry - count, since an entry is one line and can link a thread beside its own. Quotations are masked - first, so a body quoting a section is not read as carrying one. + thread's first comment, and how many of its entries carry a link. An entry is a list item at + the margin, and one can link a thread beside its own or hold an indented back-reference, so + its links overstate its entries. Quotations are masked first, so a body quoting a section is + not read as carrying one. An open section's entries are findings, each checked against the threads, and a resolved section's are threads an earlier round raised. The count-first open section also states the @@ -1997,7 +1999,11 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: digits = re.search(r"\d+", EMPHASIS.sub("", summary)) count = counted(digits.group()) if digits else None text = masked[start:end] - linked = sum(1 for line in text.splitlines() if DISCUSSION_LINK.search(line)) + linked = sum( + 1 + for line in text.splitlines() + if ENTRY_LINE.match(line) and DISCUSSION_LINK.search(line) + ) sections.append((role, count, DISCUSSION_LINK.findall(text), linked)) return verdict, " ".join(headline), sections @@ -2115,8 +2121,8 @@ def unthreaded_entries(pr: dict) -> int: a finding named only in the review body, which no thread poll reaches. Distinct ids are counted, and a resolved section's entries are not, being threads an earlier round raised. A thread past the hundred the query reads counts here too, overstating rather than hiding. - An `Open (N)` section counting more entries than it links adds the difference, since an entry - carrying no link, or one this reader cannot recognize, links no thread either. + An `Open (N)` section counting more entries than it lists with a link adds the difference, + since an entry carrying no link, or one this reader cannot recognize, links no thread either. """ known = { str(first_comment(thread)["fullDatabaseId"]) @@ -3755,7 +3761,7 @@ def digest( else f" and opened {listed} thread{'' if listed == 1 else 's'}" ) + ( - f", and {unthreaded} of the entries its open section lists " + f", and {unthreaded} of the entries its open sections count " f"link{'s' if unthreaded == 1 else ''} no thread on this pull request" if unthreaded else "" diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 574b506a3..b989afb11 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -4218,7 +4218,7 @@ def test_an_unthreaded_entry_under_no_stated_total_says_so(self) -> None: out, _ = pr_review.digest("o", "r", 7) self.assertIn("overview=?/0", out) self.assertIn( - "states no total and opened 0 threads, and 1 of the entries its open section lists " + "states no total and opened 0 threads, and 1 of the entries its open sections count " "links no thread", out, ) @@ -4233,7 +4233,7 @@ def test_an_open_list_entry_carrying_no_link_is_counted(self) -> None: self.answer(payload([review(body=body)], [thread("T1", cid="4000000001")])) out, _ = pr_review.digest("o", "r", 7) self.assertIn("FINDINGS WITH NO THREAD (1)", out) - self.assertIn("and 1 of the entries its open section lists links no thread", out) + self.assertIn("and 1 of the entries its open sections count links no thread", out) def test_a_second_link_on_one_entry_does_not_cover_an_unlinked_entry(self) -> None: """An entry linking a thread beside its own is still one entry, so the unlinked one counts.""" @@ -4248,6 +4248,19 @@ def test_a_second_link_on_one_entry_does_not_cover_an_unlinked_entry(self) -> No out, _ = pr_review.digest("o", "r", 7) self.assertIn("FINDINGS WITH NO THREAD (1)", out) + def test_an_indented_back_reference_does_not_cover_an_unlinked_entry(self) -> None: + """A back-reference on its own line under an entry is not another entry.""" + linked = "- [Finding 1](#discussion_r4000000001) New" + body = overview_v2(findings="", entries=1) + self.assertIn(linked, body) + body = body.replace("Open (1)", "Open (2)").replace( + linked, f"{linked}\n - see [earlier](#discussion_r4000000007)\n- Finding 2 New" + ) + threads = [thread("T1", cid="4000000001"), thread("T7", cid="4000000007")] + self.answer(payload([review(body=body)], threads)) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("FINDINGS WITH NO THREAD (1)", out) + def test_an_entry_linking_no_thread_is_counted_where_the_totals_balance(self) -> None: """The round opened as many threads as it states, and one entry links none of them, so the totals alone pass a finding named only in the body.""" @@ -4260,7 +4273,7 @@ def test_an_entry_linking_no_thread_is_counted_where_the_totals_balance(self) -> out, _ = pr_review.digest("o", "r", 7) self.assertIn("overview=2/2", out) self.assertIn("FINDINGS WITH NO THREAD (1)", out) - self.assertIn("and 1 of the entries its open section lists links no thread", out) + self.assertIn("and 1 of the entries its open sections count links no thread", out) def test_a_resolved_section_s_entries_are_not_findings(self) -> None: """Its entries link threads an earlier round raised, so none needs a thread of its own.""" From 0849634e0dddbf816210b564ea411d288ee91a10 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 11:56:55 -0700 Subject: [PATCH 4/9] Name the Section in the Open (N) Docstrings `read_overview`'s docstring bound "its links" to an entry rather than the section, and `unlinked_open_entries` said linked lines where it counts linked entries. Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 29c428337..af9765e14 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -1965,9 +1965,9 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: `
` whose own summary is a row of `OVERVIEW_SECTIONS`, as its role, the count that summary states, the thread id each of its entries links, which is the database id of that thread's first comment, and how many of its entries carry a link. An entry is a list item at - the margin, and one can link a thread beside its own or hold an indented back-reference, so - its links overstate its entries. Quotations are masked first, so a body quoting a section is - not read as carrying one. + the margin, and one can link a thread beside its own or hold an indented back-reference, so a + section's links overstate the section's entries. Quotations are masked first, so a body + quoting a section is not read as carrying one. An open section's entries are findings, each checked against the threads, and a resolved section's are threads an earlier round raised. The count-first open section also states the @@ -2133,7 +2133,7 @@ def unthreaded_entries(pr: dict) -> int: def unlinked_open_entries(pr: dict) -> int: - """How many entries the head round's `Open (N)` sections count beyond their linked lines.""" + """How many entries the head round's `Open (N)` sections count beyond their linked entries.""" newest = second_format_head(pr) if newest is None: return 0 From 26cd35a30fa7ff3c6c1456ad60f6bcbc7e197219 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 11:59:46 -0700 Subject: [PATCH 5/9] Define a Linked Entry as a Bullet at the Margin The docstring called any margin list item an entry, and the README named only a missing or unrecognized link as what makes an entry unlinked. The code counts a bullet at the margin carrying a recognized link, so a numbered or wrapped entry counts as unlinked too, and both now say so. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 42d0c9f7f..6455b5ff5 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. Such an entry carries no link, or one spelled in a way this reader does not recognize. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. Only a bullet at the margin carrying a link this reader recognizes is a linked entry, so any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index af9765e14..f36d940a8 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -1964,8 +1964,8 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: code span the headline names survives. Each section is a top-level `
` whose own summary is a row of `OVERVIEW_SECTIONS`, as its role, the count that summary states, the thread id each of its entries links, which is the database id of that - thread's first comment, and how many of its entries carry a link. An entry is a list item at - the margin, and one can link a thread beside its own or hold an indented back-reference, so a + thread's first comment, and how many of its entries carry a link. An entry is a bullet at the + margin, and one can link a thread beside its own or hold an indented back-reference, so a section's links overstate the section's entries. Quotations are masked first, so a body quoting a section is not read as carrying one. From c632fe8ffbf4b99fc3e21e2ef6ce84be6596bde2 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 12:02:48 -0700 Subject: [PATCH 6/9] Say an Unlinked Entry Carries No Link This Script Reads An entry whose link the reader does not recognize can still link a real thread, so the digest no longer says it links no thread. `read_overview` names its linked entry count as the bullets at the margin carrying a recognized link, rather than redefining an entry. Co-Authored-By: Claude Opus 5.5 --- scripts/pr_review.py | 16 +++++++++------- tests/test_pr_review.py | 6 +++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index f36d940a8..6e28f8895 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -1964,10 +1964,10 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: code span the headline names survives. Each section is a top-level `
` whose own summary is a row of `OVERVIEW_SECTIONS`, as its role, the count that summary states, the thread id each of its entries links, which is the database id of that - thread's first comment, and how many of its entries carry a link. An entry is a bullet at the - margin, and one can link a thread beside its own or hold an indented back-reference, so a - section's links overstate the section's entries. Quotations are masked first, so a body - quoting a section is not read as carrying one. + thread's first comment, and its linked entry count, the bullets at its margin carrying a link + this reader recognizes. One entry can link a thread beside its own or hold an indented + back-reference, so a section's links overstate its linked entries. Quotations are masked + first, so a body quoting a section is not read as carrying one. An open section's entries are findings, each checked against the threads, and a resolved section's are threads an earlier round raised. The count-first open section also states the @@ -2121,8 +2121,9 @@ def unthreaded_entries(pr: dict) -> int: a finding named only in the review body, which no thread poll reaches. Distinct ids are counted, and a resolved section's entries are not, being threads an earlier round raised. A thread past the hundred the query reads counts here too, overstating rather than hiding. - An `Open (N)` section counting more entries than it lists with a link adds the difference, - since an entry carrying no link, or one this reader cannot recognize, links no thread either. + An `Open (N)` section counting more entries than it has linked entries adds the difference. + An entry carrying a link this reader does not recognize lands there too, overstating rather + than hiding. """ known = { str(first_comment(thread)["fullDatabaseId"]) @@ -3762,7 +3763,8 @@ def digest( ) + ( f", and {unthreaded} of the entries its open sections count " - f"link{'s' if unthreaded == 1 else ''} no thread on this pull request" + f"{'carries' if unthreaded == 1 else 'carry'} no link this script reads to a " + "thread on this pull request" if unthreaded else "" ) diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index b989afb11..30211bc66 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -4219,7 +4219,7 @@ def test_an_unthreaded_entry_under_no_stated_total_says_so(self) -> None: self.assertIn("overview=?/0", out) self.assertIn( "states no total and opened 0 threads, and 1 of the entries its open sections count " - "links no thread", + "carries no link this script reads to a thread", out, ) @@ -4233,7 +4233,7 @@ def test_an_open_list_entry_carrying_no_link_is_counted(self) -> None: self.answer(payload([review(body=body)], [thread("T1", cid="4000000001")])) out, _ = pr_review.digest("o", "r", 7) self.assertIn("FINDINGS WITH NO THREAD (1)", out) - self.assertIn("and 1 of the entries its open sections count links no thread", out) + self.assertIn("open sections count carries no link this script reads to a thread", out) def test_a_second_link_on_one_entry_does_not_cover_an_unlinked_entry(self) -> None: """An entry linking a thread beside its own is still one entry, so the unlinked one counts.""" @@ -4273,7 +4273,7 @@ def test_an_entry_linking_no_thread_is_counted_where_the_totals_balance(self) -> out, _ = pr_review.digest("o", "r", 7) self.assertIn("overview=2/2", out) self.assertIn("FINDINGS WITH NO THREAD (1)", out) - self.assertIn("and 1 of the entries its open sections count links no thread", out) + self.assertIn("open sections count carries no link this script reads to a thread", out) def test_a_resolved_section_s_entries_are_not_findings(self) -> None: """Its entries link threads an earlier round raised, so none needs a thread of its own.""" From ccb63d0ef8755a3131b0ea18937666e16380fa14 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 12:12:40 -0700 Subject: [PATCH 7/9] Read Linked Entries Outside a Block Collapsed Within an Entry A list collapsed inside an entry stayed in the section's text, and a bullet at its margin counted as a linked entry, which could cancel an unlinked one. The count now reads only the section's own text outside nested blocks. Tests now cover the zero floor and the `*` and `+` markers, and the module docstring and README state what a linked entry is. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 19 +++++++++++-------- tests/test_pr_review.py | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 6455b5ff5..d0bc91514 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. Only a bullet at the margin carrying a link this reader recognizes is a linked entry, so any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. A linked entry is a bullet at the section's margin carrying a link this reader recognizes, and a bullet inside a block collapsed within an entry is not one. Any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 6e28f8895..c08477ce2 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -162,10 +162,11 @@ `T`, since that format's later revision writes the bare zero line only for a zero. Each entry an open section lists links its thread, and an entry linking no thread on the pull request is counted in that same block whatever the totals say. So is each - entry an `Open (N)` section counts beyond those it lists with a link. Where the - open-findings section supplied `T`, the block is measured against its entries rather - than against `M`, so an entry linking an earlier round's thread is no finding without - one, while a finding that section's count names and no entry links is still counted. + entry an `Open (N)` section counts beyond its linked entries, the bullets at its + margin carrying a link this script reads. Where the open-findings section supplied + `T`, the block is measured against its entries rather than against `M`, so an entry + linking an earlier round's thread is no finding without one, while a finding that + section's count names and no entry links is still counted. Ten bodies in that format have been read here, so what follows describes those rather than the format in general. None carried a `Suppressed comments` heading, which is why this field rather than `suppressed=` is what finds a withheld finding in one. @@ -1965,9 +1966,10 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: `
` whose own summary is a row of `OVERVIEW_SECTIONS`, as its role, the count that summary states, the thread id each of its entries links, which is the database id of that thread's first comment, and its linked entry count, the bullets at its margin carrying a link - this reader recognizes. One entry can link a thread beside its own or hold an indented - back-reference, so a section's links overstate its linked entries. Quotations are masked - first, so a body quoting a section is not read as carrying one. + this reader recognizes, outside any block collapsed within it. One entry can link a thread + beside its own, hold an indented back-reference, or collapse a list of its own, so a section's + links overstate its linked entries. Quotations are masked first, so a body quoting a section + is not read as carrying one. An open section's entries are findings, each checked against the threads, and a resolved section's are threads an earlier round raised. The count-first open section also states the @@ -1999,9 +2001,10 @@ def read_overview(body: str) -> tuple[str, str, list[OverviewSection]]: digits = re.search(r"\d+", EMPHASIS.sub("", summary)) count = counted(digits.group()) if digits else None text = masked[start:end] + own = "\n".join(text[a:b] for a, b in details_spans(tags[start:end])[1]) linked = sum( 1 - for line in text.splitlines() + for line in own.splitlines() if ENTRY_LINE.match(line) and DISCUSSION_LINK.search(line) ) sections.append((role, count, DISCUSSION_LINK.findall(text), linked)) diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 30211bc66..a5d2604fe 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -4261,6 +4261,45 @@ def test_an_indented_back_reference_does_not_cover_an_unlinked_entry(self) -> No out, _ = pr_review.digest("o", "r", 7) self.assertIn("FINDINGS WITH NO THREAD (1)", out) + def test_a_bullet_collapsed_within_an_entry_does_not_cover_an_unlinked_entry(self) -> None: + """A list collapsed inside an entry holds back-references rather than entries.""" + linked = "- [Finding 1](#discussion_r4000000001) New" + body = overview_v2(findings="", entries=1) + self.assertIn(linked, body) + nested = ( + "
Related\n\n" + "- see [earlier](#discussion_r4000000007)\n\n
\n" + ) + body = body.replace("Open (1)", "Open (2)").replace( + linked, f"{linked}\n{nested}- Finding 2 New" + ) + threads = [thread("T1", cid="4000000001"), thread("T7", cid="4000000007")] + self.answer(payload([review(body=body)], threads)) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("FINDINGS WITH NO THREAD (1)", out) + + def test_more_linked_entries_than_counted_cancel_no_unthreaded_entry(self) -> None: + """A section counting fewer entries than it links still counts an id no thread carries.""" + linked = "- [Finding 1](#discussion_r4000000001) New" + body = overview_v2(findings="", entries=1) + self.assertIn(linked, body) + body = body.replace(linked, f"{linked}\n- [Finding 7](#discussion_r4000000007) New") + self.answer(payload([review(body=body)], [thread("T7", cid="4000000007")])) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("FINDINGS WITH NO THREAD (1)", out) + + def test_every_bullet_marker_reads_as_a_linked_entry(self) -> None: + """`*` and `+` open an entry as `-` does, so a list in either is fully linked.""" + for marker in ("*", "+"): + with self.subTest(marker=marker): + linked = "- [Finding 1](#discussion_r4000000001) New" + body = overview_v2(findings="", entries=1) + self.assertIn(linked, body) + body = body.replace(linked, f"{marker}{linked[1:]}") + self.answer(payload([review(body=body)], [thread("T1", cid="4000000001")])) + out, _ = pr_review.digest("o", "r", 7) + self.assertNotIn("FINDINGS WITH NO THREAD", out) + def test_an_entry_linking_no_thread_is_counted_where_the_totals_balance(self) -> None: """The round opened as many threads as it states, and one entry links none of them, so the totals alone pass a finding named only in the body.""" From 844b332d6c2b59050eb7fad5c3b00f4cb76f2199 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 12:12:49 -0700 Subject: [PATCH 8/9] Split the Linked-Entry Sentence Under the Length Gate Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/README.md b/scripts/README.md index d0bc91514..ebccfc216 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. A linked entry is a bullet at the section's margin carrying a link this reader recognizes, and a bullet inside a block collapsed within an entry is not one. Any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. A linked entry is a bullet at the section's margin carrying a link this reader recognizes. A bullet inside a block collapsed within an entry is not one. Any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. From b7d81720a4e98df6508e58afaf3e0ad64e78053f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 8 Oct 2026 12:15:39 -0700 Subject: [PATCH 9/9] State the Nested-Block Exclusion in Every Linked-Entry Definition The README limited the exclusion to a block inside an entry, and the module docstring left it out, where the code drops every bullet inside any block collapsed within the section. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index ebccfc216..b175cd8f4 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -225,7 +225,7 @@ The digest reports the **previously missed findings** the second body format col Copilot's reviews arrive in two body formats. None of the ten bodies read here in the second carried a `Suppressed comments` heading, so the reading above found nothing in it, and that format states its own finding total instead, as a count per severity that is summed across the badges the line pairs them with, since reading its first number alone reported two findings for a round stating two of one severity and three of another. The markup is masked to one sentinel before any count is read, a badge carrying digits of its own in its attributes, and a run of more than four digits is an identifier rather than a count, which is the second guard under a mask that can terminate early on a bracket a URL carries itself. Every count the mask left is then summed, rather than each being paired with markup beside it, since which side of a count that markup sits on and whether the severity carries any is presentation rather than structure: a bold split, a bare prose split, and a reference-style link each read as one severity under a paired reading. Summing overstates and never understates, which is the direction chosen deliberately, and its named cost is that a line stating a total and then its context, `12 findings across 4 files`, sums to sixteen. A shortfall overstated prints a block a reader checks against the body and blocks no merge, where one suppressed closes the gate on findings nobody answered. `overview=T/M` reports that total beside the number of review threads the round actually opened, `?` for `T` where no total is found in the overview preamble or a count-first open-findings section, since a total absent and a total of zero are different readings. A `?` covers a round stating no total, a round stating one only after its first collapsed section in anything but the open-findings section described below, and a round stating one as a bullet, that last blocking at exit 43 as an unvetted metadata label. None of them says the round withheld nothing. `M` comes from the API rather than the review's prose: that format enumerates the findings it opened threads for, and the anchor each entry links is the database id of that thread's own comment, so the threads are the same set already structured. Reading the enumeration instead took six attempts and four of them cancelled a shortfall by counting something that was not an entry, which needs a Markdown parser to tell apart rather than a line scan. A total larger than the thread count is findings raised where polling threads cannot see them, and a `FINDINGS WITH NO THREAD` block follows naming the shortfall, except where a count-first open section supplied the total, as the next paragraph says. A thread past the hundred `reviewThreads` reads would count in `M` too, so a cut page overstates the shortfall and `threads=` carries the trailing `+` that says so. The comparison holds while the round's enumeration lists what that round raised, and a format listing findings still open from earlier rounds states a total `M` undercounts, which is a shortfall to confirm against the body rather than to act on from the number. The field is present only where the round covering the head is written in that format, and it is head-scoped where `suppressed=` is not, because a total is one round's statement about one diff and the push that changes that diff raises a round stating its own, where a suppressed block carries the finding text an answer is still owed to. -One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. A linked entry is a bullet at the section's margin carrying a link this reader recognizes. A bullet inside a block collapsed within an entry is not one. Any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. +One reader, `read_overview`, reads every section of that format's overview. Each section it added arrived as a spelling nothing read, and each stopped the loop at exit `43` in turn. The `OVERVIEW_SECTIONS` table in the script names each collapsible section that lists thread links, with its role. The open-findings section states the later revision's total, since that revision writes the bare zero line only for a zero. So where the preamble states no total, that section's count is `T`. `Open (N)` is not read as a total, since it listed an earlier round's thread in 2 of the 41 rounds measured. Each open-section entry links the database id of its thread's first comment. An entry linking no thread on the pull request is a finding named only in the body. `FINDINGS WITH NO THREAD` counts it whatever the totals say. It also counts each entry an `Open (N)` section counts beyond those it lists with a link. A linked entry is a bullet at the section's margin carrying a link this reader recognizes. A bullet inside any block collapsed within the section is not one. Any other entry counts here too. Where the open-findings section supplied `T`, the block is measured against its entries rather than against `M`. So an entry linking an earlier round's thread is no finding without one. A finding that section's count names and no entry links is still counted. A resolved section's entries link threads an earlier round raised, so they are read as information rather than findings. `status` and `wait` both exit `50` where the head's round opens on a verdict other than the clean one and counts no finding. It states none, lists none in an open section, opened no thread, and collapsed none, and no reviewer's thread is open. Every count reads that round as a clean pass, so the digest prints its headline under `VERDICT WITH NO COUNTED FINDING`. Over the rounds read here in that format, most such headlines named a finding and the rest a caution about the change. Only reading the headline tells the two apart, so it takes the triage a suppressed finding gets, fixed or answered with `comment`. The code repeats on that head after the answer, since nothing here reads one. An open thread defers the reading rather than cancelling it, since the headline may name that thread. The code ranks under `42`, `43`, and `45`, a part-reviewed diff being the larger gap. It outranks `wait`'s `44`, so on such a head a stuck check shows only in the digest. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index c08477ce2..26f7a4d9c 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -163,10 +163,11 @@ Each entry an open section lists links its thread, and an entry linking no thread on the pull request is counted in that same block whatever the totals say. So is each entry an `Open (N)` section counts beyond its linked entries, the bullets at its - margin carrying a link this script reads. Where the open-findings section supplied - `T`, the block is measured against its entries rather than against `M`, so an entry - linking an earlier round's thread is no finding without one, while a finding that - section's count names and no entry links is still counted. + margin carrying a link this script reads, outside any block collapsed within it. + Where the open-findings section supplied `T`, the block is measured against its + entries rather than against `M`, so an entry linking an earlier round's thread is no + finding without one, while a finding that section's count names and no entry links is + still counted. Ten bodies in that format have been read here, so what follows describes those rather than the format in general. None carried a `Suppressed comments` heading, which is why this field rather than `suppressed=` is what finds a withheld finding in one.