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 <[email protected]> --- 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
