From 3bce165d241cc991ded8a383164c23d196b72461 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 28 May 2026 12:11:25 -0700 Subject: [PATCH 1/2] fix(parsers/python): segment-match path exclusion/classification + resolve relative-import anchors Three independent defects in parsers/python/function_extractor.py: 1. extract_all(): the no-args scan excluded files with `any(excl in str(file_path) for excl in [...])` -- an unanchored substring test on the full path, so a file whose path merely contains a token ('myvenv/keep.py' contains 'venv') was silently dropped, and an ancestor directory containing a token could exclude the whole scan. Now matches whole path SEGMENTS: `{tokens} & set(file_path.relative_to(repo_path).parts)`. Python's own token set (__pycache__/.git/venv/.venv/node_modules) is preserved. 2. classify_function(): classification used `'' in path_lower` substring tests, so 'interviews/api.py' was classified 'view_function'. 'view_function' is in entry_point_detector.ENTRY_POINT_TYPES (:26-32), so that misclassification became a false entry-point seed that cascades into false reachability (consumed at entry_point_detector.py:177). The 'views' token now matches a whole path segment via a new _path_has_segment helper. The 'middleware' token is given the same segment fix because it shares the substring defect, but note 'middleware' (the python label) is NOT in ENTRY_POINT_TYPES -- so that half is classification accuracy, not a reachability change. The 'test' classifier is left as a substring on purpose (test-file conventions use 'tests/' and 'test_*'/'*_test' forms a segment match would miss; 'test' is not an entry-point type, so it seeds no false reachability). 3. extract_imports(): the ast.ImportFrom branch read node.module but never node.level, so relative imports lost their package anchor ('from . import X' stored bare 'X'; 'from ..pkg import Y' stored anchor-less 'pkg.Y'). call_graph_builder._resolve_import then rebuilt a wrong/no file path and the edges were dropped (verified: pre-fix the candidate resolves to None, post-fix it resolves to the real pkg/sub/helpers.py). Now reconstructs the absolute anchor from the importing file's package location (level=1 -> own package, level=2 -> parent, ...); over-deep levels degrade to no leading dot. Absolute imports (level=0) are unchanged. Scope: the php/ruby function_extractor.py extract_all + classify siblings carry related defects and are not widened here. Tests: tests/test_python_function_extractor.py -- loads the module under a unique importlib name (the bare 'function_extractor' name is shared by five other parsers, so a plain import would pollute sys.modules for the rest of the suite). Three checks: segment-vs-substring exclusion, entry-point classification by segment, and relative-import anchor reconstruction. RED 3 failed (pre-fix) -> GREEN 3 passed; full suite 179 passed / 63 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../parsers/python/function_extractor.py | 47 +++++++++-- .../tests/test_python_function_extractor.py | 81 +++++++++++++++++++ 2 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 libs/openant-core/tests/test_python_function_extractor.py diff --git a/libs/openant-core/parsers/python/function_extractor.py b/libs/openant-core/parsers/python/function_extractor.py index 8714e9dc..37475e57 100644 --- a/libs/openant-core/parsers/python/function_extractor.py +++ b/libs/openant-core/parsers/python/function_extractor.py @@ -194,6 +194,19 @@ def get_docstring(self, node: ast.AST) -> Optional[str]: """Extract docstring from a function or class.""" return ast.get_docstring(node) + def _path_has_segment(self, file_path: str, token: str) -> bool: + """True if `token` equals a whole path segment (a directory name or the filename stem), + case-insensitively -- used instead of a bare ``token in path`` substring test so that, for + example, 'views' classifies ``app/views.py`` and ``app/views/x.py`` but NOT ``interviews/a.py`` + or ``app/previews/b.py``.""" + p = Path(file_path) + try: + p = p.relative_to(self.repo_path) + except ValueError: + pass + segments = {s.lower() for s in p.with_suffix('').parts} + return token.lower() in segments + def classify_function(self, func_name: str, decorators: List[str], class_name: Optional[str], file_path: str) -> str: """Classify a function by its type/purpose.""" @@ -207,7 +220,7 @@ def classify_function(self, func_name: str, decorators: List[str], return 'route_handler' # Django views - if 'views' in path_lower and class_name is None: + if self._path_has_segment(file_path, 'views') and class_name is None: return 'view_function' # Class methods @@ -225,10 +238,15 @@ def classify_function(self, func_name: str, decorators: List[str], return 'method' # Middleware/decorators - if 'middleware' in func_name.lower() or 'middleware' in path_lower: + if 'middleware' in func_name.lower() or self._path_has_segment(file_path, 'middleware'): return 'middleware' - # Test functions + # Test functions. + # 'test' is matched as a substring here on purpose: test-file conventions use plural/affixed + # forms ('tests/' dir, 'test_*'/'*_test' files) that a whole-segment match would miss. The + # substring over-match (e.g. 'latest'/'contest') is the is_test_file family handled in its own + # scheduled units -- 'test' is NOT an entry-point type, so unlike 'views' it seeds no false + # reachability and is intentionally left as a substring test in this fix. if func_name.startswith('test_') or 'test' in path_lower: return 'test' @@ -261,9 +279,21 @@ def extract_imports(self, tree: ast.AST, file_path: str) -> Dict[str, str]: imports[name] = alias.name elif isinstance(node, ast.ImportFrom): module = node.module or '' + level = node.level or 0 + if level > 0: + # Relative import: reconstruct the absolute package anchor from the importing + # file's location so the dotted path resolves to a real module. file_path is + # repo-relative (e.g. 'pkg/sub/mod.py'); its package is the directory parts. + # level=1 -> the file's own package, level=2 -> the parent package, etc. + pkg_parts = list(Path(file_path).parts[:-1]) + keep = max(0, len(pkg_parts) - (level - 1)) + anchor = pkg_parts[:keep] + base_parts = anchor + ([module] if module else []) + else: + base_parts = [module] if module else [] for alias in node.names: name = alias.asname or alias.name - full_path = f"{module}.{alias.name}" if module else alias.name + full_path = '.'.join(base_parts + [alias.name]) if base_parts else alias.name imports[name] = full_path return imports @@ -552,9 +582,12 @@ def extract_all(self, files: Optional[List[str]] = None) -> Dict: else: # Scan all .py files for file_path in self.repo_path.rglob('*.py'): - # Skip common exclude patterns - path_str = str(file_path) - if any(excl in path_str for excl in ['__pycache__', '.git', 'venv', '.venv', 'node_modules']): + # Skip common excluded directories. Match whole path SEGMENTS (not a substring of the + # full path) so e.g. 'venv' excludes a real venv/ dir but not 'myvenv/keep.py', and an + # ancestor dir whose name contains a token cannot poison the whole scan. rglob yields + # paths under repo_path, so relative_to never raises. + excluded = {'__pycache__', '.git', 'venv', '.venv', 'node_modules'} + if excluded & set(file_path.relative_to(self.repo_path).parts): continue self.process_file(file_path) diff --git a/libs/openant-core/tests/test_python_function_extractor.py b/libs/openant-core/tests/test_python_function_extractor.py new file mode 100644 index 00000000..c5c0d69c --- /dev/null +++ b/libs/openant-core/tests/test_python_function_extractor.py @@ -0,0 +1,81 @@ +"""Regression tests for three independent defects in parsers/python/function_extractor.py. + +Segment-vs-substring exclusion: extract_all() no-args branch excludes files via an UNANCHORED + substring test (`any(excl in str(file_path) ...)`), so a file whose path merely contains a token + ('myvenv/keep.py' contains 'venv') is wrongly skipped. Fix: match whole path SEGMENTS. +Entry-point classification: classify_function uses `'' in path_lower` substring tests to + assign ENTRY-POINT unit_types, so 'interviews/api.py' is classified 'view_function' (a false + reachability seed). Fix: match the 'views'/'middleware' tokens as whole path segments. ('test' is + intentionally left as a substring -- see the in-code note; it is not an entry-point type.) +Relative-import anchor: extract_imports ignores ast.ImportFrom.level, so relative imports lose their + package anchor ('from . import X' -> bare 'X'). Fix: reconstruct the absolute anchor from the + importing file's location. + +Loads function_extractor under a UNIQUE module name (not the bare 'function_extractor', which the c/ +go/php/ruby/zig parsers also ship) so a bare import cannot pollute sys.modules for sibling tests. +""" +import ast +import importlib.util +import sys +from pathlib import Path + +CORE = Path(__file__).resolve().parents[1] # libs/openant-core +if str(CORE) not in sys.path: + sys.path.insert(0, str(CORE)) # for utilities.* if imported + +_spec = importlib.util.spec_from_file_location( + "py_function_extractor_isolated", str(CORE / "parsers" / "python" / "function_extractor.py")) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) +FunctionExtractor = _mod.FunctionExtractor + + +# ---- extract_all excludes by path segment, not substring ---- +def test_extract_all_excludes_on_path_segments_not_substring(tmp_path, monkeypatch): + repo = tmp_path / "repo" + for d in ("myvenv", "venv", ".git", "pkg/__pycache__", "src"): + (repo / d).mkdir(parents=True, exist_ok=True) + (repo / "myvenv" / "keep.py").write_text("def f(): pass\n") # 'venv' substring -> wrongly skipped pre-fix + (repo / "src" / "clean.py").write_text("def g(): pass\n") + (repo / "venv" / "skip.py").write_text("def h(): pass\n") # real venv/ -> excluded + (repo / ".git" / "hook.py").write_text("def i(): pass\n") # .git -> excluded + (repo / "pkg" / "__pycache__" / "c.py").write_text("def j(): pass\n") # __pycache__ -> excluded + + ex = FunctionExtractor(str(repo)) + processed = [] + monkeypatch.setattr(ex, "process_file", + lambda fp: processed.append(str(Path(fp).relative_to(ex.repo_path)))) + ex.extract_all() + seen = set(processed) + + assert "myvenv/keep.py" in seen, f"'myvenv' wrongly excluded by 'venv' substring: {sorted(seen)}" + assert "src/clean.py" in seen + assert "venv/skip.py" not in seen, "a real venv/ directory must stay excluded" + assert ".git/hook.py" not in seen, ".git must stay excluded" + assert "pkg/__pycache__/c.py" not in seen, "__pycache__ must stay excluded" + + +# ---- classify_function matches entry-point tokens by segment, not substring ---- +def test_classify_function_entrypoint_tokens_match_segments_not_substring(tmp_path): + ex = FunctionExtractor(str(tmp_path)) + c = lambda path: ex.classify_function("handler", [], None, path) + + # genuine segments still classify (no regression) + assert c("app/views.py") == "view_function" + assert c("app/views/handlers.py") == "view_function" + assert c("app/middleware/auth.py") == "middleware" + # substring over-matches must NOT seed entry-point types + assert c("interviews/api.py") != "view_function", "'interviews' wrongly matched 'views'" + assert c("app/previews/x.py") != "view_function", "'previews' wrongly matched 'views'" + assert c("app/previewmiddleware.py") != "middleware", "'previewmiddleware' wrongly matched 'middleware'" + + +# ---- extract_imports preserves the relative-import package anchor (node.level) ---- +def test_extract_imports_preserves_relative_package_anchor(tmp_path): + ex = FunctionExtractor(str(tmp_path)) + src = "from . import helpers\nfrom ..util import U\nfrom os import path\n" + imports = ex.extract_imports(ast.parse(src), "pkg/sub/mod.py") + + assert imports["helpers"] == "pkg.sub.helpers", f"relative 'from . import' lost anchor: {imports}" + assert imports["U"] == "pkg.util.U", f"relative 'from ..util import' lost anchor: {imports}" + assert imports["path"] == "os.path", f"absolute import must be unchanged: {imports}" From 85addd352c935dbef5c02c04f08ff7f10f369001 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 28 May 2026 17:25:49 -0700 Subject: [PATCH 2/2] test(parsers/python): normalize recorded path to forward slashes for Windows CI The segment-vs-substring exclusion regression test recorded the processed path via str(Path.relative_to(...)), which yields backslash separators on Windows and fails the forward-slash 'in seen' assertions. Use .as_posix() so the comparison is OS-independent. The substring-over-exclusion assertions are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- libs/openant-core/tests/test_python_function_extractor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/openant-core/tests/test_python_function_extractor.py b/libs/openant-core/tests/test_python_function_extractor.py index c5c0d69c..43d2908e 100644 --- a/libs/openant-core/tests/test_python_function_extractor.py +++ b/libs/openant-core/tests/test_python_function_extractor.py @@ -44,7 +44,7 @@ def test_extract_all_excludes_on_path_segments_not_substring(tmp_path, monkeypat ex = FunctionExtractor(str(repo)) processed = [] monkeypatch.setattr(ex, "process_file", - lambda fp: processed.append(str(Path(fp).relative_to(ex.repo_path)))) + lambda fp: processed.append(Path(fp).relative_to(ex.repo_path).as_posix())) ex.extract_all() seen = set(processed)