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

[Matthew Gee]

This patch fixes a bug where review-patch.py would detect and report an
error or warning only 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 keeping track of whether the for loop parser is within an error
or warning section; analyzing the first non-whitespace line within the
section. If the first non-whitespace line matches known filler then the
section can be ignored.  It has been observed that if the AI includes
filler then there is no actual concern.

These changes were tested against 10+ markdown AI review outputs with
several variations in formatting and filler text. The changes caught
error or warning sections with actual concerns and successfully ignored
sections containing only filler.

Signed-off-by: Matthew Gee <mgee at iol.unh.edu>
---
 devtools/ai/review-patch.py | 48 +++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..53ae7e6a4f 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 enum import Flag, auto
 
 from _common import (
     PROVIDERS,
@@ -120,6 +121,12 @@
 EXIT_ERRORS = 3
 
 
+class ReviewParseState(Flag):
+    NORMAL = auto()
+    IN_ERROR = auto()
+    IN_WARNING = auto()
+
+
 def classify_review(review_text: str, output_format: str) -> int:
     """Classify review result and return appropriate exit code.
 
@@ -147,22 +154,43 @@ 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()
-            # Skip quoted patch context lines
-            if stripped.startswith(">") or stripped.startswith("diff --git"):
+        # Matches against error or warning section headers
+        rgx_header_match: str = r"(#+\s)?(\*+)?(<h[1-3]>)?{err_or_warn}"
+        # Matches against observed filler text
+        rgx_filler_match: str = r"(none(.)?$|\(must fix\)$|$)"
+
+        curr_line: str
+        for curr_line in review_text.splitlines():
+            stripped: str = curr_line.strip().lower()
+
+            if (
+                stripped.startswith(">")
+                or stripped.startswith("diff --git")
+                or stripped == ""
+            ):
                 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_header_match.format(err_or_warn="error"), stripped):
+                curr_state = ReviewParseState.IN_ERROR
+
+            elif re.match(rgx_header_match.format(err_or_warn="warning"), stripped):
+                curr_state = ReviewParseState.IN_WARNING
+
+            elif curr_state == ReviewParseState.IN_ERROR and not re.match(
+                rgx_filler_match, stripped
             ):
+                curr_state = ReviewParseState.NORMAL
                 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 curr_state == ReviewParseState.IN_WARNING and not re.match(
+                rgx_filler_match, stripped
             ):
+                curr_state = ReviewParseState.NORMAL
                 has_warnings = True
 
+            else:
+                curr_state = ReviewParseState.NORMAL
+
     if has_errors:
         return EXIT_ERRORS
     if has_warnings:
-- 
2.54.0