mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 18:47:04 +01:00
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
parent
6c999e3242
commit
c113a3a192
@ -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."
|
||||
}
|
||||
@ -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 <the touched files and artifacts>",
|
||||
"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."
|
||||
]
|
||||
}
|
||||
@ -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]
|
||||
|
||||
145
python_pkg/code_tutor/_deps.py
Normal file
145
python_pkg/code_tutor/_deps.py
Normal file
@ -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]
|
||||
@ -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.
|
||||
|
||||
48
python_pkg/code_tutor/_scan_tables.py
Normal file
48
python_pkg/code_tutor/_scan_tables.py
Normal file
@ -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,
|
||||
)
|
||||
@ -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 (
|
||||
|
||||
@ -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,
|
||||
)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user