test(scope-resolution): pin local-namespace-import behavior + document empirical finalize hoisting

Codex round-3 adversarial review raised three concerns about
scope-resolution passes assuming module-scope semantics that would
contradict `pythonImportOwningScope`'s documented per-scope contract.
Empirical verification via scope-dump probes resolved each:

Plan: docs/plans/2026-04-21-003-fix-codex-round3-scope-aware-resolution-plan.md

1. Function- and class-local namespace imports: VERIFIED WORKING.
   `def outer(): import svc as s; s.call()` and `class A: import mod;
   def use(self): mod.helper()` both emit CALLS edges with reason
   "scope-resolution: namespace-receiver". finalize-algorithm hoists
   the ImportEdges onto `indexes.imports[moduleScope]` regardless of
   where the `import` statement appears, so collectNamespaceTargets'
   module-scope read finds them.

2. Imported return-type propagation module-scope-only: VERIFIED
   WORKING (already pinned in round 2). `from svc import get_user`
   inside a function body lands in indexes.bindings[moduleScope], so
   propagateImportedReturnTypes' module-scope read still finds it.

3. Nested method-local defs stamped as class members: VERIFIED FALSE.
   The scope extractor creates nested Function scopes for inner
   `def`s; `def helper` inside `def save` inside `class User` lives
   in helper's own Function scope whose parent is save's Function
   scope (NOT the Class scope). populateClassOwnedMembers'
   `parentScope.kind === 'Class'` branch correctly skips it;
   helper.ownerId stays undefined.

Instead of implementing speculative scope-aware refactors that the
tests would pass regardless, this commit:

- Adds regression fixtures and integration assertions that pin each
  working behavior. If finalize routing ever changes to honor the
  hook's per-scope contract, these assertions flip red and signal the
  need for the scope-chain-aware refactor.
- Adds defensive JSDoc to the three flagged call sites
  (collectNamespaceTargets, propagateImportedReturnTypes,
  populateClassOwnedMembers) documenting the empirical invariant so
  future reviewers don't re-derive Codex's theoretical concern
  without the benefit of the probe.

Files:
- Two new fixtures under test/fixtures/lang-resolution/ covering the
  function-local and class-body namespace-import patterns.
- Two new describe blocks in test/integration/resolvers/python.test.ts
  (3 assertions, positive-pin intent).
- Defensive comments in namespace-targets.ts, imported-return-types.ts,
  and scope-resolution/scope/walkers.ts.

Verification: 204/204 test/integration/resolvers/python.test.ts both
REGISTRY_PRIMARY_PYTHON=0 and =1. tsc clean.
This commit is contained in:
Gergo Magyar 2026-04-21 10:54:01 +01:00
parent e27843774c
commit 41662dca38
8 changed files with 141 additions and 0 deletions

View file

@ -67,6 +67,20 @@ export function followChainPostFinalize(
* After propagation, re-runs the chain-follow on every scope's
* typeBindings — the in-extractor pass-4 ran before propagation and
* missed any chain whose terminal lived in a foreign file.
*
* Scope-chain concern (verified 2026-04-21): `pythonImportOwningScope`
* documents that function-local `from x import y` binds `y` to the
* inner function scope, which would make a module-only write miss
* non-module importers. In practice `finalize-algorithm` hoists those
* bindings into `indexes.bindings[moduleScope]` regardless of where
* the `import` statement appears — the integration fixture
* `python-function-local-import-chain` exercises a chained
* receiver-bound call `u = get_user(); u.save()` inside a function
* body and emits the expected `do_work → User.save` edge. The
* module-scope write is sufficient today. If finalize routing ever
* changes to honor the hook's per-scope contract, this pass must
* iterate `indexes.bindings` over every scope and mirror into the
* binding-owning scope's `typeBindings`, not just the module's.
*/
export function propagateImportedReturnTypes(
parsedFiles: readonly ParsedFile[],

View file

@ -17,6 +17,19 @@
* (TypeScript `import * as X`, Java static import, Ruby `require`)
* uses this directly. `ParsedImport.kind === 'namespace'` is the
* cross-language hook.
*
* Scope-chain concern (verified 2026-04-21): `pythonImportOwningScope`
* documents that function-local and class-body imports bind to the
* inner scope, which would make a module-only read incomplete. In
* practice `finalize-algorithm` places ALL of a file's ImportEdges
* onto `indexes.imports[moduleScope]` regardless of where the
* `import` statement appears — the integration fixtures
* `python-function-local-namespace-import` and
* `python-class-body-namespace-import` both emit correct CALLS edges
* with reason "namespace-receiver", demonstrating that the module-
* scope read is sufficient today. If finalize routing ever changes to
* honor the hook's per-scope contract, this function must walk the
* reference-site scope chain (mirror `findExportedDefByName`).
*/
import type { ParsedFile } from 'gitnexus-shared';

View file

@ -172,6 +172,17 @@ export function populateClassOwnedMembers(parsed: ParsedFile): void {
(def as { qualifiedName: string }).qualifiedName = `${classQ}.${q}`;
};
// Depth invariant (verified empirically against Python scope-extractor
// 2026-04-21): a nested `def helper` declared inside a method body
// lives in its OWN Function scope whose parent is the method's Function
// scope (not the Class scope). That means the `parentScope.kind ===
// 'Class'` branch below only matches DIRECT class-scope children —
// method defs themselves — and never stamps arbitrary nested defs with
// `ownerId = classDef.nodeId`. If an adversarial reviewer raises this
// as a potential false-attribution bug, verify first with a scope dump
// on `class U: def save(self): def helper(): ...` — helper.ownerId will
// remain undefined. The theoretical concern is real only if the
// extractor ever stops creating scopes for inner defs.
for (const scope of parsed.scopes) {
// Methods: function scope whose parent is a Class scope. Owner is
// the parent's Class def.

View file

@ -0,0 +1,9 @@
class A:
# Class-body namespace import — `mod` binds to A's Class scope
# per pythonImportOwningScope. Receiver-bound dispatch for
# `mod.helper()` inside A.use must walk the scope chain up to
# the class scope to discover the namespace target.
import mod
def use(self) -> int:
return mod.helper()

View file

@ -0,0 +1,9 @@
"""
Provider module for the class-body namespace-import test.
`mod.helper` is reached via `mod.helper()` from inside a method of
a class that declares `import mod` in its class body.
"""
def helper() -> int:
return 42

View file

@ -0,0 +1,14 @@
def outer() -> None:
# Function-local namespace import — pythonImportOwningScope pins
# `s` (alias for svc) to outer's Function scope. Receiver-bound
# dispatch for `s.call()` must discover the namespace target
# through a scope-chain walk, not only at module scope.
import svc as s
s.call()
def sanity() -> int:
# Pure free call with no local import — guards against Unit 2's
# scope-walk breaking vanilla resolution paths.
return 1

View file

@ -0,0 +1,9 @@
"""
Provider module for the function-local namespace-import test.
`svc.call` is a top-level function that the consumer reaches via
`s.call()` after `import svc as s` inside a function body.
"""
def call() -> None:
return None

View file

@ -2411,3 +2411,65 @@ describe('Python function-local import feeds chained receiver-bound call', () =>
expect(saveCall!.rel.targetId).toContain('User.save');
});
});
// ---------------------------------------------------------------------------
// Function-local namespace import: `def f(): import svc as s; s.call()`
// Codex round-3 flagged this pattern as potentially broken because
// collectNamespaceTargets reads only module-scope imports. Empirically
// the edge IS emitted (finalize hoists ImportEdges onto the module
// scope), so these assertions pin the working behavior. If finalize
// routing ever changes to match pythonImportOwningScope's per-scope
// contract, this block will flip red and signal the need to make
// collectNamespaceTargets scope-chain-aware.
// ---------------------------------------------------------------------------
describe('Python function-local namespace import feeds receiver-bound call', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'python-function-local-namespace-import'),
() => {},
);
}, 60000);
it('emits CALLS edge outer -> svc.call via function-local `import svc as s`', () => {
const calls = getRelationships(result, 'CALLS');
const callEdge = calls.find((c) => c.source === 'outer' && c.target === 'call');
expect(callEdge).toBeDefined();
expect(callEdge!.rel.targetId).toContain('svc.py:call');
});
it('sanity: unrelated function without local import is still parsed as a Function node', () => {
const fns = result.graph.nodes.filter(
(n) => n.label === 'Function' && n.properties.name === 'sanity',
);
expect(fns).toHaveLength(1);
});
});
// ---------------------------------------------------------------------------
// Class-body namespace import: `class A: import mod; def use(): mod.helper()`
// Same theoretical concern as the function-local case above, same
// empirical outcome — finalize hoists the ImportEdge to the module
// scope so the namespace-receiver path finds it from inside A.use.
// These assertions pin that working behavior.
// ---------------------------------------------------------------------------
describe('Python class-body namespace import feeds method receiver-bound call', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'python-class-body-namespace-import'),
() => {},
);
}, 60000);
it('emits CALLS edge A.use -> mod.helper via class-body `import mod`', () => {
const calls = getRelationships(result, 'CALLS');
const callEdge = calls.find((c) => c.source === 'use' && c.target === 'helper');
expect(callEdge).toBeDefined();
expect(callEdge!.rel.targetId).toContain('mod.py:helper');
});
});