From ec6f176498d73eeab09260bb14524e63a53d239b Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 1 Sep 2026 15:14:18 -0700 Subject: [PATCH 1/4] Treat an Escaped Backtick as a Literal, Not a Span Delimiter Qodo on the promotion PR #1179, and it is real. CODE_SPAN read a backslash-escaped backtick as an opening run, so it paired with the next real tick and masked everything between them. That is the silent direction: a genuine unknown section disappears and the digest closes the loop on a body nobody read. Reproduced against a control body that reports the section the escaped tick hid. Both runs now exclude a tick preceded by a backslash. The guard reads one backslash, so an escaped backslash under-masks rather than over-masks, which is the safe direction: a quoted tag reported as a section refuses loudly, where a hidden section does not. Also trimmed the CODE_SPAN comment from fifteen lines to four, which Qodo raised separately. The prose gate requires one sentence per line, so the lever is fewer sentences rather than tighter wrapping. A regression test covers the escaped tick and fails with the guard reverted. --- scripts/pr_review.py | 12 +++--------- scripts/tests/test_pr_review.py | 13 +++++++++++++ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 29d8e841f..f47135a65 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -272,15 +272,9 @@ FENCE = re.compile(r"^ {0,3}```.*?^ {0,3}```[^\n]*", re.DOTALL | re.MULTILINE) # An inline code span is a quotation for the same reason a fenced block is. # A reviewer naming `` in prose was read as opening one. -# That swallowed the body from there to the next real close tag. -# The backreference between two boundary guards matches the run length exactly. -# So a one-backtick span carrying a two-backtick run closes on its own run. -# A span cannot cross a blank line, per CommonMark. -# Unbounded it paired two stray backticks and masked every marker between them. -# Replaced by a space rather than deleted, since deleting joins the text on either side. -# ```` would otherwise become an opening tag the body never carried. -# A space also leaves nothing unprintable in a marker this script quotes to a reader. -CODE_SPAN = re.compile(r"(? None: line = "- **Files reviewed:** 31/31 changed files" self.assertEqual([line], pr_review.coverage_statements(OVERVIEW + "\n" + line + "\n")) + def test_an_escaped_backtick_does_not_open_a_span(self) -> None: + """A backslash-escaped backtick is a literal, so it delimits nothing. + + Read as an opener it paired with the next real tick and masked everything between, which + is the silent direction: a genuine unknown section disappears and the loop closes on a + body nobody read. + """ + body = ( + OVERVIEW + + "\nEscaped \\` then
Bogus
then ` a span `." + ) + self.assertIn("summary: Bogus", pr_review.unrecognized_in(body)) + def test_every_vetted_marker_together_reads_as_recognized(self) -> None: """The corpus shape in one body, so the lists are held against what they were built from.""" self.assertEqual([], pr_review.unrecognized_in(nested())) From 6fab2519de002fe602a09299856419640b1d7234 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 1 Sep 2026 15:36:19 -0700 Subject: [PATCH 2/4] Guard Only the Opener Against a Backslash Qodo on #1180, and it is right: guarding the closing run the same way as the opener skipped a valid close. A backslash is literal inside a code span rather than an escape, so `` `a\` `` is a span ending in a backslash. Reading its close as escaped let the span run on and mask the section after it, which is the same silent failure the opener guard was added to prevent. Reproduced before changing anything, against a body whose section disappears with the closer guarded and reports with it removed. The opener keeps the guard, since a backslash before an opening run does escape it. A regression test covers the span ending in a backslash and fails with the closer guard put back. --- scripts/pr_review.py | 5 +++-- scripts/tests/test_pr_review.py | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index f47135a65..1d098d119 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -272,9 +272,10 @@ FENCE = re.compile(r"^ {0,3}```.*?^ {0,3}```[^\n]*", re.DOTALL | re.MULTILINE) # An inline code span is a quotation for the same reason a fenced block is. # A reviewer naming `` in prose was read as opening one. -# Bounded to a paragraph, and an escaped tick delimits nothing, since either masks a section. +# Bounded to a paragraph, and an escaped tick opens nothing, since either masks a section. +# Only the opener takes that guard, a backslash being literal inside a span rather than an escape. # Replaced by a space, since deleting joins the sides into a tag the body never carried. -CODE_SPAN = re.compile(r"(? None: ) self.assertIn("summary: Bogus", pr_review.unrecognized_in(body)) + def test_a_span_ending_in_a_backslash_still_closes(self) -> None: + """A backslash inside a code span is literal, so it does not escape the closing run. + + Guarding the closer the same way as the opener skipped a valid close, and the span then + ran on and masked the section after it, which is the failure the opener guard exists to + prevent rather than to cause. + """ + body = ( + OVERVIEW + + "\nA span `a\\` then
Hidden
and ` more." + ) + self.assertIn("summary: Hidden", pr_review.unrecognized_in(body)) + def test_every_vetted_marker_together_reads_as_recognized(self) -> None: """The corpus shape in one body, so the lists are held against what they were built from.""" self.assertEqual([], pr_review.unrecognized_in(nested())) From 7bcd5defc62bad9f872685e6a92bfca48aa0d702 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 1 Sep 2026 15:45:02 -0700 Subject: [PATCH 3/4] Assert the Whole Marker List, and Cover the Close Guard From Both Sides Copilot on #1180, both findings taken. assertIn only proved the expected marker was present, so an extra one would pass unnoticed. Switched to assertEqual, as the nearby tests do, and it immediately earned its keep: two of the three expected lists were wrong. They named a `### Pull request overview` heading that OVERVIEW does not carry and that would be vetted anyway, so the tests were passing on a mental model of the output rather than the output. Added the case Copilot asked for, a span that both carries a marker and ends in a backslash. It is the other side of the close guard: the earlier test proves a valid close is recognized, this one proves recognizing it keeps the quoted marker masked. Each guard is now independently covered. Reverting the closer guard fails two tests, reverting the opener guard fails one. --- scripts/tests/test_pr_review.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/tests/test_pr_review.py b/scripts/tests/test_pr_review.py index 04bc42026..2af57fffc 100755 --- a/scripts/tests/test_pr_review.py +++ b/scripts/tests/test_pr_review.py @@ -1711,7 +1711,7 @@ def test_an_escaped_backtick_does_not_open_a_span(self) -> None: OVERVIEW + "\nEscaped \\` then
Bogus
then ` a span `." ) - self.assertIn("summary: Bogus", pr_review.unrecognized_in(body)) + self.assertEqual(["summary: Bogus"], pr_review.unrecognized_in(body)) def test_a_span_ending_in_a_backslash_still_closes(self) -> None: """A backslash inside a code span is literal, so it does not escape the closing run. @@ -1724,7 +1724,17 @@ def test_a_span_ending_in_a_backslash_still_closes(self) -> None: OVERVIEW + "\nA span `a\\` then
Hidden
and ` more." ) - self.assertIn("summary: Hidden", pr_review.unrecognized_in(body)) + self.assertEqual(["summary: Hidden"], pr_review.unrecognized_in(body)) + + def test_a_quoted_marker_stays_masked_when_the_span_ends_in_a_backslash(self) -> None: + """The other side of the close guard: a quoted marker must not escape through it. + + Where the span both carries a marker and ends in a backslash, failing to recognize the + close would report the quoted marker as a real section, which is the defect the masking + was added for in the first place. + """ + body = OVERVIEW + "\nQuoting `
Quoted
\\` here." + self.assertEqual([], pr_review.unrecognized_in(body)) def test_every_vetted_marker_together_reads_as_recognized(self) -> None: """The corpus shape in one body, so the lists are held against what they were built from.""" From 159d880828a2271fbc47a1670af2f0f25b9e5ec4 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 1 Sep 2026 15:51:21 -0700 Subject: [PATCH 4/4] State the Span Docstrings as Invariants Rather Than History Qodo: the new test docstrings narrate the parser's former failure, which couples them to an implementation that may be rewritten without the narration following. The staleness point is right, and it is separable from the convention. This file's docstrings say why a case exists rather than only what it asserts, and that stays. What changes is the tense: each now states what happens when the guard is absent, in the present, rather than what happened before it was added. The reader still learns why the case is there, and nothing goes stale if the regex is rewritten. --- scripts/tests/test_pr_review.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/tests/test_pr_review.py b/scripts/tests/test_pr_review.py index 2af57fffc..8b512ef99 100755 --- a/scripts/tests/test_pr_review.py +++ b/scripts/tests/test_pr_review.py @@ -1703,9 +1703,9 @@ def test_a_real_coverage_line_survives_the_code_span_mask(self) -> None: def test_an_escaped_backtick_does_not_open_a_span(self) -> None: """A backslash-escaped backtick is a literal, so it delimits nothing. - Read as an opener it paired with the next real tick and masked everything between, which - is the silent direction: a genuine unknown section disappears and the loop closes on a - body nobody read. + Reading one as an opener pairs it with the next real tick and masks everything between, + which is the silent direction: a genuine unknown section disappears and the loop closes + on a body nobody read. """ body = ( OVERVIEW @@ -1716,9 +1716,9 @@ def test_an_escaped_backtick_does_not_open_a_span(self) -> None: def test_a_span_ending_in_a_backslash_still_closes(self) -> None: """A backslash inside a code span is literal, so it does not escape the closing run. - Guarding the closer the same way as the opener skipped a valid close, and the span then - ran on and masked the section after it, which is the failure the opener guard exists to - prevent rather than to cause. + Guarding the closer the same way as the opener skips a valid close, so the span runs on + and masks the section after it, which is the failure the opener guard exists to prevent + rather than to cause. """ body = ( OVERVIEW @@ -1730,8 +1730,8 @@ def test_a_quoted_marker_stays_masked_when_the_span_ends_in_a_backslash(self) -> """The other side of the close guard: a quoted marker must not escape through it. Where the span both carries a marker and ends in a backslash, failing to recognize the - close would report the quoted marker as a real section, which is the defect the masking - was added for in the first place. + close reports the quoted marker as a real section, which is what the masking exists to + prevent. """ body = OVERVIEW + "\nQuoting `
Quoted
\\` here." self.assertEqual([], pr_review.unrecognized_in(body))