Skip to content

fix(calls): stop binding generic Python member calls to an arbitrary same-named method - #2495

Merged
vitali87 merged 3 commits into
mainfrom
claude/generic-method-fallback-2360
Oct 3, 2026
Merged

vitali87 merged 3 commits into
mainfrom
claude/generic-method-fallback-2360

Conversation

@vitali87

@vitali87 vitali87 commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • A Python mod.attr(...) call whose mod is imported from outside the project (subprocess.run, asyncio.run, sp.run after import subprocess as sp, a function-level import subprocess, a try:/except ImportError: fallback import, an optional dependency) no longer reaches the name-only probes. On main those probes bound it to whichever first-party run sat closest by import distance, which is how _git_state and _probe_version became callers of GraphUpdater.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.
  • A Python member call on an untyped value (a parameter, a dict lookup's result, an unannotated self.attr) no longer picks one of several same-named first-party methods. This was the frontend.run(...) / rag_agent.run(...) false edge. A method name defined only once still binds with resolution: 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 only Circle.area, the one class-name ordering happened to choose. It now gets no edge whatever the class names are. (Fanning out to overrides, so that tests-reaching Square.area finds the test, would need callers/tests-reaching to follow OVERRIDES and is left out of this PR.)
  • A bare Python name (asyncio.to_thread(run, name), where run is 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 the property(_get_x) case still binds.

Demo

cgr graph callers GraphUpdater.run before and after

Animated run

cgr graph callers GraphUpdater.run before and after (animated)

The screenshot runs cgr graph callers …GraphUpdater.run on a small repository that copies the issue's call shapes: subprocess.run in _git_state and in a function-level import in _probe_version, asyncio.run, an untyped rag_agent.run, frontend.run on a dict lookup, and a run callback 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.sync and main.index, both [exact].

Type of Change

  • Bug fix
  • New feature
  • Performance improvement
  • Refactoring (no functional changes)
  • Documentation
  • CI/CD or tooling
  • Dependencies

Related Issues

Fixes #2360
Fixes #2636 (calls on an imported external module such as os.close, json.loads, os.environ.get, os.path.join and np.save no longer bind to a same-named first-party function; checked on cb85233 with the issue's own repro: all six false edges are gone and import pool; pool.loads(s) still resolves exact).

Test Plan

  • Unit tests pass (uv run pytest -n auto -m "not integration")
  • New tests added
  • Integration tests pass (make test-integration, requires Docker)
  • Manual testing (describe below)

New file codebase_rag/tests/test_generic_method_fallback.py, which indexes small synthetic repositories.

Red on main (12 failed, each binding GraphUpdater.run or Session.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_bind
  • test_untyped_local_does_not_guess_between_same_named_methods
  • test_untyped_parameter_does_not_guess_between_same_named_methods
  • test_untyped_attribute_does_not_guess_between_same_named_methods
  • test_local_named_like_a_method_is_not_bound_to_it
  • test_import_rebinds_are_recorded_and_reset_on_reparse

Negative 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_exactly
  • test_annotated_attribute_receiver_still_resolves_exactly
  • test_self_call_inside_the_class_still_resolves
  • test_untyped_receiver_with_a_unique_method_name_still_binds (stays heuristic)
  • test_first_party_module_receiver_still_resolves
  • test_imported_class_receiver_still_resolves
  • test_module_level_class_alias_receiver_still_binds
  • test_sibling_script_module_imported_by_bare_name_still_binds
  • test_root_package_module_imported_by_bare_name_still_binds
  • test_fallback_import_with_a_first_party_branch_still_binds
  • test_class_body_reference_to_its_own_method_still_binds

Full unit suite (pytest -n 2 -m "not integration"): 14191 passed, 124 skipped, 2 xfailed, 2 failed. The two failures are TestCsharpGuard in test_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 callers for GraphUpdater.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

  • PR title follows Conventional Commits format
  • All pre-commit checks pass (make pre-commit)
  • No hardcoded strings in non-config/non-constants files
  • No # type: ignore, cast(), Any, or object type hints
  • New comments and docstrings explain why, not what (see Comment Policy)

🤖 Generated with Claude Code

https://claude.ai/code/session_01QjpNsgSNyL5gsmAFqfuoD6

Summary by CodeRabbit

  • Bug Fixes
    • Improved Python call resolution so calls made through external modules or ambiguous, untyped receivers are less likely to be incorrectly linked to a first-party method.
    • Preserved resolution for known receivers and unique method-name matches, helping relevant calls continue to link correctly.

Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml

Review profile: QUIET

Plan: Advanced

Run ID: aeb17336-87b2-484b-8d20-6c426e6aa85d

📥 Commits

Reviewing files that changed from the base of the PR and between cd9a089 and 4178309.

📒 Files selected for processing (4)
  • codebase_rag/logs.py
  • codebase_rag/parsers/call_resolver.py
  • codebase_rag/parsers/import_processor.py
  • codebase_rag/tests/test_generic_method_fallback.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Python 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.

Changes

Python Call Resolution

Layer / File(s) Summary
Track Python import rebindings
codebase_rag/parsers/import_processor.py, codebase_rag/tests/test_generic_method_fallback.py
ImportProcessor records prior and current targets when Python imports rebind a local name. Re-parsing a module clears its recorded rebindings. Tests cover fallback import rebinding and removal.
Filter trie fallback candidates
codebase_rag/parsers/call_resolver.py, codebase_rag/tests/test_generic_method_fallback.py
Trie fallback uses a cached final name segment and nameability filters. Bare Python calls exclude methods outside the caller’s module. Tests cover first-party module and class references.
Guard external and ambiguous receivers
codebase_rag/logs.py, codebase_rag/parsers/call_resolver.py, codebase_rag/tests/test_generic_method_fallback.py
Python calls on external-module receivers and ambiguous untyped receivers can remain unresolved. Logs identify these cases. Tests cover these guards and retained resolution for known or unique receivers.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 41783

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 Review

Security architecture risk: 🔵 Low · up to 41783

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Repository-authored import and receiver names influence graph relationships within the indexed project. In the traced path, the affected outcome is static analysis metadata, not invocation of the selected callee. No additional tenant, credential, or service authority was established.

Trust Boundaries and Controls

  • observed — External classification uses project ownership, registered classes, namespaces, and matching first-party members as counterevidence. This is an inference safeguard, not an authorization control; treating an unresolved call as proof of runtime unreachability would exceed the demonstrated contract.

Resilience and Maintainability Implications

  • observed — Import parsing clears existing state and catches exceptions without restoring the previous generation. That behavior predates this PR. The caller can continue into later analysis, so complete-state publication after parse failure is not guaranteed by the inspected path. A resulting security-control bypass was not established.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses the coding requirements in [#2360]. CallResolver rejects external Python module receivers when all recorded import bindings are external. It rejects ambiguous untyped Python member …
Out of Scope Changes check ✅ Passed The changes stay within [#2360]. The import-rebinding state supports correct external-module classification. The log constants support the new resolution outcomes. The added tests verify the issue beh…
Title check ✅ Passed The title clearly and concisely describes the primary change: preventing generic Python member calls from binding to arbitrary same-named methods.
Description check ✅ Passed The description follows the required template and covers the summary, demo, change type, related issues, test plan, checklist, reported test results, and known unrelated environment failures.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ep the guard within Sonar's complexity limit (#2360)
@sonarqubecloud

sonarqubecloud Bot commented Oct 3, 2026

Copy link
Copy Markdown

@vitali87
vitali87 merged commit d100790 into main Oct 3, 2026
35 checks passed
@vitali87
vitali87 deleted the claude/generic-method-fallback-2360 branch October 3, 2026 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

1 participant