From 865ef37f14cf8c9ca8b48bcb057f3186361a9830 Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Fri, 16 Oct 2026 22:24:11 +0200 Subject: [PATCH] fix(code_review): use regex matching and AST loop inspection in check_performance_issues (ticket-009) --- .governance/standard-adoption.json | 530 ++++++++++++++++++++++++++++- code2logic/code_review.py | 150 +++++++- project/TICKETS.md | 2 + project/ticket-009/README.md | 22 ++ project/ticket-009/intent.json | 25 ++ tests/test_code_review.py | 69 ++++ 6 files changed, 785 insertions(+), 13 deletions(-) create mode 100644 project/ticket-009/README.md create mode 100644 project/ticket-009/intent.json create mode 100644 tests/test_code_review.py diff --git a/.governance/standard-adoption.json b/.governance/standard-adoption.json index 04c8062..3f2b5fe 100644 --- a/.governance/standard-adoption.json +++ b/.governance/standard-adoption.json @@ -2,12 +2,538 @@ "schema": "wellmanifest.standard-adoption/v1", "mode": "audit", "profile": "baseline", - "repositoryRole": "unclassified", + "repositoryRole": "baseline", "updates": { "enabled": true, "trigger": "pre-commit", "action": "prepare-and-abort", "executor": "goal" }, - "adoptions": [] + "adoptions": [ + { + "id": "wellmanifest/authority-lifecycle", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "runtime-conformance", + "level": "S5", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S5", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/code-dsl", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S4", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/deployment", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "runtime-conformance", + "level": "S5", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S5", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/docs", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S3", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/git-lifecycle", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S4", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/llm", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S3", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/logs", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "runtime-conformance", + "level": "S5", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S5", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/merge", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "runtime-conformance", + "level": "S5", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S5", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/new-project", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S4", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/poa", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "runtime-conformance", + "level": "S5", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S5", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/ticket-lifecycle", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S3", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/validation-attestation", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S4", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S4", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + }, + { + "id": "wellmanifest/worktrees", + "version": "0.20.92", + "revision": "fcbb13e0ae04c688d825d5a87e2d2721dfab483d", + "model": "protected-conformance", + "level": "S3", + "artifacts": [ + { + "target": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ], + "evidence": [ + { + "level": "S0", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S1", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S2", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + }, + { + "level": "S3", + "uri": ".governance/manifest.json", + "sha256": "3a63e819e01eeca10f9333c3286054168ed00eac6081a239d78994d8d7574319" + } + ] + } + ] } diff --git a/code2logic/code_review.py b/code2logic/code_review.py index 335ffb1..7a3fe8d 100644 --- a/code2logic/code_review.py +++ b/code2logic/code_review.py @@ -10,7 +10,10 @@ from code2logic.code_review import CodeReviewer, analyze_code_quality """ +import ast +import re from collections import defaultdict +from pathlib import Path from typing import Any, Dict, List # Security patterns to check @@ -24,11 +27,42 @@ # Performance anti-patterns PERFORMANCE_PATTERNS = { - "n_plus_one": ["for.*in.*:", ".all()", ".filter("], + "n_plus_one": ["for.*in.*:", ".all()", ".filter(", ".execute(", "sqlite3.connect("], + "subprocess_in_loop": [ + "subprocess.run", + "subprocess.Popen", + "subprocess.check_output", + "subprocess.call", + "run_process", + ], + "uncached_parse_in_loop": ["yaml.safe_load", "yaml.load", "ast.parse"], "large_memory": ["readlines()", "list(", ".to_list()"], "blocking_io": ["requests.get", "urllib.urlopen", "open("], } +_REGEX_META = re.compile(r"(\.\*|\\[bBsSwWdD]|[\^\$\[\]\|\+\?])") +_LOOP_ONLY_CATEGORIES = {"subprocess_in_loop", "uncached_parse_in_loop"} +_AST_LOOP_TYPES = ( + ast.For, + ast.AsyncFor, + ast.While, + ast.ListComp, + ast.SetComp, + ast.DictComp, + ast.GeneratorExp, +) + + +def _pattern_matches(pattern: str, text: str) -> bool: + """Match pattern against text using regex when regex metacharacters are present.""" + if _REGEX_META.search(pattern): + try: + return bool(re.search(pattern, text, re.IGNORECASE)) + except re.error: + pass + return pattern.lower() in text.lower() + + # Thresholds COMPLEXITY_HIGH = 15 COMPLEXITY_MEDIUM = 10 @@ -161,6 +195,81 @@ def check_security_issues(project) -> Dict[str, List[Dict]]: return dict(issues) +def _call_name_from_ast(node: ast.Call) -> str: + if isinstance(node.func, ast.Name): + return f"{node.func.id}(" + if isinstance(node.func, ast.Attribute): + if isinstance(node.func.value, ast.Name): + return f"{node.func.value.id}.{node.func.attr}(" + return f".{node.func.attr}(" + return "" + + +def _resolve_module_source(project, module) -> str: + source = getattr(module, "source", None) or getattr(module, "content", None) + if isinstance(source, str) and source: + return source + mod_path = getattr(module, "path", "") + if not mod_path: + return "" + candidates = [Path(mod_path)] + root_path = getattr(project, "root_path", "") + if root_path: + candidates.insert(0, Path(root_path) / mod_path) + for candidate in candidates: + try: + if candidate.is_file(): + return candidate.read_text(encoding="utf-8") + except (OSError, UnicodeError): + continue + return "" + + +def _scan_ast_loops(module_path: str, source: str, issues: Dict[str, List[Dict]], seen: set) -> None: + try: + tree = ast.parse(source, filename=module_path) + except (SyntaxError, ValueError, RecursionError): + return + + def record(category: str, func_name: str, call_label: str) -> None: + key = (category, module_path, func_name, call_label) + if key not in seen: + seen.add(key) + issues[category].append( + { + "path": module_path, + "name": func_name, + "call": call_label, + "severity": "medium", + } + ) + + def walk(node: ast.AST, func_name: str, in_loop: bool) -> None: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + next_name = f"{func_name}.{node.name}" if func_name != "" else node.name + for child in ast.iter_child_nodes(node): + walk(child, next_name, False) + return + if isinstance(node, ast.ClassDef): + next_name = node.name + for child in ast.iter_child_nodes(node): + walk(child, next_name, False) + return + if isinstance(node, ast.Call) and in_loop: + call_str = _call_name_from_ast(node) + if call_str: + for category in ("n_plus_one", "subprocess_in_loop", "uncached_parse_in_loop"): + for pattern in PERFORMANCE_PATTERNS.get(category, []): + if _pattern_matches(pattern, call_str): + record(category, func_name, call_str.rstrip("(")) + break + next_loop = in_loop or isinstance(node, _AST_LOOP_TYPES) + for child in ast.iter_child_nodes(node): + walk(child, func_name, next_loop) + + walk(tree, "", False) + + def check_performance_issues(project) -> Dict[str, List[Dict]]: """Check for performance anti-patterns. @@ -171,25 +280,44 @@ def check_performance_issues(project) -> Dict[str, List[Dict]]: Dictionary of performance issues """ issues = defaultdict(list) + seen = set() for module in project.modules: - for func in module.functions: + source = _resolve_module_source(project, module) + has_ast = False + if source and (getattr(module, "language", "python") == "python" or str(module.path).endswith(".py")): + _scan_ast_loops(module.path, source, issues, seen) + has_ast = True + + funcs = list(getattr(module, "functions", [])) + for cls in getattr(module, "classes", []): + for method in getattr(cls, "methods", []): + funcs.append(method) + + for func in funcs: for call in func.calls: for category, patterns in PERFORMANCE_PATTERNS.items(): + if category in _LOOP_ONLY_CATEGORIES and has_ast: + continue for pattern in patterns: - if pattern.lower() in call.lower(): - issues[category].append( - { - "path": module.path, - "name": func.name, - "call": call, - "severity": "medium", - } - ) + if _pattern_matches(pattern, call): + key = (category, module.path, func.name, call) + if key not in seen: + seen.add(key) + issues[category].append( + { + "path": module.path, + "name": func.name, + "call": call, + "severity": "medium", + } + ) + break return dict(issues) + class CodeReviewer: """Automated code review with optional LLM enhancement.""" diff --git a/project/TICKETS.md b/project/TICKETS.md index b052bc2..0847486 100644 --- a/project/TICKETS.md +++ b/project/TICKETS.md @@ -9,4 +9,6 @@ | **ticket-005** | [`README.md`](./ticket-005/README.md) | - | - | - | - | - | | **ticket-006** | [`README.md`](./ticket-006/README.md) | - | - | - | - | - | | **ticket-007** | [`README.md`](./ticket-007/README.md) | - | - | - | - | - | +| **ticket-008** | [`README.md`](./ticket-008/README.md) | - | - | - | - | - | +| **ticket-009** | [`README.md`](./ticket-009/README.md) | - | - | - | - | - | diff --git a/project/ticket-009/README.md b/project/ticket-009/README.md new file mode 100644 index 0000000..076781b --- /dev/null +++ b/project/ticket-009/README.md @@ -0,0 +1,22 @@ +# Ticket 009: Fix check_performance_issues regex and AST loop detection + +- **ID**: ticket-009 +- **Owner**: unresolved:human +- **Status**: IN_PROGRESS +- **Workflow state**: EDIT +- **Created**: 2026-10-16 + +## Goal and scope + +Fix `check_performance_issues` in `code2logic/code_review.py` so regex patterns (such as `for.*in.*:`) are evaluated with `re.search` instead of literal substring checks, and inspect Python AST loop context to detect N+1 queries, subprocess calls in loops, and uncached YAML/AST parsing in loops. + +## Acceptance criteria + +- [x] AC-01: Update `code2logic/code_review.py` to support regex pattern matching and AST loop inspection for N+1 queries, subprocesses in loops, and uncached parsing in loops. +- [x] AC-02: Add unit tests in `tests/test_code_review.py`. +- [x] AC-03: Governance check (`./project/governance-check.sh`) and test suite pass. + +## Tracking boundary + +This directory contains the minimal reviewed intent. Optional participant prose +and raw command logs are not required delivery output. diff --git a/project/ticket-009/intent.json b/project/ticket-009/intent.json new file mode 100644 index 0000000..56b8ac0 --- /dev/null +++ b/project/ticket-009/intent.json @@ -0,0 +1,25 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-009", + "summary": "Fix check_performance_issues regex and AST loop detection", + "workstream": "application", + "classification": { + "kind": "SERVICE", + "priority": "P2", + "origin": "health" + }, + "allowedPaths": [ + "project/ticket-009/**", + "TODO.md", + "project/TICKETS.md", + "code2logic/code_review.py", + "tests/test_code_review.py" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null +} diff --git a/tests/test_code_review.py b/tests/test_code_review.py new file mode 100644 index 0000000..02d2676 --- /dev/null +++ b/tests/test_code_review.py @@ -0,0 +1,69 @@ +"""Unit tests for code2logic.code_review performance anti-pattern detection.""" + +import tempfile +from pathlib import Path + +from code2logic.code_review import _pattern_matches, check_performance_issues +from code2logic.models import FunctionInfo, ModuleInfo, ProjectInfo + + +def _make_project(root_path: str, modules: list[ModuleInfo]) -> ProjectInfo: + return ProjectInfo( + name="test-project", + root_path=root_path, + languages={"python": len(modules)}, + modules=modules, + dependency_graph={}, + dependency_metrics={}, + entrypoints=[], + similar_functions={}, + total_files=len(modules), + total_lines=100, + ) + + +def test_pattern_matches_regex_and_literal(): + assert _pattern_matches("for.*in.*:", "for item in items:") + assert not _pattern_matches("for.*in.*:", "format_in_place") + assert _pattern_matches(".filter(", "User.objects.filter(id=1)") + assert _pattern_matches("list(", "list(items)") + + +def test_check_performance_issues_ast_loops(): + source = ( + "import ast\n" + "import sqlite3\n" + "import subprocess\n" + "import yaml\n\n" + "def process_items(db_path, paths):\n" + " conn = sqlite3.connect(db_path)\n" + " for p in paths:\n" + " conn.execute('SELECT * FROM t WHERE path = ?', (str(p),))\n" + " subprocess.run(['git', 'log', str(p)], check=False)\n" + " yaml.safe_load(p.read_text())\n" + " return [ast.parse(p.read_text()) for p in paths]\n" + ) + with tempfile.TemporaryDirectory() as tmpdir: + mod_file = Path(tmpdir) / "worker.py" + mod_file.write_text(source, encoding="utf-8") + module = ModuleInfo( + path="worker.py", + language="python", + functions=[ + FunctionInfo( + name="process_items", + calls=["sqlite3.connect", "conn.execute", "subprocess.run", "yaml.safe_load", "ast.parse"], + ) + ], + ) + project = _make_project(tmpdir, [module]) + issues = check_performance_issues(project) + + assert "n_plus_one" in issues + assert any(i["name"] == "process_items" and "execute" in i["call"] for i in issues["n_plus_one"]) + assert "subprocess_in_loop" in issues + assert any(i["name"] == "process_items" and "subprocess.run" in i["call"] for i in issues["subprocess_in_loop"]) + assert "uncached_parse_in_loop" in issues + calls = {i["call"] for i in issues["uncached_parse_in_loop"]} + assert "yaml.safe_load" in calls + assert "ast.parse" in calls