Fix/python dead code false positives - #72
Merged
Merged
Conversation
The extractor only resolved relative imports, so functions reached via an absolute intra-project import had no incoming edge and were mis-reported as dead code. - Resolve absolute intra-project imports to call edges; add a post-pass that rewrites dotted targets to canonical slash symbol names and drops stdlib/third-party edges - Emit file-scope edges for module-level calls - Record decorator applications, decorator-call arguments, and parameter defaults as uses - Register function-local (lazy) imports so calls through them resolve - Emit reference edges for functions passed by name as call arguments - Tag click/Typer command and group functions - Bump cacheVersion (with coverage and tests)
The extractor missed several ways Python code references symbols, so many used functions had no incoming edge and were mis-reported as dead. - Resolve absolute and function-local imports to call edges; add a post-pass that rewrites dotted targets to canonical symbol names and drops external ones - Emit file-scope edges for module-level and class-body wiring - Track value references: call arguments, parameter defaults, decorator arguments, attributes, and collection/dict literals - Credit same-module references safely, guarding against parameter shadowing - Resolve string-literal symbol paths and pyproject entry points - Detect route decorators in more forms and tag framework-dispatched handlers, CLI commands, and registration-decorated functions as entry points - Bump cacheVersion (with coverage and tests)
…path filter
The performance analyzer consumes per-function complexity facts from the
language extractors. Two gaps led to systematic false positives: loop-nesting
depth was treated as the Big-O exponent even when a loop iterates a
constant/bounded range, and generated/vendored source was analyzed as if it
were hand-written.
Extractors (Python, TypeScript, Go):
- Emit `scaling_loop_depth` alongside `loop_depth` — loop nesting that counts
only input-scaling loops. Loops over literal/constant collections,
range(<const>), fixed varargs, and infinite while(true)/for{} event or retry
loops are discounted, so a structurally deep but bounded body no longer reads
as O(n^k). Emitted whenever loops exist (even as 0) so consumers can tell
"all loops bounded" from "signal absent"; languages that don't emit it fall
back to loop_depth, unchanged.
Python extractor:
- Tag bodies that directly invoke a DB/network/file primitive (io_direct) and
propagate it transitively across the call graph into `performs_io` via a
monotone fixpoint, mirroring the TypeScript pass. This lets a consumer
distinguish a real per-iteration I/O call from a name that merely collides
with a DB verb.
mcputil:
- Widen IsGeneratedPath to also exclude vendored and machine-generated sources:
the vendor/openapi-gen/third_party/__generated__ path segments and
unambiguous codegen/bundle suffixes (.gen.ts, .pb.go, _pb2.py, .min.js, ...).
Kept to exact segments and explicit suffixes so legitimately named packages
are never hidden.
Bump cacheVersion so cached snapshots re-extract with the new signals.
The performance analyzer flags a call inside a loop as a likely N+1. But a call
that only ever runs inside a bounded loop — an iteration over a literal/constant
collection, range(<const>), a composite literal, or an infinite while(true)/for{}
event loop — runs a fixed number of times, not a pattern that scales with input,
so it should not be treated as an N+1.
Extractors (Python, TypeScript, Go):
- Emit a new prop calls_in_scaling_loop — the subset of calls_in_loop made while
the scaling depth (loops that scale with input) is >= 1, reusing the per-call-site
scaling counter already tracked for scaling_loop_depth. calls_in_loop is unchanged
(still every in-loop call), so the compounding call graph is unaffected; the new
list is the precise input for N+1 detection. Extractors that don't emit it let the
consumer fall back to calls_in_loop, behavior unchanged.
Bump cacheVersion so cached snapshots re-extract with the new signal.
…lasses The Python extractor emitted `abstract` only for formal ABC/Protocol/ @AbstractMethod, and never emitted `enum` or `data_class`. As a result package-metrics could not distinguish Python value-carrier and enum packages from logic packages, and abstractness (A) was ~0 for the many idiomatic base classes that signal "abstract" by raising NotImplementedError rather than subclassing ABC. handleClass now sets, in addition to the existing `abstract` detection: - `enum` for Enum/IntEnum/StrEnum/Flag/IntFlag subclasses, so a pure-enum package is excluded from N (parity with the Kotlin enum handling). - `data_class` for DTO/schema/record classes: @DataClass and @attrs (define/frozen/mutable/attrs/attr.s) decorated classes, and subclasses of Pydantic BaseModel/RootModel/GenericModel/BaseSettings (plus any `*BaseModel` name, covering project-local bases like StrictBaseModel), typing.NamedTuple, and TypedDict. - `abstract` for the duck-typed abstract pattern: a class with a method whose whole body is `raise NotImplementedError` (optionally after a docstring). Conservative — bare `pass`/`...` stub bodies are not treated as abstract. Bump cacheVersion to v86 (v85 introduced the props; v86 broadens data_class to RootModel/*BaseModel subclasses) so cached Python snapshots re-extract.
A repo-local .venv/venv holds the entire third-party dependency tree (thousands of .py files). The Python extractor doesn't walk files itself — it consumes the engine's file list, which prunes only paths matched by the config `ignore` globs. Neither mcp-arch.yaml nor config.Default() listed .venv/site-packages, so the whole dependency tree was walked, parsed, and cached, dominating snapshot time. Add Python virtualenv / dependency / tool-cache directories to every ignore source, using the any-depth `**/<dir>/**` form so nested venvs in monorepos are pruned too. walkRepo already SkipDir's an ignored directory, so the tree is skipped without descending into it: - config.Default() (compiled fallback) and mcp-arch.yaml (shipped config): .venv, venv, site-packages, .tox, .nox, .eggs, __pycache__, and the mypy/ pytest/ruff caches. site-packages is the definitive catch for any oddly named env (conda/direnv/.tox all nest one). - mcputil.IsGeneratedPath: add .venv/venv/site-packages as a hardcoded defense-in-depth guard, so even a custom config that omits them cannot surface dependency symbols. - examples/python.yaml and examples/full.yaml: add the site-packages catch. Bare `env` is intentionally NOT excluded (too common a legitimate directory name); a test asserts app/env/settings.py stays indexed. No cacheVersion bump — this changes file discovery, not extractor output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.