Skip to content

fix(python): resolve calls through injected fields and method results - #4726

Open
keny11300 wants to merge 1 commit into
Graphify-Labs:v8from
keny11300:fix/python-injected-receiver-flow
Open

keny11300 wants to merge 1 commit into
Graphify-Labs:v8from
keny11300:fix/python-injected-receiver-flow

Conversation

@keny11300

Copy link
Copy Markdown

What does this PR do?

Fix four missing Python method-call edges in a small task-service project:

TaskService.create_task   -> TaskRepository.add
TaskService.complete_task -> TaskRepository.get
TaskService.complete_task -> Task.complete
TaskService.pending_tasks -> TaskRepository.all

The service receives repository: TaskRepository and assigns self.repository = repository. The extractor previously lost that binding, so calls through the field stayed unresolved. In task = self.repository.get(key); task.complete(), the local receiver also lost the resolved method's -> Task annotation.

This change carries unchanged typed bindings into fields and links a local method result to its exact producer call span. The resolver uses the producer's single-class return annotation, while preserving caller file/line provenance and INFERRED confidence. CLI and watch rebuilds retain unchanged return-type references as resolution context, and AST cache schema 26 retires cached calls without these markers.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

A receiver must resolve to one class before a call edge is added. Unknown, rebound, shadowed, container-valued, or ambiguously typed results stay unresolved. A nested argument call on the same line cannot substitute for the assigned producer; there is no bare-method-name fallback.

The new tests cover all four edges, source locations, negative cases, cold/warm caches, schema invalidation, changed-file extraction, and actual CLI/watch rebuilds.

Limits: this does not add general data-flow inference. Changing a producer's return annotation still requires a full rebuild to recompute inferred relationships for unchanged callers.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test.
  • Kept the PR description synchronized with the final implementation.
  • Documented limitations / unsupported cases explicitly.

How was this tested?

macOS, CPython 3.13.2, dependencies installed with uv sync --all-extras --frozen. Base: 92741d162b2094ffc2fed58cfbd8ee823e9da62c (0.9.95).

.venv/bin/python -m pytest tests/test_python_injected_factory_calls.py -q --tb=short --disable-warnings
# Base: 8 failed, 12 passed. Patched: 20 passed.

.venv/bin/python -m pytest tests/test_python_*.py tests/test_issue_3405_python_resolution.py tests/test_extract.py tests/test_cache.py tests/test_incremental.py tests/test_watch.py tests/test_language_resolvers.py tests/test_skill_code_only_incremental_update.py -q --tb=short --disable-warnings
# 882 passed, 1 xfailed.

.venv/bin/ruff check .
.venv/bin/python -m tools.skillgen --check
git diff --cached --check
# All passed; 134 generated artifacts match.

GRAPHIFY_BACKEND=none .venv/bin/python -m graphify update . --no-cluster
# Completed locally.

.venv/bin/pyright --pythonpath .venv/bin/python --outputjson
# 657 errors on both base and patch; no added diagnostics.

The full pytest tests/ -q --tb=short --disable-warnings run reported 8723 passed, 12 failed, 26 skipped, 1 xfailed. Eleven failures came from missing historical skill baselines in the shallow checkout. After git fetch --unshallow origin v8, rerunning tests/test_skillgen.py tests/test_skill_no_backtick_substitution.py gave 233 passed, 1 failed.

The remaining failure, test_control_a_backticked_unknown_command_is_caught, also fails on the unmodified base with this Mac's Bash 3.2. The test relies on BASH_XTRACEFD; no skill-generation or shell-test source is changed here. Pyright's existing diagnostics were likewise compared against the unmodified base rather than treated as passing.

Graphify-specific checklist

  • Generated skill sources are unchanged; the generated-artifact check passes.
  • AST/structural extraction remains deterministic.
  • Reviewed for security implications; no source execution or unsafe interpolation added.
  • No API keys or local-only graph data included.
  • Material AI assistance disclosed with a Co-Authored-By: commit trailer.

Carry unchanged typed bindings through self.field assignments and associate
method-result receivers with the exact resolved producer call. Preserve
INFERRED provenance, fail closed on ambiguous or rebound values, and retain
return-type context during incremental CLI and watch rebuilds.

Bump the AST cache schema and cover the four missing task-service calls,
cache reuse, incremental rebuilds, and negative resolution cases.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
@keny11300
keny11300 requested a review from safishamsi as a code owner October 23, 2026 20:57
@github-actions

Copy link
Copy Markdown

Thanks for the pull request, @keny11300. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.

Not checked: tests were not run; formal verification proved 0 of 6 changed function(s) (6 not verified).

Formal verification. PR-changed functions: 0/6 verified (0 proven, 0 may-equivalent, 0 distinguished) · 6 not verified (4 vacuous, 2 unsupported).

Not verified on this run: dispatch\_command (vacuous: never exercised), \_resolve\_python\_member\_calls (vacuous: never exercised), \_extract\_generic (unsupported), \_python\_local\_class\_bindings (vacuous: never exercised), \_python\_self\_attr\_bindings (vacuous: never exercised), \_rebuild\_code (unsupported).


Graphify review — findings

Adds Python call inference through injected fields and factory results: self.field = value keeps an unchanged parameter or local's declared class, and a local assigned a member-call result takes the resolved method's single scalar return-type reference, matched by the producer call's exact byte span rather than a same-line argument or bare name. Unknown, conflicting, shadowed, container-returning, and ambiguous cases emit no call, surviving edges stay INFERRED, and receiver lookup on the factory path covers the producer's own class, not inherited methods. Bumps the AST cache schema to 26 so stale extractions re-run, and makes changed-caller rebuilds carry _py_return_type markers and return_type reference edges as resolver context, though changing a producer's return annotation still needs a full rebuild to refresh unchanged callers.

No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 4342 functions depend on the 982 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract() — 1018 callers, 62 callees
  • worse: _rebuild_code() — 189 callers, 60 callees
  • worse: _extract_generic() — 20 callers, 47 callees
  • worse: save_cached() — 54 callers, 10 callees

Verification — 4342 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: 4288 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

454 of 454 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/pi.test.ts — full-run-safety
  • tests/test_affected_cli.py — impact, full-run-safety
  • tests/test_affected_defines.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_affected_overrides.py — impact, full-run-safety
  • tests/test_affected_path_file_node.py — full-run-safety
  • tests/test_agents_platform.py — impact, full-run-safety
  • tests/test_alias_folding.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — impact, full-run-safety
  • tests/test_astro_import_ids.py — impact, full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_backend_opt_in.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_benchmark_real_query.py — full-run-safety
  • tests/test_binary_media_words.py — impact, full-run-safety
  • tests/test_blade_extractor.py — impact, full-run-safety
  • tests/test_build.py — impact, full-run-safety
  • tests/test_build_located_semantic_identity.py — full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — impact, full-run-safety
  • tests/test_c_include_resolution.py — impact, full-run-safety
  • tests/test_cache.py — impact, full-run-safety
  • tests/test_cache_long_paths.py — impact, full-run-safety
  • tests/test_cache_stale_import_target.py — impact, full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_cargo_missing_manifest.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — impact, full-run-safety
  • tests/test_charmap_encoding.py — impact, full-run-safety
  • tests/test_chunking.py — impact, full-run-safety
  • tests/test_cjs_module_extension.py — impact, full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — impact, full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cloud_cta.py — impact, full-run-safety
  • … and 404 more

non-code file(s) changed (ARCHITECTURE.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (ARCHITECTURE.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Docs that may be stale (advisory)

Formal verification

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, 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: no capturable inputs from the test suite; property tier: not verifiable: all 40 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_resolve\_python\_member\_calls.

The verifier did not have enough to check \_resolve\_python\_member\_calls, 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: no capturable inputs from the test suite; property tier: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous

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: no capturable inputs from the test suite; property tier: parameter `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_python\_local\_class\_bindings.

The verifier did not have enough to check \_python\_local\_class\_bindings, 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: no capturable inputs from the test suite; property tier: not verifiable: all 228 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_python\_self\_attr\_bindings.

The verifier did not have enough to check \_python\_self\_attr\_bindings, 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: no capturable inputs from the test suite; property tier: not verifiable: all 228 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous

Could not verify: Could not verify \_rebuild\_code.

The verifier did not have enough to check \_rebuild\_code, 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: no capturable inputs from the test suite; property tier: nondeterministic output (both versions disagreed with themselves on the witness) — not a behaviour change; a function whose output is not a function of its inputs (random / time / uuid / os.urandom, or state carried across calls) cannot be soundly distinguished by execution, so this tier abstains for the whole function

· 4 more finding(s) on lines outside this diff (see the check run).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant