Skip to content

fix(pipeline): suppress weak Python member calls - #1324

Closed
Enferlain wants to merge 1 commit into
DeusData:mainfrom
Enferlain:fix/1276-python-weak-member-calls
Closed

fix(pipeline): suppress weak Python member calls#1324
Enferlain wants to merge 1 commit into
DeusData:mainfrom
Enferlain:fix/1276-python-weak-member-calls

Conversation

@Enferlain

@Enferlain Enferlain commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #1276.

Python attribute calls were not consistently classified as member calls, allowing weak suffix_match, unique, and field_type_hint resolutions to create false-positive CALLS edges when no import-reachable target existed.

This change marks unresolved receiver calls as method calls and applies the existing weak-member suppression policy consistently in sequential and parallel resolution. Direct self/cls/super calls and receivers rooted in an imported local name are exempt, so canonical calls such as from pkg import helper; helper.compute() still resolve. Strong LSP, import, same-module, and service resolutions remain available.

Regression coverage exercises extraction plus sequential and parallel pipelines, including positive imported-module receiver calls and negative unrelated member calls.

Verification

  • nix develop --command make -j8 -f Makefile.cbm test-focused TEST_SUITES='extraction registry pipeline'
  • Result: 568 passed.
  • nix develop --command make -f Makefile.cbm lint-format lint-no-suppress
  • Result: passed.
  • git diff --check and changed-range clang-format --dry-run --Werror: passed.
  • Full local smoke was blocked by the active installed MCP version-cohort guard; the updated PR CI is the clean-process smoke verification.
  • Full lint-ci reached cppcheck but returned its repository-wide toomanyconfigs informational diagnostic; no patch-local format or suppression violation remained.

Checklist

  • Every commit is signed off (git commit -s) and follows the Contributor License Agreement
  • Full test suite passes locally (make -f Makefile.cbm test) — focused sanitized suites passed
  • Patch-local formatting and suppression checks pass
  • New behavior is covered by regression tests that would fail without this fix

@DeusData

Copy link
Copy Markdown
Owner

Reviewed properly, and this needs changes — but I want to start with what is right, because the direction is correct and the diagnosis behind it is excellent.

Issue #1276 is one of the best bug reports this repo has received. Reproducible on public code, a quantified repo-wide invariant (55 production→test edges in a 12k-node repo), and a correct implementation-level diagnosis — unflagged is_method plus field_type_hint doing little more than capitalising the receiver token. The fabricated edges you documented are genuinely wrong: accelerator.print() landing on a test mock at 0.85 confidence, .backward() on an unrelated FlashAttentionFunction, .step() picking one of 84 candidates at 0.01. Under our "a wrong edge is worse than a missing one" rule, suppressing those is right, and you landed it on the established Perl and TS/JS precedent including the subtle explicit-drop-list rationale that protects the lsp_* strategies in the parallel path.

Your tests are exemplary too — both resolver paths exercised, the parallel one reconstructed with the field_type_hint trigger, and assertions on the final graph rather than intermediate confidences. That is exactly what #1276's acceptance criteria asked for.

Now the problem, and unlike most red PRs I have looked at tonight, this CI failure is genuinely yours.

Our smoke fixture is:

from pkg import helper
result = helper.compute(42)

and all three smoke legs fail identically with FAIL: trace_path found 0 callers for 'compute'. The suppression removed a true main → compute edge.

The reason is a real Python-versus-TypeScript semantic difference rather than a coding slip. In Python, module.function() is the dominant cross-file call shape, and its receiver is an imported module the indexer already knows — not an unresolvable object. But the blanket is_method flag cannot distinguish a module receiver from an unknown-object receiver, so whenever such a call resolves through a weak strategy the true edge dies. TS/JS never had this failure mode because ES-module imports are not attribute calls on a namespace in the same way. This is not an edge case; it is the canonical Python pattern.

The fix shape: exempt receivers that are known imports of the file. Extraction already has the import table, or the resolution side can consult the import map before applying the guard — flag is_method only when the receiver identifier is not bound by an import. That preserves everything #1276 wants suppressed (accelerator and trainer are parameters, not imports) while keeping helper.compute alive.

Please also add a positive test pinning module.function() survival — that coverage gap is precisely what let this through, and your own checklist honestly noted the smoke suite was not run locally.

One more genuine failure: clang-format violations in the new extract_calls.c block, lines 2224-2235. make -f Makefile.cbm lint-ci reproduces it. One caveat that will save you a wasted round trip — our formatter is the Homebrew LLVM build; a standalone clang-format-20 produces spurious whole-file drift that looks nothing like the real lines.

Two of your six failures are not yours: security / codeql-gate timed out while the actual analyze job passed — a nondeterministic gate we have seen race on other PRs tonight — and ci-ok is just its aggregator.

One thing worth the maintainer's eyes that I have flagged separately: the unique_name drop also costs true edges in small projects or where the LSP misses. That trade was accepted for TS/JS on the strength of an LSP backstop, and Python's hybrid LSP gives a similar one — but Python is the flagship language, so it is worth a deliberate nod rather than inheritance.

With the import-receiver exemption this lands as a flagship graph-quality improvement. The hard part — finding and proving the false-edge factory — is already done.

Signed-off-by: imi <hoshinoimi@gmail.com>
@Enferlain
Enferlain force-pushed the fix/1276-python-weak-member-calls branch from 7c3dbfc to 1ec5212 Compare July 31, 2026 18:49
@Enferlain

Copy link
Copy Markdown
Contributor Author

Addressed the review findings in 1ec5212:

  • imported module receivers are now exempt from weak-member suppression, including roots of attribute chains,
  • added positive regressions for imported-module calls in extraction and both sequential/parallel pipelines,
  • retained negative coverage for unrelated member calls,
  • fixed the affected formatting, and
  • rebased onto current main.

Local verification:

  • focused sanitized extraction registry pipeline: 568 passed
  • lint-format and lint-no-suppress: passed
  • changed-range clang-format --dry-run --Werror and git diff --check: passed

The full local smoke cannot coexist with the currently active installed MCP version cohort, so I left that process untouched and am relying on the updated PR's clean CI environment for smoke verification. Full lint-ci reached cppcheck but ended on its repository-wide toomanyconfigs informational diagnostic; there are no remaining patch-local format or suppression failures.

@Enferlain
Enferlain marked this pull request as ready for review July 31, 2026 19:01
@Enferlain
Enferlain requested a review from DeusData as a code owner July 31, 2026 19:01
@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Aug 3, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Aug 3, 2026
@DeusData

DeusData commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Thank you for updating the Python weak-member-call suppression after the earlier review. It is now routed as a high-priority parsing fix in 0.9.1-rc and is back in the maintainer review queue. The queue is currently full, so the final re-review may take a little time.

@DeusData

Copy link
Copy Markdown
Owner

The direction is right — Python needs the same weak-match suppression TS/JS got (#592/#606 precedent), and #1276 is real. But CI is telling us the guard overshoots: the resolution probe fails on all five legs at python/S6/inherited_method (calls=0, expected>=1) — a legitimate inherited-method edge your suppression removes. Per this repo's rules the probe must not be weakened; production has to keep emitting that edge, e.g. by exempting receivers resolvable to class-hierarchy members before the weak-match check. That plus a rebase (cbm.h/extract_calls.c drifted) and we're glad to re-review. Also note the blanket is_method=true for all Python attribute calls touches every other is_method consumer — worth narrowing or justifying explicitly.

@DeusData

Copy link
Copy Markdown
Owner

Landed. On main as of 5d96377a (PR #1903), with Co-authored-by: credit to you. Fixes #1276.

@Enferlain — thank you. The design, the receiver exemption and the tests are yours; the distill exists because the branch had drifted 584 commits and I would rather absorb that than hand it back to you.

Your scoping was right, and that mattered more than it might look. A parallel PR in this cluster restricted cbm_registry_resolve globally instead of per-language. It was measured against the full corpus set: −244,489 CALLS edges, −11.4% of the entire call graph, 32.5 edges destroyed per suspicious edge removed — and 5,571 Go call sites lost a correct same-module target and gained a wrong cross-package one. Yours is gated on file language, wired at both pass_calls.c and pass_parallel.c, and keeps same_module — which is why it lands and that one does not.

One thing I had to fix in the rebase, worth knowing. Main's gate grew CBM_LANG_ARKTS after ArkTS landed in 0df990aa. Your line predates it, so taking it verbatim would have silently dropped ArkTS from the weak-member guard and reintroduced the #592/#606 false-edge class — with no test to catch it, since ArkTS did not exist when you wrote this. Both gates now read PYTHON || JAVASCRIPT || TYPESCRIPT || TSX || ARKTS, and the lockstep requirement is written into the code at both sites so nobody adds a language to one resolver only.

On the recall question you raised, now settled with measurement. The guard costs one real edge class: an inherited method reached through a parameter-held receiver. python/S6 was measured to be resolving via strategy=unique_name, cands=1, conf=0.7500 — a short-name guess that happened to be right in a 2-file fixture. TypeScript hit exactly this when the receiver-aware guard landed and the repo accepted it, rewriting its S6 probe from >= 1 to == 0 with a tripwire. Python now records the same trade, with the measurement in the comment and the flip-back condition noted for when py_lsp_cross resolves inheritance. That work is underway.

CI then caught one more: test_parallel.c's LSP-ambiguity probe also depended on a weak match. Measured before changing anything — the suppressed edge was suffix_match with 3 candidates at conf 0.55, a coin flip among Alpha/Beta/Legacy.render, while sibling tests on identical source resolve via lsp_method at conf 0.90 and are untouched. That contrast is the line between a guess and knowledge, and it is why the guard is doing the right thing rather than over-reaching.

Your aliased-import handling still holds too, pinned by test — #1371 changed Python aliased-from-import handling since you wrote this, and CBMImport.local_name survives it.

Thank you for a well-scoped fix with tests that pinned both directions. It made the distill straightforward, and the queue is better for it.

@DeusData DeusData closed this Aug 29, 2026
pull Bot pushed a commit to MrDolphin/codebase-memory-mcp that referenced this pull request Aug 29, 2026
…Data#1324)

Python attribute calls were not classified as member calls, so weak
short-name strategies (suffix_match / unique_name / field_type_hint)
could bind `obj.method()` to an arbitrary same-named project symbol —
`accelerator.print()` resolving to MockAccelerator.print.

Flag a Python attribute call is_method when its receiver is unresolved,
and extend the existing TS/JS weak-member guard to Python. Receivers that
ARE known stay exempt: self/cls, super(), and any identifier bound by one
of the file's imports (including the root of an attribute chain), so
Python's canonical `module.function()` cross-file call still resolves.

The guard helper is renamed cbm_tsjs_suppress_weak_method_match ->
cbm_suppress_weak_member_match. Its drop-list is unchanged, and the
per-language gate stays at the two call sites, which enumerate the same
language set in pass_calls.c and pass_parallel.c so the sequential and
parallel resolvers cannot diverge. ArkTS remains in that set.

python/S6/inherited_method in the resolution probe now asserts calls == 0,
recording the same trade TypeScript took when the receiver-aware guard
landed: the edge it used to produce came from a unique_name registry
fallback (measured on main: strategy=unique_name, cands=1, conf=0.7500)
where a weak short-name guess happened to be right in a 2-file fixture.
It flips back to >= 1 once py_lsp_cross resolves inheritance, matching the
flip-back note already carried on the TypeScript S6 case.

Fixes DeusData#1276
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Enferlain <15861396+Enferlain@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python unresolved member calls produce false-positive CALLS edges via suffix_match and field_type_hint

2 participants