Repository navigation
fix(calls): stop binding generic Python member calls to an arbitrary same-named method - #2495
Conversation
…same-named method (#2360)
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml Review profile: QUIET Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughPython call resolution now records Python import rebindings and avoids some name-only method matches when receiver evidence is absent or ambiguous. Trie fallback also filters bare Python calls to exclude methods outside the caller’s module. ChangesPython Call Resolution
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change narrows unsupported Python call guesses. Same-module bare-call false positives remain a pre-existing limitation; no new merge-blocking defect was established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change narrows inferred call relationships without adding execution privileges. No introduced security vulnerability was established. Some uncertainty remains around incomplete indexing and concurrent access to analysis state. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ep the guard within Sonar's complexity limit (#2360)
|



Summary
mod.attr(...)call whosemodis imported from outside the project (subprocess.run,asyncio.run,sp.runafterimport subprocess as sp, a function-levelimport subprocess, atry:/except ImportError:fallback import, an optional dependency) no longer reaches the name-only probes. On main those probes bound it to whichever first-partyrunsat closest by import distance, which is how_git_stateand_probe_versionbecame callers ofGraphUpdater.run. The import processor now also keeps every target a name was bound to (python_import_rebinds). A fallback import counts as external only when every branch is external. If a branch binds first-party code, the fallbacks still find it.self.attr) no longer picks one of several same-named first-party methods. This was thefrontend.run(...)/rag_agent.run(...)false edge. A method name defined only once still binds withresolution: heuristic, the same rule JS/TS untyped receivers already follow. Typed receivers,self.m(), imports, classes and module-level class aliases resolve exactly as before. This is also the "consistent rule" asked for in the issue comment:total(shapes)→s.area()no longer binds onlyCircle.area, the one class-name ordering happened to choose. It now gets no edge whatever the class names are. (Fanning out to overrides, so thattests-reaching Square.areafinds the test, would needcallers/tests-reachingto followOVERRIDESand is left out of this PR.)asyncio.to_thread(run, name), whererunis a parameter) no longer resolves to a method in another module. A method is in scope by its bare name only inside its own class body, which is why theproperty(_get_x)case still binds.Demo
Animated run
The screenshot runs
cgr graph callers …GraphUpdater.runon a small repository that copies the issue's call shapes:subprocess.runin_git_stateand in a function-level import in_probe_version,asyncio.run, an untypedrag_agent.run,frontend.runon a dict lookup, and aruncallback parameter. On main the query returns 8 callers, 6 of them false[heuristic]edges. On this branch it returns only the two real call sites,cli.syncandmain.index, both[exact].Type of Change
Related Issues
Fixes #2360
Fixes #2636 (calls on an imported external module such as
os.close,json.loads,os.environ.get,os.path.joinandnp.saveno longer bind to a same-named first-party function; checked on cb85233 with the issue's own repro: all six false edges are gone andimport pool; pool.loads(s)still resolvesexact).Test Plan
uv run pytest -n auto -m "not integration")make test-integration, requires Docker)New file
codebase_rag/tests/test_generic_method_fallback.py, which indexes small synthetic repositories.Red on main (12 failed, each binding
GraphUpdater.runorSession.run, or with no rebind record):test_external_module_call_does_not_bind_a_first_party_method[plain|asyncio|aliased|from-package|try-except|optional]test_function_level_import_of_an_external_module_does_not_bindtest_untyped_local_does_not_guess_between_same_named_methodstest_untyped_parameter_does_not_guess_between_same_named_methodstest_untyped_attribute_does_not_guess_between_same_named_methodstest_local_named_like_a_method_is_not_bound_to_ittest_import_rebinds_are_recorded_and_reset_on_reparseNegative tests. All 13 pass on main and on this branch. I removed six guards one at a time, and each removal turned one of these tests red: the rebind lookup, the spelled-member check, the registry-namespace check, the class-alias check, the own-module exemption, and the unique-name rule.
test_constructed_receiver_still_resolves_exactly[constructed-local|inline-construction]test_annotated_parameter_receiver_still_resolves_exactlytest_annotated_attribute_receiver_still_resolves_exactlytest_self_call_inside_the_class_still_resolvestest_untyped_receiver_with_a_unique_method_name_still_binds(staysheuristic)test_first_party_module_receiver_still_resolvestest_imported_class_receiver_still_resolvestest_module_level_class_alias_receiver_still_bindstest_sibling_script_module_imported_by_bare_name_still_bindstest_root_package_module_imported_by_bare_name_still_bindstest_fallback_import_with_a_first_party_branch_still_bindstest_class_body_reference_to_its_own_method_still_bindsFull unit suite (
pytest -n 2 -m "not integration"): 14191 passed, 124 skipped, 2 xfailed, 2 failed. The two failures areTestCsharpGuardintest_oracle_toolchain_guards.py; they fail because this environment has no .NET SDK and are unrelated to this change. ruff, ty and pre-commit all pass.Manual testing: I indexed the demo repository into a local Memgraph 3.3 with main and then with this branch, and compared
cgr graph callersforGraphUpdater.run(see Demo). I also indexed small synthetic repositories with typed receivers (Protocol-annotated__init__parameter, class-level and dataclass field annotations, constructor-assigned field, annotated parameter, constructed local). Every one of those edges is identical on main and on this branch.Checklist
make pre-commit)# type: ignore,cast(),Any, orobjecttype hints🤖 Generated with Claude Code
https://claude.ai/code/session_01QjpNsgSNyL5gsmAFqfuoD6
Summary by CodeRabbit
Generated by Claude Code