Skip to content

fix(pipeline): never bind an external Python import to a same-named project symbol (#2127) - #2345

Merged
DeusData merged 2 commits into
mainfrom
fix/issue-2127
Oct 1, 2026
Merged

DeusData merged 2 commits into
mainfrom
fix/issue-2127

Conversation

@DeusData

Copy link
Copy Markdown
Owner

For Python, an import from a module outside the project (from unittest.mock import patch, import json) could be bound by the import resolver's name-only fallback to any same-named project definition. That gave import_map CALLS edges at 0.95; when the import edge was missing, the weak unique_name/suffix_match fallbacks did the same.

  • Import side (pass_pkgmap.c, shared by both drivers): for external Python imports, a Strategy-3 fallback hit must contain the import's module chain as an ordered subsequence, and module-naming segments only bind Module/File nodes. Project-internal imports and re-exports are unchanged.
  • Call side, identical gates in the sequential and parallel resolvers: weak-strategy CALLS (unique_name, suffix_match, field_type_hint, fuzzy) whose root is bound by an external import and contradict its chain are not emitted. import_map, same_module and lsp_* edges are untouched. Per-language (Python only), keyed on the module chain rather than a name list.

django/django (fresh index each, parallel pipeline): CALLS 62,430 → 60,123, TESTS 29,872 → 28,571, IMPORTS 16,068 → 15,491. A random sample of 25 removed CALLS was 25/25 false (e.g. os.path.join → a template filter, json.loads → signing.loads, mock.patch → RedirectView.patch); 6 IMPORTS were re-targeted to the real definition.

Tests: edge_imports::ei_py_external_import_never_binds_project_symbol (sequential and parallel legs, with a recall pin for re-exports) plus registry unit tests import_binding_suppress_drops_only_weak_contradicted_calls and python_import_binding_contradicts_only_foreign_chains. RED before the fix; RED on revert, and disabling only the call guard also goes RED, so both halves bind.

Known leftovers (not in this PR): a plain import X of a stdlib name can still bind to a nested project module of the same name; internal re-exported names can still get weak false targets for bare calls.


Touches pass_calls.c / pass_parallel.c (one extra operand in drop_plain_call) alongside #2332 and #2339.

Fixes #2127

Aerox912 pushed a commit to Aerox912/codebase-memory-mcp that referenced this pull request Sep 30, 2026
The parallel-scheduler contract failed twice on test-windows CLANG64 while
running fake suites only:

- PR DeusData#2394 (run 36364479687): "Windows timeout race refused without a
  surviving descendant to refuse over". The refusal came 38s after the
  forced leader exit (a cold powershell/CIM descendant probe, once in the
  wave loop and again in the cleanup pass), while the fixture's descendant
  was a `time.sleep(30)` that had already exited on its own.
- PR DeusData#2345 (run 36199749518): "suite 'hang_after_summary' taskkill could
  not prove process-tree cleanup". The scheduler bounded taskkill.exe with
  --kill-grace, which the contract passes as 1s, so a slow cold start of
  the tool was reported as a failed cleanup.

Both verdicts were a race between a clock and runner latency. An audit of
the fixture found more of the same shape:

- Every hanging fixture ended on its own after 30s. A starved scheduler
  could therefore see a "hung" leader exit 0, and a descendant the
  scheduler failed to kill could vanish before the leak check looked.
- The 1s --timeout could kill a slow leader before it had printed its
  summary (asserted pass=1), spawned its descendant or written the
  descendant pid. That pid was then read back with int() from a file
  written create-then-write, so a reader could see no file or an empty one.
  The empty-file read recorded earlier was the scheduler's ready file,
  which 3b9052c already publishes atomically; the fixture's own writer
  had the same shape.
- The descendant installed its SIGTERM-ignore during its own start-up. A
  SIGTERM that won that race skipped the SIGKILL escalation the POSIX leg
  exists to cover, silently and with the verdict unchanged.
- On Windows, the stubborn-tree leak check was a single look right after
  taskkill, but TerminateProcess returns before its target has exited.

Fix it by construction, not by budget:

- A gate. Every process that must outlive the scheduler's decision blocks
  reading stdin. That is a pipe whose only write end the contract holds,
  handed down through the scheduler, and the read returns only at EOF. The
  contract closes the pipe once the verdict is recorded. If the contract
  dies, EOF releases everything, so nothing is orphaned and no fixture
  ends on a clock.
- The three hanging scenarios run under the scheduler's existing
  pre-terminate barrier. They are released only once the fixture has
  published `<suite>.established`, which happens after the summary is
  flushed, the descendant has confirmed it is armed, and its pid is known.
  The marker is written to a temp file and moved into place with
  os.replace, so a reader sees either no file or the whole file.
- The Windows leak check waits on the descendant's process handle. That
  wait is exact, not a race: the descendant is held by the gate, so the
  scheduler's kill is the only way it can end.
- run-test-wave.py runs taskkill through windows_taskkill_tree() with its
  own stable-state budget (WINDOWS_TASKKILL_SECONDS = 15), as 132e8fc did
  for the descendant probe. --kill-grace keeps bounding what its name
  says. Real runs already pass --kill-grace 15, so they are unchanged.
  The contract pins this structurally, the same way it pins the probe:
  exactly one taskkill site, no kill-grace there, and a helper bounded by
  the constant that fails closed on a timeout, a start failure or a
  non-zero exit.

Every assertion keeps its contract:
- A leader that hangs after a green summary is recorded as rc=124 pass=1.
- A zero-test child is recorded as rc=97.
- The wave continues after a bounded failure.
- A SIGTERM-resistant tree is killed in full.
- A leader that exits between the timeout decision and termination is
  still cleaned up on POSIX. On Windows the scheduler refuses (rc=2,
  naming the cleanup failure) over a descendant that is really alive.
A new self-check fails loudly if a held leader or descendant is not live
while the barrier holds, that is, if the fixture no longer exercises what
those assertions claim.

Proof, on macOS arm64 and a Linux arm64 container. The Windows branch was
reasoned through; the VM is down.
- Torture hook (proof only) that delays each hanging leader 2s, past the
  1s timeout. The old fixture is RED 3/3 on every scenario (pass=0,
  FileNotFoundError on descendant.pid, "lost its bounded result"). The new
  one is green, including at 8s, and 10/10 under the hook plus CPU load.
- The DeusData#2394 mechanism: after an injected 31s refusal latency the old
  descendant is gone and the new one is live. It exits 0.02s after the
  gate closes.
- The taskkill pin is RED on the old scheduler and green on the new one.
- Stress, 20 runs each under CPU load. The old full contract passed 20/20
  on macOS and 20/20 on Linux, so these hosts do not reproduce the CI
  timing. The rebuilt held scenarios passed 20/20 on macOS and 20/20 on
  Linux. The full contract after the change passes a single run on macOS
  and on Linux, and 20/20 on each leg under CPU load (4 load workers;
  the Linux container is capped at 4 CPUs).

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…roject symbol (#2127)

`from unittest.mock import patch` names a module outside the project.
Strategy 1 of cbm_pipeline_resolve_import_node cannot resolve it, so the
Strategy-3 symbol-name fallback bound the import to the only project
definition whose leaf is `patch` -- a REST view's PkgConfigView.patch --
and every `patch(...)` call became an import_map CALLS edge at confidence
0.95. Once that IMPORTS edge is gone the same call falls through to the
weak short-name strategies (unique_name 0.75, suffix_match), which bind it
again; `mock.patch(...)`, `json.loads(...)`, `os.path.join(...)` reached
project methods the same way, because the member guard exempts
import-rooted receivers.

Two per-language (Python-only) rules, both keyed on the import's module
chain rather than on any spelling list:

- Import side (pass_pkgmap.c, shared by both drivers): for an import
  whose module is not in the project, a Strategy-3 hit must spell the
  import's module chain as an ordered subsequence of its QN (a sys.path
  root or src/ layout the resolver missed still qualifies), and whatever
  names a module (an enclosing path segment, any segment of a plain
  `import x`) may only bind a Module/File node. Imports of project
  modules are unchanged (they may re-export from anywhere).
- Call side (pass_calls.c + pass_parallel.c, identical gates): a call
  whose root identifier is bound by an external import (no IMPORTS edge
  materialized it) and whose weak-strategy target contradicts that
  import's chain is not emitted as a plain CALLS edge. import_map /
  same_module / lsp_* edges are never touched.

django/django (parallel pipeline): CALLS 62,430 -> 60,123, TESTS
29,872 -> 28,571, IMPORTS 16,068 -> 15,491. Removed CALLS are
member calls through stdlib imports (1,900: os.path.join ->
defaultfilters.join, json.loads -> signing.loads, mock.patch ->
RedirectView.patch) and bare calls of stdlib names (398: date ->
defaultfilters.date, urlsplit -> MultiPartParser.parse); a random
25-edge sample was 25/25 false. Six IMPORTS are re-targeted to the real
definition (template_tests.utils.setup, multiple_database TestRouter,
pr_quality errors.Message).

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The memory-core ratchet flagged src/pipeline/pass_pkgmap.c growing by 2 raw
allocator sites (82 -> 84). Both are frees of strings returned by
cbm_pipeline_fqn_compute and cbm_pipeline_resolve_module, which the core
never counted, so they now go through safe_free as the rest of the codebase
does rather than raw free or cbm_free. Behaviour is unchanged;
pass_pkgmap.c is back at its baseline of 82.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData merged commit bedbc0a into main Oct 1, 2026
40 checks passed
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.

High-confidence import_map collides on common method names

1 participant