fix(python): resolve module-qualified constructor calls — 0 CALLS edges (Issue #337)#463
Conversation
Python repos were producing 0 CALLS edges for module-qualified constructor calls like `models.User()` where `import models` is a bare module import. Root causes: 1. `SupportedLanguages.Python` was absent from `WILDCARD_IMPORT_LANGUAGES`, so `synthesizeWildcardImportBindings` never ran for Python files — bare module imports never received per-symbol namedImportMap bindings. 2. Synthesis only ran in the Phase 14 pre-pass, after all chunks had already been call-resolved. When `models.User()` was processed in Phase 3+4, `namedImportMap` was empty for Python → Tier 2a-named fell through to Tier 2a which found both `models.py:User` and `auth.py:User` (ambiguous). 3. `filterCallableCandidates` with `callForm='member'` excluded `Class` nodes (only `CALLABLE_SYMBOL_TYPES` = Function/Method/Constructor/…). With 2 ambiguous Class candidates both were dropped, producing 0 CALLS edges. Fixes: - Add `SupportedLanguages.Python` to `WILDCARD_IMPORT_LANGUAGES` so that `import models` expands to per-symbol namedImportMap entries (first-seen semantics: `User→models.py:User`, `Admin→auth.py:Admin`). - Call `synthesizeWildcardImportBindings` inline in the chunk loop, after `processImportsFromExtracted` but BEFORE `processCallsFromExtracted`. This ensures Tier 2a-named can disambiguate `module.ClassName()` at initial call-resolution time. The Phase 14 pre-pass remains as a final safety net. - Add a fallback in `resolveCallTarget`: if `callForm='member'` yields 0 filtered candidates, retry with `callForm='constructor'`. This handles the case where a module-qualified class instantiation (e.g. `models.User()`) is syntactically an attribute-access call but semantically a constructor call. The fallback only triggers for 0-candidate member calls, so it cannot over-eagerly promote normal member calls. Tests: add `python-module-import` fixture (models.py/auth.py/app.py) with 4 regression tests covering IMPORTS edges, name-collision disambiguation for `models.User()`, and `auth.Admin()`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@ShunsukeHayashi is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
CI Report✅ All checks passed Pipeline
Tests
✅ All 3692 tests passed across 1060 files 18 test(s) skipped
Coverage
📋 Full run · Coverage from Ubuntu · Generated by CI |
|
@claude ultra-think while reviewing this PR's architectural fit and if it delivers what's written in the summary. Does it follow the patterns we've already established and doesn't violate the responsibility boundaries. Finally check if performance issues, gaps and test coverage completeness. |
|
Claude finished @magyargergo's task in 3m 51s —— View job PR Review: fix(python) — module-qualified constructor calls (Issue #337)
Summary VerdictThe PR correctly diagnoses and fixes the reported bug. The three root causes are real, the fixes are targeted, and the CI is green. There are no architectural violations. However there are two meaningful gaps and one performance concern worth addressing before merge. 1. Architectural Fit — ✅ CorrectThe pipeline phase ordering is the right mental model here. The fix correctly places The comment in the Phase 14 pre-pass (pipeline.ts:725-732) accurately describes its new role as a "safety net" rather than the primary synthesis trigger — that's a clean retrofit. 2. Responsibility Boundaries — ✅ Clean, with one note
The fallback in 3. Pattern Consistency — ✅ Follows Established ConventionsAdding Python to The 4.
|
| Category | Status |
|---|---|
| Delivers what the summary claims | ✅ Yes |
| Architectural fit | ✅ Clean |
| Responsibility boundaries | ✅ No violations |
| Pattern consistency | ✅ Follows established conventions |
| Performance | |
| Test coverage completeness | |
| Test assertion quality |
The PR is merge-ready for the specific bug it targets. The performance concern is a secondary issue that won't cause correctness failures. The coverage gap and assertion weakness are the most actionable items — I'd recommend at minimum adding the total-call-count assertion and the comment documenting first-seen semantics before merging.
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #337 where Python repositories could produce 0 CALLS edges for module-qualified constructor calls (e.g., models.User()) by improving import binding synthesis timing and expanding callable-candidate filtering to handle class instantiation.
Changes:
- Add Python to wildcard-import binding synthesis and run
synthesizeWildcardImportBindings()earlier (per chunk) so call resolution can use the synthesized bindings. - Update call resolution to retry member calls as constructor calls when member filtering yields zero candidates.
- Add a Python fixture + integration tests covering module imports and module-qualified constructor calls.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
gitnexus/src/core/ingestion/pipeline.ts |
Adds Python to wildcard-binding synthesis and runs synthesis before call resolution (per chunk + final safety pass). |
gitnexus/src/core/ingestion/call-processor.ts |
Adds a fallback to treat some member calls as constructor calls when filtering otherwise yields 0 candidates. |
gitnexus/test/integration/resolvers/python.test.ts |
Adds an integration test suite for Python module-import constructor call resolution. |
gitnexus/test/fixtures/lang-resolution/python-module-import/models.py |
Fixture module exporting User. |
gitnexus/test/fixtures/lang-resolution/python-module-import/auth.py |
Fixture module exporting User and Admin (name collision scenario). |
gitnexus/test/fixtures/lang-resolution/python-module-import/app.py |
Fixture that imports both modules and calls models.User() / auth.Admin(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Synthesize namedImportMap entries for module-qualified calls like Python's | ||
| // `models.User()`. Must run after imports are resolved (importMap is populated) | ||
| // but BEFORE call resolution so Tier 2a-named can disambiguate `module.Name()`. | ||
| // Idempotent: first-seen semantics prevents double-counting across chunks. | ||
| synthesizeWildcardImportBindings(graph, ctx); |
There was a problem hiding this comment.
synthesizeWildcardImportBindings() scans all graph nodes to rebuild exportedSymbolsByFile and iterates the entire ctx.importMap every time it runs. Calling it once per chunk in the worker path risks O(chunks × graph_size) work on large repos. Consider caching exported symbols (or maintaining them incrementally) and/or synthesizing only for the importing files affected by the current chunk’s newly-resolved imports, then keep the final pass as a safety net.
| // Synthesize namedImportMap entries for module-qualified calls like Python's | |
| // `models.User()`. Must run after imports are resolved (importMap is populated) | |
| // but BEFORE call resolution so Tier 2a-named can disambiguate `module.Name()`. | |
| // Idempotent: first-seen semantics prevents double-counting across chunks. | |
| synthesizeWildcardImportBindings(graph, ctx); | |
| // NOTE: Per-chunk wildcard-import synthesis has been removed from the worker path | |
| // to avoid O(chunks × graph_size) behavior. A final/global synthesis pass should | |
| // run after all imports are resolved but before any global call resolution. |
|
@ShunsukeHayashi can you please address them? 🙏 |
|
I'll take on this today! This is quite important! |
Previously, Python was added to WILDCARD_IMPORT_LANGUAGES which expanded all exported symbols into namedImportMap using first-seen wins. This caused `auth.User()` to incorrectly resolve to `models.py:User` when both modules exported a class named User. Root cause: Python `import models` is a namespace import, not wildcard symbol expansion. Expanding all symbols produces ambiguous bindings that cannot be disambiguated later. Fix: - Remove Python from WILDCARD_IMPORT_LANGUAGES - Add ModuleAliasMap (callerFile → alias → sourceFile) to ResolutionContext - In synthesizeWildcardImportBindings, build moduleAliasMap for Python using the filename stem as the module alias - In resolveCallTarget, add module-alias disambiguation step: when multiple candidates survive filtering and the receiver name matches a module alias, narrow candidates to the aliased file Result: `models.User()` → models.py:User, `auth.User()` → auth.py:User even when both modules export a class named User. Adds regression test for the ambiguity case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addressing Copilot review commentsThanks for the detailed review! Here's what was addressed: 1. Correctness: ambiguous resolution when multiple modules export the same name ✅Root cause confirmed: Adding Python to Fix (commit
2. Regression test for the ambiguity case ✅Added to au = auth.User() # both models.py and auth.py export UserNew test: it('resolves auth.User() CALLS edge to auth.py:User (not models.py:User) when both modules export User', () => {
// confirmed passing
});3. Performance:
|
… perf guard - Rewrite Issue #337 test suite: 5 tests → 19 tests with exact node/edge counts, sourceFilePath guards, negative tests, method call disambiguation (u.save(), a.login(), v.verify()), HAS_METHOD verification, and cross-module collision assertions - Un-skip 2 match/case as-pattern tests (they pass now) and remove leftover DEBUG test - Add per-chunk language guard for synthesizeWildcardImportBindings — skips full graph traversal for TS/JS-only chunks, avoiding O(chunks × graph_size) - Rename fixture method check → verify to avoid BUILT_IN_NAMES noise filter - Expand fixture with method calls on constructor-inferred receivers - Fix stale comment referencing only "Go package imports"
Replace Set<number> + flatMap with a simple boolean array indexed by chunk — cleaner data structure for sequential integer keys.
…n-calls-zero-337 fix(python): resolve module-qualified constructor calls — 0 CALLS edges (Issue abhigyanpatwari#337)
Summary
Fixes #337 — Python repos produce 0 CALLS edges for module-qualified constructor calls like
models.User().Root Causes
Three compounding bugs caused 0 CALLS edges:
SupportedLanguages.Pythonmissing fromWILDCARD_IMPORT_LANGUAGES— bare module imports (import models) were never expanded to per-symbolnamedImportMapbindings, soTier 2a-namedcould never disambiguatemodels.User()fromauth.User().Synthesis ran after call resolution —
synthesizeWildcardImportBindingswas called in the Phase 14 pre-pass, after all chunks had already been call-resolved. Whenmodels.User()was processed,namedImportMapwas empty for Python → bothmodels.py:Userandauth.py:Userwere returned as ambiguous Tier 2a candidates.callForm='member'excludedClassnodes —filterCallableCandidateswithcallForm='member'only acceptsFunction/Method/Constructor/Macro/Delegate(notClass). Module-qualified class instantiation likemodels.User()is syntactically an attribute access, but semantically a constructor call. With both ambiguous Class nodes dropped, 0 CALLS edges were emitted.Fixes
pipeline.tsSupportedLanguages.PythontoWILDCARD_IMPORT_LANGUAGESpipeline.tssynthesizeWildcardImportBindingsinline per-chunk, afterprocessImportsFromExtractedbut beforeprocessCallsFromExtracted. Phase 14 pre-pass retained as safety net.call-processor.tscallForm='member'produces 0 filtered candidates, retry withcallForm='constructor'. Only triggers for 0-candidate cases — no false-positive risk for normal member calls.Test Plan
New fixture:
test/fixtures/lang-resolution/python-module-import/models.py—class User(save)auth.py—class User(check),class Admin(login) ← name collision!app.py—import models; import auth; models.User(); auth.Admin()detects User (×2) and Admin classes— symbols present in graphresolves IMPORTS edges from app.py— import resolution worksresolves models.User() CALLS edge to models.py:User— key regression test (name collision disambiguated via first-seen synthesis)resolves auth.Admin() CALLS edge to auth.py:Admin— constructor fallback worksFull test suite: 3643 passed, 0 regressions (2 pre-existing failures in
skip-git-cli.test.tsunrelated to this change)🤖 Generated with Claude Code