feat(extractor+resolver): local type propagation — instance method resolution (#12)#52
Conversation
…solution (#12) Extract local variable types from assignments (`eng = Engine()`) and parameter annotations (`store: SQLiteStore`) during AST walking, then resolve dotted calls (`eng.execute()`, `store.get_nodes()`) to their absolute FQNs via the local_types metadata on each function node. Key design decisions: - Accumulator pattern: collect local_types in a dict[func_id, {var: fqn}] during _walk, apply via model_copy once in parse() — avoids mutating frozen nodes in-place and the stale-reference / nodes.index() bugs of the previous approach - Byte-slice the `type` node directly for raw type text so union types like `A | None` reach _clean_python_type_string intact; tree-sitter typed_parameter uses named_children[0] for the param name, not a "left" field - Extract _resolve_local_type_call and _resolve_via_global_symbols from _resolve_global_call to bring cyclomatic complexity back under 10 Resolves: issue #12 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements local type propagation and instance method resolution for the Python extractor and resolver engine. It collects variable types from assignments and parameter annotations, stores them in function node metadata, and uses them to resolve dotted method calls. The review feedback highlights a bug in _clean_python_type_string where Optional and Union wrappers are not parsed correctly, a potential TypeError in engine.py when retrieving metadata that might be None, and the need to support module-prefixed types (e.g., module.ClassName) when resolving types against the import map.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
`_collect_assignment_type` was recording the call-expression identifier (e.g. `obj.method`) as a class name when the RHS was `obj.method()`. Because that dotted string happened to start with a known internal root, the resolver produced spurious INTERNAL virtual edges like `cli.current_tree.add.add`. Guard: skip any `class_name` that contains a dot — we cannot infer return types statically. Also adds a regression test `test_method_call_result_not_captured_as_local_type`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix `_clean_python_type_string`: `Optional[X]` and `Union[X, Y]` now
extract the first type argument (X) instead of returning the wrapper
name. Fixes the docstring example that was silently wrong.
- Extract `_resolve_type_fqn` helper to handle both plain and
module-prefixed type names (`models.Store`) via import_map. Used by
both `_collect_assignment_type` and `_collect_param_type` to avoid
duplication and support `import models; x = models.Store()` patterns.
- Tighten method-call-result guard: only skip dotted class names where
the prefix is NOT a known import alias; keep module-qualified ctors.
- `_resolve_local_type_call` (engine.py): use `or {}` fallback so an
explicit `None` in metadata never causes TypeError on iteration.
- Extract `_handle_import_node` from `_walk` to reduce cognitive
complexity from 17 to ≤15 (SonarQube alert).
Adds 2 regression tests: Optional[X] annotation resolution and
module-prefixed constructor type capture.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces local type propagation and instance method resolution for Python files. It updates the AST extractor to collect local types from assignments and typed parameters, storing them in function node metadata, and enhances the resolver engine to utilize this metadata for resolving instance method calls. A comprehensive test suite is also added. The reviewer identified two bugs in the type-string cleaning logic: it fails to handle module-prefixed generic wrappers (e.g., typing.Optional) and malforms nested generics due to aggressive trailing bracket stripping. A robust recursive parsing implementation was suggested to address these issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…type string cleaner
Two bugs in `_clean_python_type_string`:
1. `typing.Optional[X]` was not matched because `outer = "typing.Optional"`
is not in `_GENERIC_WRAPPERS`. Fix: use `outer.split(".")[-1]` to
extract the base name before checking the set.
2. `Optional[list[Node]]` called `rstrip("]")` which stripped all
trailing brackets, mangling `"list[Node]]"` to `"list[Node"`.
Fix: strip only the single outer `]` with `[:-1]`, then recurse so
`list[Node]` further reduces to `list`.
Adds two regression tests: typing.Optional prefix and nested generic.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
1 participant
Summary
Closes #12 — resolves instance method calls (
store.get_nodes(),console.print()) by propagating local variable types extracted during AST walking.What changed:
python_extractor.py: during_walk, collect{var_name: class_fqn}from assignments (x = ClassName(...)) and typed parameters (def f(x: SomeClass)) into alocal_types_accdict; apply to nodes once viamodel_copyafter the walk completesresolver/engine.py: new_resolve_local_type_call— before import-map lookup, check if the source node'slocal_typesmetadata resolvesvar.methodcore/models.py: removedNode.with_local_type()— extractor-specific mutation logic doesn't belong in the domain modelWhy the previous implementation was broken (3 bugs fixed):
node.child_by_field_name("left")ontyped_parameteralways returnsNone— tree-sitter Python usesnamed_children[0]for the parameter namefile_pathwas undefined inside_process_parameter_type(NameError in the same-module fallback branch)nodes.index(func_node)used stale object equality — after the first update, subsequent assignments in the same function silently failed (ValueErrorswallowed), so only the first local type was ever recordedAccumulator pattern vs. in-place mutation:
Collecting into
local_types_acc: dict[str, dict[str, str]]and applying once avoids mutating frozen Pydantic nodes during traversal entirely — no index lookups, no stale references.Test plan
test_local_type_resolution_assignment—eng = Engine()→eng.execute()resolves tosrc.mod.Engine.executetest_local_type_resolution_param_annotation—def run(store: SQLiteStore)→store.get_nodes()resolvestest_local_type_resolution_union_type—def f(x: Node | None)→x.model_dump()resolvestest_local_type_resolution_external_dep—console = Console()→console.print()resolves torich.console.Console.print🤖 Generated with Claude Code