From f2441ddd2a32001300e3798e6c7a66a6489fd75b Mon Sep 17 00:00:00 2001 From: Hongsheng Liu Date: Thu, 1 Oct 2026 17:18:54 -0700 Subject: [PATCH] fix: align migrated review helpers with repository lint policy --- .../review-pr/scripts/verify_line_numbers.py | 21 +++++++++++++++---- .../skills/review-pr/tests/test_helpers.py | 21 +++++++++---------- 2 files changed, 27 insertions(+), 15 deletions(-) mode change 100755 => 100644 .claude/skills/review-pr/scripts/verify_line_numbers.py diff --git a/.claude/skills/review-pr/scripts/verify_line_numbers.py b/.claude/skills/review-pr/scripts/verify_line_numbers.py old mode 100755 new mode 100644 index 86ddf8e57..1bec8b274 --- a/.claude/skills/review-pr/scripts/verify_line_numbers.py +++ b/.claude/skills/review-pr/scripts/verify_line_numbers.py @@ -6,11 +6,24 @@ import argparse import ast import json -import re import subprocess import sys -HUNK = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") + +def parse_hunk(text): + """Parse a unified-diff hunk header without a runtime dependency.""" + parts = text.split(" ", 4) + if len(parts) < 4 or parts[0] != "@@" or not parts[3].startswith("@@"): + return None + bounds = [] + for value, sign in zip(parts[1:3], ("-", "+")): + if not value.startswith(sign): + return None + numbers = value[1:].split(",") + if len(numbers) not in (1, 2) or not all(number.isdecimal() for number in numbers): + return None + bounds.extend((numbers[0], numbers[1] if len(numbers) == 2 else None)) + return tuple(bounds) def diff_path(value): @@ -29,8 +42,8 @@ def diff_lines(diff): if text.startswith("diff --git "): old_path = new_path = None old_remaining = new_remaining = 0 - elif match := HUNK.match(text): - old_line, old_count, new_line, new_count = match.groups() + elif hunk := parse_hunk(text): + old_line, old_count, new_line, new_count = hunk old_line, new_line = int(old_line), int(new_line) old_remaining = int(old_count) if old_count is not None else 1 new_remaining = int(new_count) if new_count is not None else 1 diff --git a/.claude/skills/review-pr/tests/test_helpers.py b/.claude/skills/review-pr/tests/test_helpers.py index 1c09bcbdd..4960572a9 100644 --- a/.claude/skills/review-pr/tests/test_helpers.py +++ b/.claude/skills/review-pr/tests/test_helpers.py @@ -60,7 +60,6 @@ else: check=False, ) - def test_select_filters_drafts_wip_and_own_prs(self): prs = [ {"number": i, "title": title, "author": {"login": author}, "isDraft": draft} @@ -113,17 +112,17 @@ else: self.assertNotEqual(result.returncode, 0) def test_verify_deleted_file_on_left(self): - diff = ( - "diff --git a/gone.py b/gone.py\n--- a/gone.py\n+++ /dev/null\n" - "@@ -1 +0,0 @@\n-old\n" - ) - result = self.verify( - [{"path": "gone.py", "line": 1, "side": "LEFT", "body": "Check"}], diff - ) + diff = "diff --git a/gone.py b/gone.py\n--- a/gone.py\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\n" + result = self.verify([{"path": "gone.py", "line": 1, "side": "LEFT", "body": "Check"}], diff) self.assertEqual(result.returncode, 0, result.stderr) - result = self.verify( - [{"path": "gone.py", "line": 1, "side": "RIGHT", "body": "Check"}], diff - ) + result = self.verify([{"path": "gone.py", "line": 1, "side": "RIGHT", "body": "Check"}], diff) + self.assertNotEqual(result.returncode, 0) + + def test_verify_new_file_with_explicit_zero_old_count(self): + diff = "diff --git a/new.py b/new.py\n--- /dev/null\n+++ b/new.py\n@@ -0,0 +1,2 @@\n+one\n+two\n" + result = self.verify([{"path": "new.py", "line": 2, "side": "RIGHT", "body": "Check"}], diff) + self.assertEqual(result.returncode, 0, result.stderr) + result = self.verify([{"path": "new.py", "line": 1, "side": "LEFT", "body": "Check"}], diff) self.assertNotEqual(result.returncode, 0) def test_verify_checks_multiline_comment_start(self):