Coverage gate fails a judge hook whose tests skip the shared base

scripts/check_hook_test_coverage.py

@@ -161,6 +161,76 @@ def hooks_with_detector() -> list[str]:
     return found
 
 
+def _parse_python_files(folder: str, skip_tests: bool) -> tuple[list[ast.AST], list[str]]:
+    trees: list[ast.AST] = []
+    unreadable: list[str] = []
+    for root, dirs, files in os.walk(folder):
+        if skip_tests:
+            dirs[:] = [directory for directory in dirs if directory != "tests"]
+        for filename in sorted(files):
+            if not filename.endswith(".py"):
+                continue
+            path = os.path.join(root, filename)
+            try:
+                with open(path, encoding="utf-8") as handle:
+                    trees.append(ast.parse(handle.read(), filename=path))
+            except (OSError, SyntaxError, UnicodeDecodeError) as error:
+                unreadable.append(f"{path} ({type(error).__name__}: {error})")
+    return trees, unreadable
+
+
+def _imports_judge(tree: ast.AST) -> bool:
+    for node in ast.walk(tree):
+        if isinstance(node, ast.Import) and any(
+            alias.name == "judge" or alias.name.endswith(".judge") for alias in node.names
+        ):
+            return True
+        if isinstance(node, ast.ImportFrom) and node.module and (
+            node.module == "judge" or node.module.endswith(".judge")
+        ):
+            return True
+    return False
+
+
+def _uses_judge_test_base(tree: ast.AST) -> bool:
+    for node in ast.walk(tree):
+        if isinstance(node, ast.ImportFrom) and any(alias.name == "JudgeTestCase" for alias in node.names):
+            return True
+        if isinstance(node, ast.ClassDef) and any(
+            (isinstance(base, ast.Name) and base.id == "JudgeTestCase")
+            or (isinstance(base, ast.Attribute) and base.attr == "JudgeTestCase")
+            for base in node.bases
+        ):
+            return True
+    return False
+
+
+def judge_isolation_problems(hook_dir: str) -> list[str]:
+    name = os.path.basename(hook_dir)
+    sources, unreadable = _parse_python_files(hook_dir, skip_tests=True)
+    problems = [f"{name}: could not read {path}, so its judge imports are unchecked" for path in unreadable]
+    if not any(_imports_judge(tree) for tree in sources):
+        return problems
+    tests_dir = os.path.join(hook_dir, "tests")
+    if not os.path.isdir(tests_dir):
+        return problems + [f"{name}: imports llm-judge but has no tests/ dir using JudgeTestCase"]
+    tests, unreadable_tests = _parse_python_files(tests_dir, skip_tests=False)
+    problems += [f"{name}: could not read {path}, so its JudgeTestCase use is unchecked" for path in unreadable_tests]
+    if not any(_uses_judge_test_base(tree) for tree in tests):
+        problems.append(f"{name}: imports llm-judge but no test imports or subclasses JudgeTestCase")
+    return problems
+
+
+def hook_dirs() -> list[str]:
+    if not os.path.isdir(HOOKS_DIR):
+        raise FileNotFoundError(f"hooks folder not found, so no hook's judge isolation was checked: {HOOKS_DIR}")
+    return [
+        os.path.join(HOOKS_DIR, name)
+        for name in sorted(os.listdir(HOOKS_DIR))
+        if os.path.isdir(os.path.join(HOOKS_DIR, name))
+    ]
+
+
 def check_hook(hook_dir: str) -> list[str]:
     """Return a list of problems for this hook, empty if it passes."""
     name = os.path.basename(hook_dir)
@@ -203,9 +273,10 @@ def main() -> int:
 
     all_problems: list[str] = []
     for hook_dir in targets:
-        if not os.path.isfile(os.path.join(hook_dir, "detect.py")):
-            continue
-        all_problems.extend(check_hook(hook_dir))
+        if os.path.isfile(os.path.join(hook_dir, "detect.py")):
+            all_problems.extend(check_hook(hook_dir))
+    for hook_dir in [os.path.abspath(a) for a in args] if args else hook_dirs():
+        all_problems.extend(judge_isolation_problems(hook_dir))
 
     if all_problems:
         print("check_hook_test_coverage: FAIL")

tests/test_check_hook_test_coverage.py

@@ -12,6 +12,7 @@
 import tempfile
 import unittest
 from pathlib import Path
+from unittest.mock import patch
 
 REPO = Path(__file__).resolve().parents[1]
 sys.path.insert(0, str(REPO / "scripts"))
@@ -97,3 +98,51 @@ def test_the_real_repo_passes_this_gate(self):
 
 if __name__ == "__main__":
     unittest.main()
+
+
+JUDGE_CALLER = "def ask(text):\n    import judge\n    return judge.ask(text)\n"
+USES_BASE = "from testing import JudgeTestCase\n\n\nclass TestCaller(JudgeTestCase):\n    pass\n"
+NAMES_BASE_IN_COMMENT = "import unittest\n\n\nclass TestCaller(unittest.TestCase):\n    pass  # JudgeTestCase\n"
+
+
+def judge_hook(root: Path, source: str, tests: str | None) -> Path:
+    hook_dir = root / "caller"
+    hook_dir.mkdir()
+    (hook_dir / "caller.py").write_text(source, encoding="utf-8")
+    if tests is not None:
+        (hook_dir / "tests").mkdir()
+        (hook_dir / "tests" / "test_caller.py").write_text(tests, encoding="utf-8")
+    return hook_dir
+
+
+class TestJudgeIsolationRule(unittest.TestCase):
+    def test_hit_judge_caller_whose_tests_skip_the_base_fails(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            problems = chtc.judge_isolation_problems(str(judge_hook(Path(tmp), JUDGE_CALLER, FIRES_AND_SILENT)))
+        self.assertEqual(len(problems), 1, problems)
+        self.assertIn("JudgeTestCase", problems[0])
+
+    def test_hit_base_named_only_in_a_comment_still_fails(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            problems = chtc.judge_isolation_problems(str(judge_hook(Path(tmp), JUDGE_CALLER, NAMES_BASE_IN_COMMENT)))
+        self.assertEqual(len(problems), 1, problems)
+
+    def test_no_hit_judge_caller_whose_tests_subclass_the_base_passes(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            self.assertEqual(chtc.judge_isolation_problems(str(judge_hook(Path(tmp), JUDGE_CALLER, USES_BASE))), [])
+
+    def test_no_hit_hook_that_never_imports_judge_is_not_asked_for_the_base(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            self.assertEqual(chtc.judge_isolation_problems(str(judge_hook(Path(tmp), INLINE_DETECTOR, FIRES_AND_SILENT))), [])
+
+    def test_unreadable_source_is_reported_as_unchecked_not_clean(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            problems = chtc.judge_isolation_problems(str(judge_hook(Path(tmp), "def broken(:\n", USES_BASE)))
+        self.assertEqual(len(problems), 1, problems)
+        self.assertIn("could not read", problems[0])
+
+    def test_hit_hook_without_detect_py_is_still_checked_by_main(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            hook_dir = judge_hook(Path(tmp), JUDGE_CALLER, FIRES_AND_SILENT)
+            with patch.object(sys, "argv", ["check_hook_test_coverage.py", str(hook_dir)]):
+                self.assertEqual(chtc.main(), 1)