[PATCH v1] tools: AI review handle empty Error sections

[PATCH v1] tools: AI review handle empty Error sections

[Matthew Gee]

Previous review-patch.py would detect and report and error or warning
based off of the occurrence of the headers of the error and warning
sections. This led to consistent false positives as often AI reviewers
will create the header but put "none" or similar filler text within
the following body.

This patch updates the code in order to check if the AI review has a
body with error or warnings to fix and not just filler text. This is
done by querying multiple lines at once and adjusting the regex to
filter out headers followed only by filler text or formatting in the
review.

These changes were tested against 10+ AI review outputs with several
variations in formatting and filler text in order to catch a good
variety of cases to make sure code reviews with actual errors or
warnings are caught and not missed.

Signed-off-by: Matthew Gee <mgee@iol.unh.edu>
---
 devtools/ai/review-patch.py | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..b009368638 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -19,6 +19,7 @@
 from email.message import EmailMessage
 from pathlib import Path
 from typing import Any, Iterator
+from itertools import tee, islice, chain
 
 from _common import (
     PROVIDERS,
@@ -147,19 +148,37 @@ def classify_review(review_text: str, output_format: str) -> int:
             pass  # Fall through to text scanning
 
     if not has_errors and not has_warnings:
-        # Scan review text for severity indicators.
-        # Match section headers and inline markers across text/markdown/html.
-        for line in review_text.splitlines():
-            stripped = line.strip().lower()
+        # Matches against error or warning section headers
+        rgx_should_match: str = r"#+\s(\*+)?{err_or_warn}"
+        # Matches against headers followed only by filler text, formatting, or no additional text
+        rgx_should_not_match: str = r"#+\s(\*+)?{err_or_warn}(s?)(\*+)?(none(.)?$| \(must fix\)$|$)"
+        
+        curr_lines: iter[str]
+        next_lines: iter[str | None]
+        next_next_lines: iter[str | None]
+        curr_lines, next_lines, next_next_lines = tee(review_text.splitlines(), 3)
+        next_lines = chain(islice(next_lines, 1, None), [None])
+        next_next_lines = chain(islice(next_next_lines, 2, None), [None, None])
+        
+        curr_lines: str
+        next_lines: str | None
+        next_next_lines: str | None
+        for curr_line, next_line, next_next_line in zip(curr_lines, next_lines, next_next_lines):
+            stripped: str = curr_line.strip().lower() + str(
+                next_line or '').strip().lower() + str(next_next_line or '').strip().lower()
             # Skip quoted patch context lines
             if stripped.startswith(">") or stripped.startswith("diff --git"):
                 continue
-            if re.match(r"^(#{1,3}\s+)?(\*{0,2})error", stripped) or re.match(
-                r"^<h[1-3]>\s*error", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='error'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='error'), stripped
             ):
                 has_errors = True
-            elif re.match(r"^(#{1,3}\s+)?(\*{0,2})warning", stripped) or re.match(
-                r"^<h[1-3]>\s*warning", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='warning'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='warning'), stripped
             ):
                 has_warnings = True
 
-- 
2.54.0

[PATCH v2] tools: AI review handle empty Error sections

[Matthew Gee]

Previous review-patch.py would detect and report and error or warning
based off of the occurrence of the headers of the error and warning
sections. This led to consistent false positives as often AI reviewers
will create the header but put "none" or similar filler text within
the following body.

This patch updates the code in order to check if the AI review has a
body with error or warnings to fix and not just filler text. This is
done by querying multiple lines at once and adjusting the regex to
filter out headers followed only by filler text or formatting in the
review.

These changes were tested against 10+ AI review outputs with several
variations in formatting and filler text in order to catch a good
variety of cases to make sure code reviews with actual errors or
warnings are caught and not missed.

Signed-off-by: Matthew Gee <mgee@iol.unh.edu>
---
 devtools/ai/review-patch.py | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..eb608566d2 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -19,6 +19,7 @@
 from email.message import EmailMessage
 from pathlib import Path
 from typing import Any, Iterator
+from itertools import tee, islice, chain
 
 from _common import (
     PROVIDERS,
@@ -147,19 +148,37 @@ def classify_review(review_text: str, output_format: str) -> int:
             pass  # Fall through to text scanning
 
     if not has_errors and not has_warnings:
-        # Scan review text for severity indicators.
-        # Match section headers and inline markers across text/markdown/html.
-        for line in review_text.splitlines():
-            stripped = line.strip().lower()
+        # Matches against error or warning section headers
+        rgx_should_match: str = r"#+\s(\*+)?{err_or_warn}"
+        # Matches against headers followed only by filler text, formatting, or no additional text
+        rgx_should_not_match: str = r"#+\s(\*+)?{err_or_warn}(s?)(\*+)?(none(.)?$| \(must fix\)$|$)"
+
+        curr_lines: iter[str]
+        next_lines: iter[str | None]
+        next_next_lines: iter[str | None]
+        curr_lines, next_lines, next_next_lines = tee(review_text.splitlines(), 3)
+        next_lines = chain(islice(next_lines, 1, None), [None])
+        next_next_lines = chain(islice(next_next_lines, 2, None), [None, None])
+
+        curr_lines: str
+        next_lines: str | None
+        next_next_lines: str | None
+        for curr_line, next_line, next_next_line in zip(curr_lines, next_lines, next_next_lines):
+            stripped: str = curr_line.strip().lower() + str(
+                next_line or '').strip().lower() + str(next_next_line or '').strip().lower()
             # Skip quoted patch context lines
             if stripped.startswith(">") or stripped.startswith("diff --git"):
                 continue
-            if re.match(r"^(#{1,3}\s+)?(\*{0,2})error", stripped) or re.match(
-                r"^<h[1-3]>\s*error", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='error'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='error'), stripped
             ):
                 has_errors = True
-            elif re.match(r"^(#{1,3}\s+)?(\*{0,2})warning", stripped) or re.match(
-                r"^<h[1-3]>\s*warning", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='warning'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='warning'), stripped
             ):
                 has_warnings = True
 
-- 
2.54.0

Re: [PATCH v2] tools: AI review handle empty Error sections

[Stephen Hemminger]

On Fri, 12 Jun 2026 15:08:07 -0400
Matthew Gee <mgee@iol.unh.edu> wrote:

> Previous review-patch.py would detect and report and error or warning
> based off of the occurrence of the headers of the error and warning
> sections. This led to consistent false positives as often AI reviewers
> will create the header but put "none" or similar filler text within
> the following body.
> 
> This patch updates the code in order to check if the AI review has a
> body with error or warnings to fix and not just filler text. This is
> done by querying multiple lines at once and adjusting the regex to
> filter out headers followed only by filler text or formatting in the
> review.
> 
> These changes were tested against 10+ AI review outputs with several
> variations in formatting and filler text in order to catch a good
> variety of cases to make sure code reviews with actual errors or
> warnings are caught and not missed.
> 
> Signed-off-by: Matthew Gee <mgee@iol.unh.edu>
> ---

AI saw some stuff which I missed.

Matthew,

The windowing (tee/islice/chain) is aligned correctly, but the new
matching has a few problems.

The bigger issue is that this only recognizes markdown '#' headers now.
The old scan matched an optional '#' prefix, '**' bold, and '<h1-3>' too.
rgx_should_match requires "#+\s", so plain-text "Errors:" / "**Errors**"
and HTML "<h2>Errors</h2>" no longer match. Default --format is text and
html is supported, so reviews in those formats classify clean (exit 0)
even when they report errors, which silently breaks the 2/3 exit codes
compare-patch-reviews.sh relies on. The '#'/'**'/'<h>' alternatives need
to stay.

The 3-line concatenation also reintroduces the false positive you're
trying to kill. curr+next+next_next are joined with no separator, so

	## Errors
	None
	## Warnings

collapses to "## errorsnone## warnings"; the 'none...$' filler anchor
fails because the next header follows, and has_errors gets set. Any
output that doesn't blank-line-separate sections hits this.

And in the other direction, content more than two lines below a header
is missed: two blank lines between "## Errors" and the body leave
stripped == "## errors", the '$' branch matches, and a real finding is
suppressed. The heuristic ends up sensitive to exact line spacing both
ways.

Minor: iter[str] / iter[str | None] aren't valid generics (use the
already-imported Iterator); the vars are also re-annotated with
conflicting types and annotate the iterators rather than the loop
targets. mypy will reject these. Run black too - the new for/zip line
and the stripped assignment are over width.

Suggest keeping header matching in all three formats, then scanning past
blank lines to the first non-empty body line and suppressing only if
it's filler (none/n/a/empty). That decouples detection from spacing
instead of fixing it to a 3-line shape.