From c113a3a1922787debc8ff6c667da0a9ba8784b02 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 16 Aug 2026 17:48:32 +0200 Subject: [PATCH] Split the code_tutor import graph and scan tables out of _analyzer _analyzer.py 388 -> 226. The file walk, dotted-name map, import resolution and fingerprint go to _deps.py; the skip frozensets and the function regex to _scan_tables.py, which covered the last nine lines after the first cut landed at 259. _is_binary is the only name any test patches on _analyzer and it stays put, so no patch string needed editing. Only test_analyzer.py's import block had to be split. Pass count is 216 before and after. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY --- .../code-tutor-analyzer-split-2026-08-16.json | 19 ++ .../code-tutor-analyzer-split-2026-08-16.json | 51 ++++++ python_pkg/code_tutor/_analyzer.py | 172 +----------------- python_pkg/code_tutor/_deps.py | 145 +++++++++++++++ python_pkg/code_tutor/_plan_builder.py | 7 +- python_pkg/code_tutor/_scan_tables.py | 48 +++++ python_pkg/code_tutor/cli.py | 3 +- python_pkg/code_tutor/tests/test_analyzer.py | 8 +- 8 files changed, 280 insertions(+), 173 deletions(-) create mode 100644 docs/superpowers/contracts/code-tutor-analyzer-split-2026-08-16.json create mode 100644 docs/superpowers/evidence/code-tutor-analyzer-split-2026-08-16.json create mode 100644 python_pkg/code_tutor/_deps.py create mode 100644 python_pkg/code_tutor/_scan_tables.py diff --git a/docs/superpowers/contracts/code-tutor-analyzer-split-2026-08-16.json b/docs/superpowers/contracts/code-tutor-analyzer-split-2026-08-16.json new file mode 100644 index 00000000..3529a65d --- /dev/null +++ b/docs/superpowers/contracts/code-tutor-analyzer-split-2026-08-16.json @@ -0,0 +1,19 @@ +{ + "title": "Split code_tutor/_analyzer.py under the 250-line cap", + "objective": "Bring python_pkg/code_tutor/_analyzer.py (388 lines) under the repo-wide 250-line cap by moving the file-walking and import-graph half into _deps.py and the static scan tables into _scan_tables.py.", + "acceptance_criteria": [ + "_analyzer.py, _deps.py and _scan_tables.py are each at most 250 lines.", + "The code_tutor suite still reports exactly 216 passing tests -- the same count as before the split, proving no test was dropped or silently skipped.", + "The package keeps 100% branch coverage of the code_tutor source modules.", + "_is_binary, the only name any test patches on _analyzer, stays defined there so that patch string needs no edit.", + "Every importer of the moved functions -- cli.py, _plan_builder.py and test_analyzer.py -- names _deps directly rather than reaching them through an _analyzer re-export.", + "No assertion is changed and no test is deleted, skipped or weakened.", + "No suppression is added and no coverage exclusion is introduced." + ], + "out_of_scope": [ + "Changing what any test asserts, or which paths the analyzer skips.", + "Splitting cli.py, _verifier.py, _llm.py or the code_tutor test modules, which are separate commits.", + "Adding a re-export shim in _analyzer." + ], + "verifier": "timeout 90 env PYTHONPATH=. python3 -m pytest python_pkg/code_tutor/tests -q (expects exactly 216 passed), plus an import-time assertion that every patched name resolves, plus pre-commit run --files on the touched files." +} diff --git a/docs/superpowers/evidence/code-tutor-analyzer-split-2026-08-16.json b/docs/superpowers/evidence/code-tutor-analyzer-split-2026-08-16.json new file mode 100644 index 00000000..dbe3ac00 --- /dev/null +++ b/docs/superpowers/evidence/code-tutor-analyzer-split-2026-08-16.json @@ -0,0 +1,51 @@ +{ + "intent": "Bring code_tutor/_analyzer.py under the 250-line cap by separating the import-graph half and the static scan tables, with no behaviour change.", + "scope": [ + "python_pkg/code_tutor/_deps.py, _scan_tables.py (new)", + "python_pkg/code_tutor/_analyzer.py (388 -> 226)", + "python_pkg/code_tutor/cli.py, _plan_builder.py (import paths only)", + "python_pkg/code_tutor/tests/test_analyzer.py (import path only)", + "Non-goal: no assertion changed, no test deleted, skipped or weakened" + ], + "changes": [ + "Moved get_python_files, _build_dotted_map, _match_deps, get_file_imports and codebase_fingerprint into _deps.py, which imports _should_skip back from _analyzer and owns the hashlib import it was the only user of.", + "Moved the _SKIP_DIRS, _SKIP_SUFFIXES and _OTHER_LANGS frozensets and the _FUNCTION_RE pattern into _scan_tables.py. The first cut left _analyzer.py at 259, nine over; these tables are pure data with no patch targets, which made them the cheapest remaining lines.", + "cli.py and _plan_builder.py import the moved functions from _deps directly. test_analyzer.py's single import block was split across the two modules.", + "_is_binary is the only name any test patches on _analyzer, and it stays there, so no patch string in the package needed editing - a much cheaper seam than the _challenge one." + ], + "verification": [ + { + "command": "timeout 90 env PYTHONPATH=. python3 -m pytest python_pkg/code_tutor/tests -q", + "result": "pass", + "evidence": "216 passed in 2.2s - identical to the baseline count, and inside the timeout that the previous commit's hang made standard here." + }, + { + "command": "timeout 90 env PYTHONPATH=. python3 -m pytest python_pkg/code_tutor/tests -q (first run)", + "result": "fail then pass", + "evidence": "One collection ImportError: test_analyzer.py imports _build_dotted_map and four other moved names from _analyzer. Its import block was split across the two modules and the run returned to 216." + }, + { + "command": "PYTHONPATH=. python3 -c \"from python_pkg.code_tutor import _analyzer as a, _deps as d, cli, _plan_builder as pb, _scan_tables as t; a._is_binary; a._SKIP_DIRS; a._FUNCTION_RE; d.get_file_imports; d._should_skip; d.hashlib; cli.codebase_fingerprint; pb.get_python_files\"", + "result": "pass", + "evidence": "all patch targets resolve - including cli.codebase_fingerprint, which three test_cli.py patches name and which still resolves because cli imports it from its new home." + }, + { + "command": "wc -l python_pkg/code_tutor/_analyzer.py python_pkg/code_tutor/_deps.py python_pkg/code_tutor/_scan_tables.py", + "result": "pass", + "evidence": "226, 145 and 48 lines, all under the 250-line cap." + }, + { + "command": "pre-commit run --files ", + "result": "pass", + "evidence": "ruff auto-sorted six import statements across the new and edited modules; clean on re-run, and pytest-with-coverage passes." + } + ], + "risks": [ + "_analyzer <- _deps and _analyzer <- _scan_tables must stay one-way; _analyzer importing from _deps would be a cycle, and _deps already imports _should_skip from _analyzer.", + "The scan tables are now two modules away from the extraction code that reads them, so a future skip-list change has to be made in _scan_tables rather than where it is used." + ], + "rollback": [ + "git revert the commit; _analyzer.py returns to 388 lines and the two new modules disappear.", + "After rollback, re-run the package suite and confirm it still reports 216 passed." + ] +} diff --git a/python_pkg/code_tutor/_analyzer.py b/python_pkg/code_tutor/_analyzer.py index c09a8cc6..244563c9 100644 --- a/python_pkg/code_tutor/_analyzer.py +++ b/python_pkg/code_tutor/_analyzer.py @@ -8,47 +8,13 @@ from __future__ import annotations import ast from dataclasses import dataclass, field -import hashlib from pathlib import Path -import re -_SKIP_DIRS: frozenset[str] = frozenset( - { - "third_party", - ".venv", - "node_modules", - "__pycache__", - ".git", - "dist", - "build", - } -) -_SKIP_SUFFIXES: frozenset[str] = frozenset( - { - ".geojson", - ".png", - ".jpg", - ".jpeg", - ".gif", - ".ico", - ".svg", - ".webp", - ".pdf", - ".zip", - ".tar", - ".gz", - ".bin", - } -) -_OTHER_LANGS: frozenset[str] = frozenset( - {".js", ".ts", ".go", ".rs", ".c", ".cpp", ".dart"} -) - -# Matches leading keyword(s) followed by an identifier and opening paren. -_FUNCTION_RE: re.Pattern[str] = re.compile( - r"^[ \t]*(?:(?:pub(?:lic)?|priv(?:ate)?|prot(?:ected)?|static|async)\s+)*" - r"(?:def|function|fn|func|void|int|float|bool|string)\s+(\w+)\s*\(", - re.MULTILINE, +from python_pkg.code_tutor._scan_tables import ( + _FUNCTION_RE, + _OTHER_LANGS, + _SKIP_DIRS, + _SKIP_SUFFIXES, ) @@ -258,131 +224,3 @@ def extract_items(codebase: Path) -> list[CodeItem]: elif path.suffix in _OTHER_LANGS: items.extend(_extract_other(path, rel)) return items - - -def get_python_files(codebase: Path) -> list[Path]: - """Return all non-skipped Python files under *codebase*. - - Args: - codebase: Root directory to search. - - Returns: - Sorted list of absolute Python file paths. - """ - return [ - path - for path in sorted(codebase.rglob("*.py")) - if path.is_file() and not _should_skip(str(path.relative_to(codebase))) - ] - - -def _build_dotted_map(codebase: Path, all_files: list[Path]) -> dict[Path, str]: - """Build a mapping from file to its dotted module path relative to *codebase*. - - Args: - codebase: Root directory used for relative path computation. - all_files: All Python files within the codebase. - - Returns: - Dict mapping each file in *all_files* to its dotted module string. - Files that cannot be made relative to *codebase* are excluded. - """ - result: dict[Path, str] = {} - for f in all_files: - try: - rel = f.relative_to(codebase) - except ValueError: - continue - result[f] = ".".join(rel.with_suffix("").parts) - return result - - -def _match_deps( - imported: set[str], - dotted_map: dict[Path, str], - path: Path, -) -> set[Path]: - """Return the subset of *dotted_map* files that *path* appears to import. - - A file ``f`` is included when any imported module name equals, contains, - or is contained by ``f``'s dotted path. - - Args: - imported: Set of module name strings collected from import statements. - dotted_map: Mapping of file to dotted module path. - path: The file being analysed (excluded from its own dependency set). - - Returns: - Set of dependency files. - """ - deps: set[Path] = set() - for imp in imported: - for f, dotted in dotted_map.items(): - if f != path and ( - imp == dotted - or imp.endswith("." + dotted) - or dotted.endswith("." + imp) - ): - deps.add(f) - return deps - - -def get_file_imports(path: Path, codebase: Path, all_files: list[Path]) -> set[Path]: - """Parse Python imports and return the codebase files this file depends on. - - Uses a suffix-match heuristic: if an imported module name ends with (or - matches) the dotted path of a file in *all_files*, that file is treated as - a dependency. - - Args: - path: The Python file to analyse. - codebase: Root directory used for relative path computation. - all_files: All Python files within the codebase. - - Returns: - Subset of *all_files* that *path* imports from. - """ - try: - source = path.read_text(encoding="utf-8", errors="replace") - tree = ast.parse(source) - except SyntaxError: - return set() - - imported: set[str] = set() - for node in ast.walk(tree): - if isinstance(node, ast.Import): - for alias in node.names: - imported.add(alias.name) - elif isinstance(node, ast.ImportFrom) and node.module: - imported.add(node.module) - - dotted_map = _build_dotted_map(codebase, all_files) - return _match_deps(imported, dotted_map, path) - - -def codebase_fingerprint(codebase: Path) -> str: - """Compute a short fingerprint of all source files under *codebase*. - - Uses each file's mtime and size rather than content, so it is fast - even on large repositories. Returns a 16-character hex string that - changes whenever any tracked file is added, removed, or modified. - - Args: - codebase: Root directory of the codebase. - - Returns: - 16-character hex fingerprint string. - """ - parts: list[str] = [] - for path in sorted(codebase.rglob("*")): - if not path.is_file(): - continue - rel = str(path.relative_to(codebase)) - if _should_skip(rel): - continue - try: - s = path.stat() - parts.append(f"{rel}:{s.st_mtime_ns}:{s.st_size}") - except OSError: - pass - return hashlib.sha256("\n".join(parts).encode()).hexdigest()[:16] diff --git a/python_pkg/code_tutor/_deps.py b/python_pkg/code_tutor/_deps.py new file mode 100644 index 00000000..6b5324aa --- /dev/null +++ b/python_pkg/code_tutor/_deps.py @@ -0,0 +1,145 @@ +"""Walking the codebase for Python files and mapping their import graph. + +Split out of :mod:`python_pkg.code_tutor._analyzer` to keep it under the +250-line cap. Item extraction stays there; this module answers "which files +does this one depend on" and fingerprints the tree. +""" + +from __future__ import annotations + +import ast +import hashlib +from typing import TYPE_CHECKING + +from python_pkg.code_tutor._analyzer import _should_skip + +if TYPE_CHECKING: + from pathlib import Path + + +def get_python_files(codebase: Path) -> list[Path]: + """Return all non-skipped Python files under *codebase*. + + Args: + codebase: Root directory to search. + + Returns: + Sorted list of absolute Python file paths. + """ + return [ + path + for path in sorted(codebase.rglob("*.py")) + if path.is_file() and not _should_skip(str(path.relative_to(codebase))) + ] + + +def _build_dotted_map(codebase: Path, all_files: list[Path]) -> dict[Path, str]: + """Build a mapping from file to its dotted module path relative to *codebase*. + + Args: + codebase: Root directory used for relative path computation. + all_files: All Python files within the codebase. + + Returns: + Dict mapping each file in *all_files* to its dotted module string. + Files that cannot be made relative to *codebase* are excluded. + """ + result: dict[Path, str] = {} + for f in all_files: + try: + rel = f.relative_to(codebase) + except ValueError: + continue + result[f] = ".".join(rel.with_suffix("").parts) + return result + + +def _match_deps( + imported: set[str], + dotted_map: dict[Path, str], + path: Path, +) -> set[Path]: + """Return the subset of *dotted_map* files that *path* appears to import. + + A file ``f`` is included when any imported module name equals, contains, + or is contained by ``f``'s dotted path. + + Args: + imported: Set of module name strings collected from import statements. + dotted_map: Mapping of file to dotted module path. + path: The file being analysed (excluded from its own dependency set). + + Returns: + Set of dependency files. + """ + deps: set[Path] = set() + for imp in imported: + for f, dotted in dotted_map.items(): + if f != path and ( + imp == dotted + or imp.endswith("." + dotted) + or dotted.endswith("." + imp) + ): + deps.add(f) + return deps + + +def get_file_imports(path: Path, codebase: Path, all_files: list[Path]) -> set[Path]: + """Parse Python imports and return the codebase files this file depends on. + + Uses a suffix-match heuristic: if an imported module name ends with (or + matches) the dotted path of a file in *all_files*, that file is treated as + a dependency. + + Args: + path: The Python file to analyse. + codebase: Root directory used for relative path computation. + all_files: All Python files within the codebase. + + Returns: + Subset of *all_files* that *path* imports from. + """ + try: + source = path.read_text(encoding="utf-8", errors="replace") + tree = ast.parse(source) + except SyntaxError: + return set() + + imported: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + imported.add(alias.name) + elif isinstance(node, ast.ImportFrom) and node.module: + imported.add(node.module) + + dotted_map = _build_dotted_map(codebase, all_files) + return _match_deps(imported, dotted_map, path) + + +def codebase_fingerprint(codebase: Path) -> str: + """Compute a short fingerprint of all source files under *codebase*. + + Uses each file's mtime and size rather than content, so it is fast + even on large repositories. Returns a 16-character hex string that + changes whenever any tracked file is added, removed, or modified. + + Args: + codebase: Root directory of the codebase. + + Returns: + 16-character hex fingerprint string. + """ + parts: list[str] = [] + for path in sorted(codebase.rglob("*")): + if not path.is_file(): + continue + rel = str(path.relative_to(codebase)) + if _should_skip(rel): + continue + try: + s = path.stat() + parts.append(f"{rel}:{s.st_mtime_ns}:{s.st_size}") + except OSError: + pass + return hashlib.sha256("\n".join(parts).encode()).hexdigest()[:16] diff --git a/python_pkg/code_tutor/_plan_builder.py b/python_pkg/code_tutor/_plan_builder.py index d5a75280..9f04ca01 100644 --- a/python_pkg/code_tutor/_plan_builder.py +++ b/python_pkg/code_tutor/_plan_builder.py @@ -6,14 +6,17 @@ import dataclasses from datetime import datetime, timezone from graphlib import CycleError, TopologicalSorter from pathlib import Path +from typing import TYPE_CHECKING -from python_pkg.code_tutor._analyzer import ( - CodeItem, +from python_pkg.code_tutor._deps import ( codebase_fingerprint, get_file_imports, get_python_files, ) +if TYPE_CHECKING: + from python_pkg.code_tutor._analyzer import CodeItem + def _toposort_files(file_deps: dict[Path, set[Path]]) -> list[Path]: """Return files in topological order so dependencies come first. diff --git a/python_pkg/code_tutor/_scan_tables.py b/python_pkg/code_tutor/_scan_tables.py new file mode 100644 index 00000000..0069a41d --- /dev/null +++ b/python_pkg/code_tutor/_scan_tables.py @@ -0,0 +1,48 @@ +"""Which paths the analyzer walks past, and how it spots a function. + +Split out of :mod:`python_pkg.code_tutor._analyzer` to keep it under the +250-line cap. Pure data tables with no collaborators, so nothing patches them. +""" + +from __future__ import annotations + +import re + +_SKIP_DIRS: frozenset[str] = frozenset( + { + "third_party", + ".venv", + "node_modules", + "__pycache__", + ".git", + "dist", + "build", + } +) +_SKIP_SUFFIXES: frozenset[str] = frozenset( + { + ".geojson", + ".png", + ".jpg", + ".jpeg", + ".gif", + ".ico", + ".svg", + ".webp", + ".pdf", + ".zip", + ".tar", + ".gz", + ".bin", + } +) +_OTHER_LANGS: frozenset[str] = frozenset( + {".js", ".ts", ".go", ".rs", ".c", ".cpp", ".dart"} +) + +# Matches leading keyword(s) followed by an identifier and opening paren. +_FUNCTION_RE: re.Pattern[str] = re.compile( + r"^[ \t]*(?:(?:pub(?:lic)?|priv(?:ate)?|prot(?:ected)?|static|async)\s+)*" + r"(?:def|function|fn|func|void|int|float|bool|string)\s+(\w+)\s*\(", + re.MULTILINE, +) diff --git a/python_pkg/code_tutor/cli.py b/python_pkg/code_tutor/cli.py index 72f03b76..a0210fc5 100644 --- a/python_pkg/code_tutor/cli.py +++ b/python_pkg/code_tutor/cli.py @@ -21,7 +21,8 @@ from rich.console import Console from rich.table import Table import typer -from python_pkg.code_tutor._analyzer import codebase_fingerprint, extract_items +from python_pkg.code_tutor._analyzer import extract_items +from python_pkg.code_tutor._deps import codebase_fingerprint from python_pkg.code_tutor._llm import OllamaBackend from python_pkg.code_tutor._plan_builder import build_plan from python_pkg.code_tutor._progress import ( diff --git a/python_pkg/code_tutor/tests/test_analyzer.py b/python_pkg/code_tutor/tests/test_analyzer.py index be970cec..b5a083c0 100644 --- a/python_pkg/code_tutor/tests/test_analyzer.py +++ b/python_pkg/code_tutor/tests/test_analyzer.py @@ -8,16 +8,18 @@ from unittest.mock import patch from python_pkg.code_tutor._analyzer import ( CodeItem, - _build_dotted_map, _extract_other, _extract_python, _FunctionVisitor, _is_binary, _make_id, - _match_deps, _should_skip, - codebase_fingerprint, extract_items, +) +from python_pkg.code_tutor._deps import ( + _build_dotted_map, + _match_deps, + codebase_fingerprint, get_file_imports, get_python_files, )