fix(python): resolve cross-file inheritance by import (#2736) - #2737
fix(python): resolve cross-file inheritance by import (#2736)#2737NithishKumar04 wants to merge 1 commit into
Conversation
|
@safishamsi This incorporates your review feedback on #1231 by repointing the existing inheritance edge and removing only the now-unreferenced ghost, rather than layering a second edge. The import resolver is conservative for duplicate definitions and competing conditional imports, and the workflow is currently awaiting maintainer approval to run. I would appreciate your review of the resolver placement and ambiguity guard. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
This PR adds a dedicated resolution pass for Python class inheritance so that base classes brought in via import/from ... import statements are linked to their exact definition nodes rather than falling back to bare-name stubs. It introduces _resolve_python_inheritance_references in the resolution module, a _python_base_reference helper in the engine to extract qualified/generic base expressions, and wires the new pass into extract.py ahead of the generic stub-rewire and cross-file import steps, with logic to prevent the weaker uses [INFERRED] edges from overwriting the inherits [EXTRACTED] edges. The surface area spans extract.py, extractors/engine.py, extractors/resolution.py, and a CHANGELOG entry. The changed symbol list also includes many unrelated changelog/rationale entries across other languages (Scala, ObjC, Swift, C++, TS, Java, etc.), though the visible diff centers on the Python inheritance change.
Worth a look
- Label-match dedup can drop legitimate cross-file
usesedges —graphify/extract.py:6060· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2048 functions depend on the 723 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 454 callers, 42 callees - worse:
walk()— 1 callers, 55 callees - new:
_resolve_python_inheritance_references()— 2 callers, 7 callees
Verification — 2048 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1908 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 2 more finding(s) on lines outside this diff (see the check run).
| _apply_symbol_resolution_facts(paths, nodes, edges, root, facts) | ||
|
|
||
|
|
||
| def _resolve_python_inheritance_references( |
There was a problem hiding this comment.
_resolve_python_inheritance_references()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
The seven callees are the existing parsing, module-resolution, and type-classification helpers used by this cohesive resolution pass. I did not introduce a wrapper or split the pass solely to change the coupling metric, since that would move rather than reduce the dependencies. The correctness advisory from the same review is addressed in 9ed6cc8 with focused regression coverage.
5936018 to
9ed6cc8
Compare
|
I validated the same-label dedup advisory. The new regression imports two distinct classes both named |
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
This PR adds Python class inheritance resolution so that classes inheriting from an imported base retain an inherits [EXTRACTED] edge to the actual base definition rather than falling back to a uses [INFERRED] edge. It introduces a new _resolve_python_inheritance_references resolver (wired into extract) that matches inherited base classes to their defining nodes using exact import evidence, adds a _python_base_reference helper in the engine to recognize bare/qualified/generic base expressions, and changes the cross-file import handling to suppress the legacy uses edges the new resolver has already claimed. A CHANGELOG entry and related tests are also included. The surface area spans extract.py (orchestration and edge suppression), extractors/engine.py (base-expression parsing), extractors/resolution.py (the new resolver, including handling of aliases, re-exports, and resolution-context lookups for incremental rebuilds), and Python inheritance resolution tests.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2049 functions depend on the 724 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 455 callers, 42 callees - worse:
walk()— 1 callers, 55 callees - new:
_resolve_python_inheritance_references()— 2 callers, 8 callees
Verification — 2049 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1909 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 2 more finding(s) on lines outside this diff (see the check run).
| _apply_symbol_resolution_facts(paths, nodes, edges, root, facts) | ||
|
|
||
|
|
||
| def _resolve_python_inheritance_references( |
There was a problem hiding this comment.
_resolve_python_inheritance_references()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Summary
inherits [EXTRACTED]relationship for Python classes whose base is imported from another fileuses [INFERRED]edge from replacing or contradicting resolved inheritanceFixes #2736.
Root cause
The per-file extractor already emitted an
inherits [EXTRACTED]edge to a sourceless base stub. A later legacy Python import pass then emitteduses [INFERRED]for the same class/base pair. Graph assembly stores one edge per ordered pair, so the lexically laterusesrelationship replacedinheritsand moved the evidence location from the class declaration to the import line.The global unique-label rewire repaired only simple globally unique names. An import alias remained a ghost, while duplicate class names could leave inheritance unresolved and the legacy pass could choose the wrong module by first-writer order.
Approach
usesedge from the exact import statement that binds an inherited baseRegression coverage
Nine tests cover:
from ... import ... as ...without a ghost nodeusesedgeVerification
4348 passed, 45 skipped- full suite with isolated Git configuration and the required optional dependencyruff check graphify tests--check,--audit-coverage,--schema-singleton,--monolith-roundtrip, and--always-on-roundtripRelated work
usesedges to the referencing symbol (#2652) #2684 changes attribution inside the legacy inferred-uses pass. This change remains compatible by applying structural-edge precedence at the call site rather than rewriting that pass.