fix(extract): stop attribute-form Python decorators binding to an unrelated def (#2315) - #2356
fix(extract): stop attribute-form Python decorators binding to an unrelated def (#2315)#2356Rishet11 wants to merge 1 commit into
Conversation
…elated def (Graphify-Labs#2315) _python_decorator_name returned only the last attribute segment of an attribute-form decorator, discarding the receiver, so `@app.get("/health")` looked up the bare name "get". ensure_named_node's same-file scoped id cannot match a plain module-level function, so it fell through to the bare global id _make_id(name) and silently bound to whatever was already in seen_ids under that name — a real, unrelated def in another file. The edge was wrong, not merely missing. Add _python_decorator_receiver to recover the receiver text, and _python_local_instance_classes to collect module-level `name = LocalClass()` assignments once per Python file. A receiver that traces to a locally instantiated local class now targets that class's method node; a receiver that is locally assigned but untraced, or a dotted chain, gets a receiver-scoped stub that cannot collide with a single-token definition. A receiver that is neither (i.e. imported) keeps its existing bare-name behavior, which test_attribute_decorator_targets_the_symbol_not_the_module locks.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR changes how attribute-form Python decorators (e.g. @app.get(...)) resolve their reference target in the graph extraction engine. It adds two helpers—one to extract a decorator's receiver text and one to identify module-level variables assigned to locally-defined class instances—so the decorated-definition branch can route the edge to a local class method, a receiver-scoped stub, or the existing bare-name target depending on what the receiver resolves to. The surface area is the Python decorator handling in graphify/extractors/engine.py plus four new tests in tests/test_python_decorators.py covering local-receiver class methods, same-file unrelated defs, plain identifier decorators, and compound attribute chains.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 592 functions depend on the 200 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
_extract_generic()— 18 callers, 16 callees
Verification — 592 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: 533 function(s) in the blast radius were not formally verified this run
· 1 more finding(s) on lines outside this diff (see the check run).
Closes #2315
I wrote the original Python decorator path (#2154 /
794cf9e), so this is mine to clean up.The bug
@app.get("/health")emitted areferencesedge to an unrelated module-leveldef getin a different file. The edge was wrong, not merely missing — it pointed at a real node that has nothing to do with the decorator.Two steps caused it:
_python_decorator_name(graphify/extractors/engine.py:4725, dotted/call branch:4739-4741) returned only the last attribute segment ("get"), discardingapp.engine.py:3784) calledensure_named_node("get", line). Inensure_named_node(:2393-2415) the same-file scoped id cannot match a plain module-level function id, so it fell through to_make_id(name)— a bare global id with no file or class scoping — and silently bound to whatever was already inseen_ids. For names likeget,post,run,sendthat collision is common.The fix
_python_decorator_receiverrecovers the receiver:"app"for@app.get(...), the dotted text for@a.b.c,Nonefor non-attribute forms (which have no collision risk)._python_local_instance_classescollects module-levelname = LocalClass()assignments, computed once per Python file and only for Python. It is deliberately scoped to simple top-level assignments — no control flow, no attribute targets — so this is not a new type-inference subsystem.app.get) that cannot collide with a single-token definition since dots cannot appear in a Python identifier; anything else keeps the existing bare-name path.The imported-receiver behavior locked by
test_attribute_decorator_targets_the_symbol_not_the_module(tests/test_python_decorators.py:94) is unchanged, and I did not touch that test.Worth flagging one behavior change I consider an improvement but you may not: for
app = Flask(__name__),appis locally assigned butFlaskis not a local class, so it now takes the scoped-stub branch (app.get) instead of the bareget. Two files that both name a variableappproduce colliding sourceless stubs, and the pre-existing_disambiguate_colliding_node_idspass (resolution.py:655) salts them apart by source file — I verified that on a two-file Flask-shaped repro. So this is strictly better than the old bare-name binding for that case, but it is a change in what the stub is called.Cases that degrade to the old stub behavior rather than to a wrong edge: a class or assignment inside
if/try(not top level), and an imported receiver class. A class defined after the assignment resolves fine, since local class names are collected in a full pass first.Verification
tests/test_python_decorators.py, 5 of them new: the exact cross-file repro; the same collision with the unrelateddef getin the same file; the imported-receiver regression guard; a plain@my_decoratorguard for Python: applying a decorator creates no edge —affected <decorator>returns "No affected nodes found" for every function it wraps #2154; and@a.b.c(...)not binding to an unrelateddef c. The class-method tests assert the target id is present in the node set, so they cannot pass if the fix dropped the edge instead of retargeting it.engine.py, exactly 3 of the new tests fail and the 13 others pass.v8tree here (1 intest_llm_backends.pyfrom a missingopenaimodule, 3 env-dependent ollama backend-detection tests). Failure set unchanged, verified by re-running rather than assumed.ruffclean.python -m tools.skillgen --check→check OK: 134 artifact(s) match committed output and expected/.extract()— that layer difference is what bit me on Calls and constant reads in top-level / script context never get a caller node, so they emit no edges (uniform across languages) #1972, so the edges above come from the exportedgraph.json.A fresh-eyes review pass verified the node-id formula against the real class-node (
engine.py:2479) and method-node (:3344) construction and confirmed they are provably identical for every case this branch can reach (the receiver scan is top-level only, sonamespace_stackis always empty), and confirmed the helper runs once per file rather than per decorator.