fix(resolve): test mocks no longer erase the real cross-file call graph (#1553)

The cross-file call resolver bailed (#543/#1219 god-node guard) whenever a
bare callee name had 2+ definitions without unique import evidence — so a
single same-named test mock (or any same-named symbol) dropped the real
`calls` edge, erasing the call graph wherever a mock existed (the reporter
saw a 76-stub Pester suite wipe everything).

Replace the blunt bail with a smarter guard: when a name is ambiguous and
import evidence doesn't resolve it, apply tie-breakers — non-test
preference (a shared, segment-aware _is_test_path classifier) then path
proximity — and emit an INFERRED edge ONLY if exactly one candidate
survives, else keep bailing. A real def + a test mock resolves to the real
def; two genuine non-test defs still bail (god-node guard intact, no
fan-out). Wired into both the extract.py pass and the symbol_resolution.py
copy via the shared classifier.

Reported by @Schweinehund.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
safishamsi
2026-06-30 16:59:16 +01:00
parent bd885cc97d
commit bee3849810
6 changed files with 507 additions and 9 deletions
+22 -2
View File
@@ -33,6 +33,7 @@ from graphify.extractors.csharp import _resolve_csharp_type_references
from graphify.extractors.elixir import extract_elixir # noqa: F401
from graphify.extractors.razor import extract_razor # noqa: F401
from graphify.extractors.zig import extract_zig # noqa: F401
from graphify.paths import disambiguate_ambiguous_candidates
_RECURSION_LIMIT = 10_000
@@ -13976,10 +13977,15 @@ def extract(
# "did the caller's file import the callee's file?"
# Use relativized paths to match how file node IDs were remapped above (#502).
nid_to_file_nid: dict[str, str] = {}
# nid -> raw source_file string, for the ambiguous-name tie-breakers below
# (test/non-test classification + path proximity). Kept separate from the
# file-node-id map because tie-breaking compares the actual file paths.
nid_to_source_file: dict[str, str] = {}
for n in all_nodes:
sf = n.get("source_file")
if not sf:
continue
nid_to_source_file[n["id"]] = str(sf)
sf_path = Path(sf)
try:
sf_rel = sf_path.relative_to(root) if sf_path.is_absolute() else sf_path
@@ -14031,6 +14037,7 @@ def extract(
symbol_matches = [c for c in candidates if c in imported_symbols]
if len(symbol_matches) == 1:
tgt = symbol_matches[0]
has_import_evidence = True
else:
module_matches = [
c for c in candidates
@@ -14038,9 +14045,22 @@ def extract(
]
if len(module_matches) == 1:
tgt = module_matches[0]
has_import_evidence = True
else:
continue
has_import_evidence = True
# No unique import evidence. Instead of dropping the edge
# outright (which let a single same-named test mock erase the
# real call graph, #1553), apply the shared god-node
# tie-breakers (non-test preference, then path proximity).
# Resolve only if exactly one candidate survives; otherwise
# the #543/#1219 guard still holds and we skip.
tgt = disambiguate_ambiguous_candidates(
candidates,
{c: nid_to_source_file.get(c, "") for c in candidates},
rc.get("source_file", ""),
)
if tgt is None:
continue
has_import_evidence = False
if tgt != caller and (caller, tgt) not in existing_pairs:
existing_pairs.add((caller, tgt))
# Promote to EXTRACTED when there's a direct import edge from the
+189 -1
View File
@@ -17,10 +17,198 @@ flow) and every reader honours it.
from __future__ import annotations
import os
from pathlib import Path
import re
from pathlib import Path, PurePosixPath
GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out")
# Directory segments that, when they appear as a whole path component, mark the
# whole path as a test location. Matched against path *segments* (not raw
# substrings) so "src/contest.py" / "latest/x.py" / "src/greatest/x.py" do NOT
# match — only a segment that *equals* one of these names (case-insensitively).
_TEST_DIR_SEGMENTS = frozenset({"tests", "test", "spec", "specs", "__tests__"})
# Filename patterns marking a file as a test, matched against the *filename*
# only (case-insensitive). These are conventions across ecosystems:
# test_*.py pytest / unittest
# *_test.* Go / Python / Rust
# *.test.* JS/TS (jest, vitest)
# *.spec.* / *_spec.* Jasmine / RSpec / Karma
# *.Tests.ps1 PowerShell Pester
# *Test.java / *Tests.cs (case-sensitive convention, handled below)
_TEST_FILENAME_PATTERNS = (
re.compile(r"^test_.*", re.IGNORECASE),
re.compile(r".*_test\..+$", re.IGNORECASE),
re.compile(r".*\.test\..+$", re.IGNORECASE),
re.compile(r".*\.spec\..+$", re.IGNORECASE),
re.compile(r".*_spec\..+$", re.IGNORECASE),
re.compile(r".*\.tests\.ps1$", re.IGNORECASE),
# Java `FooTest.java` / `FooTests.java`, C# `FooTests.cs` style. Require an
# uppercase-led `Test`/`Tests` immediately before the extension so plain
# words like "greatest"/"contest.cs" do not match.
re.compile(r".*Test\.java$"),
re.compile(r".*Tests\.java$"),
re.compile(r".*Tests\.cs$"),
)
def _is_test_path(path: str) -> bool:
"""Classify a source path as a test path (case-insensitive, segment-aware).
Shared by extract.py and symbol_resolution.py so cross-file call resolution
treats test mocks/stubs identically. A path is a test path when:
* any whole path segment equals a known test dir name
(``tests``/``test``/``spec``/``specs``/``__tests__``), or
* the filename matches a known test-file naming convention.
Conservative on purpose: matches segments/filenames, never raw substrings,
so ``latest.py``, ``src/contest.py`` and ``src/greatest/x.py`` are NON-test.
"""
if not path:
return False
# Accept both POSIX and Windows separators regardless of host OS so the
# classifier is stable across the mixed paths that flow through extraction.
norm = str(path).replace("\\", "/")
pure = PurePosixPath(norm)
segments = list(pure.parts)
# Strip a leading drive/anchor segment (e.g. "C:/") that PureWindowsPath
# would surface; with the manual "\\"->"/" swap above PurePosixPath keeps
# the path body intact, but guard against a Windows drive embedded as a
# segment just in case.
for segment in segments:
if segment.lower() in _TEST_DIR_SEGMENTS:
return True
# A drive-letter colon segment like "c:" is never a test dir.
filename = pure.name
if not filename:
return False
for pattern in _TEST_FILENAME_PATTERNS:
if pattern.match(filename):
return True
return False
def _path_proximity_winner(call_site_file: str, candidate_files: dict[str, str]) -> str | None:
"""Pick the candidate whose source file is closest to the call site.
``candidate_files`` maps candidate id -> its source_file. Returns a single
winning candidate id, or ``None`` when no proximity tier yields a unique
winner. Tiers, in order:
1. same file as the call site,
2. same directory,
3. longest common path-prefix (must be a strict, unique maximum).
Used only as a secondary tie-break after the test/non-test filter, so the
god-node guard still holds when proximity is genuinely ambiguous.
"""
if not call_site_file:
return None
call_norm = str(call_site_file).replace("\\", "/")
call_dir = PurePosixPath(call_norm).parent
# Tier 1: exact same file.
same_file = [cid for cid, f in candidate_files.items()
if str(f).replace("\\", "/") == call_norm]
if len(same_file) == 1:
return same_file[0]
if len(same_file) > 1:
return None # genuinely ambiguous within one file; bail
# Tier 2: same directory.
same_dir = [cid for cid, f in candidate_files.items()
if PurePosixPath(str(f).replace("\\", "/")).parent == call_dir]
if len(same_dir) == 1:
return same_dir[0]
if len(same_dir) > 1:
return None
# Tier 3: longest common path-prefix, computed over path segments. The
# winner must be a strict unique maximum, else we bail (guard holds).
call_parts = call_dir.parts
def _common_prefix_len(f: str) -> int:
parts = PurePosixPath(str(f).replace("\\", "/")).parent.parts
n = 0
for a, b in zip(call_parts, parts):
if a != b:
break
n += 1
return n
scored = sorted(
((cid, _common_prefix_len(f)) for cid, f in candidate_files.items()),
key=lambda kv: kv[1],
reverse=True,
)
if not scored:
return None
best = scored[0][1]
winners = [cid for cid, score in scored if score == best]
if len(winners) == 1 and best > 0:
return winners[0]
return None
def disambiguate_ambiguous_candidates(
candidates: list[str],
candidate_files: dict[str, str],
call_site_file: str,
) -> str | None:
"""Resolve an ambiguous bare-name call to one candidate, or ``None``.
Shared god-node tie-breaker (#1553) used by both the inline cross-file call
pass in ``extract.py`` and ``symbol_resolution.resolve_cross_file_raw_calls``
so the heuristics stay aligned across languages. ``candidates`` is the list
of node ids sharing the callee's name; ``candidate_files`` maps each id ->
its source_file. Returns the surviving candidate id only when exactly one
survives; otherwise ``None`` (caller keeps the god-node guard / ``continue``).
Tie-breakers, in order:
1. NON-TEST preference. Classify the call site and each candidate as
test/non-test. When the call site is NON-test, drop test candidates.
When the call site IS a test file, prefer test-local candidates
(same file first, then any test candidate); fall back to the full set
only if no test candidate exists.
2. PATH PROXIMITY over whatever survived step 1.
"""
if not candidates:
return None
if len(candidates) == 1:
return candidates[0]
call_is_test = _is_test_path(call_site_file)
test_cands = [c for c in candidates if _is_test_path(candidate_files.get(c, ""))]
nontest_cands = [c for c in candidates if c not in set(test_cands)]
if call_is_test:
# Prefer a test-local definition (same file) first.
call_norm = str(call_site_file).replace("\\", "/")
same_file_test = [
c for c in test_cands
if str(candidate_files.get(c, "")).replace("\\", "/") == call_norm
]
if len(same_file_test) == 1:
return same_file_test[0]
if test_cands:
survivors = test_cands
else:
survivors = nontest_cands or candidates
else:
# Non-test call site: drop test mocks/stubs entirely.
survivors = nontest_cands
if len(survivors) == 1:
return survivors[0]
if not survivors:
return None
# Step 2: path proximity over the survivors.
return _path_proximity_winner(
call_site_file,
{c: candidate_files.get(c, "") for c in survivors},
)
# Bare directory name even when GRAPHIFY_OUT is an absolute path. Used by the
# path guards that walk parents looking for the output dir by name, and by the
# detect scan-exclude so a custom output dir is never re-ingested as source.
+22 -2
View File
@@ -11,6 +11,7 @@ from collections.abc import Sequence
from typing import Any
from graphify.ids import make_id as _shared_make_id
from graphify.paths import disambiguate_ambiguous_candidates
from graphify.security import sanitize_metadata
@@ -319,6 +320,13 @@ def resolve_cross_file_raw_calls(
label_index = build_label_index(all_nodes)
known_pairs = existing_edge_pairs(all_edges)
# nid -> source_file, for the shared god-node tie-breakers (#1553) so a
# same-named test mock no longer erases a real cross-file call.
nid_to_source_file = {
str(n.get("id")): str(n.get("source_file", ""))
for n in all_nodes
if n.get("id")
}
resolved: list[dict[str, Any]] = []
for raw_call in iter_raw_calls(per_file):
@@ -328,9 +336,21 @@ def resolve_cross_file_raw_calls(
if raw_call.get("is_member_call"):
continue
candidates = label_index.get(callee.lower(), [])
if len(candidates) != 1:
if not candidates:
continue
target = candidates[0]
if len(candidates) == 1:
target: str | None = candidates[0]
else:
# Ambiguous bare name. Apply the shared tie-breakers (non-test
# preference, then path proximity); resolve only if exactly one
# candidate survives, else preserve the god-node guard and skip.
target = disambiguate_ambiguous_candidates(
candidates,
{c: nid_to_source_file.get(c, "") for c in candidates},
str(raw_call.get("source_file", "")),
)
if target is None:
continue
caller = str(raw_call.get("caller_nid", ""))
if not caller:
continue
+85
View File
@@ -558,6 +558,91 @@ def test_cross_file_calls_skip_ambiguous_duplicate_labels(tmp_path):
)
def test_cross_file_call_survives_same_named_test_mock(tmp_path):
"""A real cross-file call must NOT be erased by a same-named test mock.
src/caller.py calls save(); src/service.py defines the real save(); a test
mock save() lives in tests/test_service.py. Before #1553 the ambiguous-name
god-node guard dropped the edge entirely. Now the non-test tie-breaker keeps
exactly one caller->save edge pointing at the SRC definition.
"""
src = tmp_path / "src"
tests = tmp_path / "tests"
src.mkdir()
tests.mkdir()
(src / "service.py").write_text("def save():\n return 'real'\n")
(src / "caller.py").write_text("def run():\n save()\n")
(tests / "test_service.py").write_text("def save():\n return 'mock'\n")
result = extract(
[src / "caller.py", src / "service.py", tests / "test_service.py"],
cache_root=tmp_path,
)
nodes = {n["id"]: n for n in result["nodes"]}
save_calls = [
e for e in result["edges"]
if e["relation"] == "calls"
and nodes[e["source"]]["label"] == "run()"
and nodes[e["target"]]["label"] == "save()"
]
assert len(save_calls) == 1, f"expected exactly one run->save edge, got {save_calls}"
target_sf = (nodes[save_calls[0]["target"]].get("source_file") or "")
assert "service.py" in target_sf and "test_service.py" not in target_sf, target_sf
def test_cross_file_call_god_node_guard_two_real_defs(tmp_path):
"""Two genuine NON-test defs of the same name + one caller => ZERO edges.
Proves #543/#1219 is not reopened by the #1553 tie-breakers: with no test
candidate to drop and no proximity winner, the guard still bails.
"""
pkg_a = tmp_path / "a"
pkg_b = tmp_path / "b"
pkg_c = tmp_path / "c"
for d in (pkg_a, pkg_b, pkg_c):
d.mkdir()
(pkg_a / "svc.py").write_text("def save():\n return 'a'\n")
(pkg_b / "svc.py").write_text("def save():\n return 'b'\n")
(pkg_c / "caller.py").write_text("def run():\n save()\n")
result = extract(
[pkg_c / "caller.py", pkg_a / "svc.py", pkg_b / "svc.py"],
cache_root=tmp_path,
)
nodes = {n["id"]: n for n in result["nodes"]}
save_calls = [
e for e in result["edges"]
if e["relation"] == "calls"
and nodes[e["source"]]["label"] == "run()"
and nodes[e["target"]]["label"] == "save()"
]
assert save_calls == [], f"god-node guard must bail, got {save_calls}"
def test_cross_file_call_survives_many_test_mocks(tmp_path):
"""One src def + many same-named test stubs + caller => exactly one src edge."""
src = tmp_path / "src"
tests = tmp_path / "tests"
src.mkdir()
tests.mkdir()
(src / "service.py").write_text("def save():\n return 'real'\n")
(src / "caller.py").write_text("def run():\n save()\n")
for i in range(5):
(tests / f"thing{i}_test.py").write_text("def save():\n return 'mock'\n")
paths = [src / "caller.py", src / "service.py"] + sorted(tests.glob("*_test.py"))
result = extract(paths, cache_root=tmp_path)
nodes = {n["id"]: n for n in result["nodes"]}
save_calls = [
e for e in result["edges"]
if e["relation"] == "calls"
and nodes[e["source"]]["label"] == "run()"
and nodes[e["target"]]["label"] == "save()"
]
assert len(save_calls) == 1, f"expected one run->save edge, got {save_calls}"
assert "service.py" in (nodes[save_calls[0]["target"]].get("source_file") or "")
def test_extract_generic_surfaces_tree_sitter_version_mismatch_hint(monkeypatch):
"""When Language() raises TypeError (e.g. old tree-sitter binding meets a
new tree-sitter API), the error message should point users at the upgrade
+99
View File
@@ -0,0 +1,99 @@
"""Tests for graphify.paths — the shared test-path classifier (#1553)."""
from __future__ import annotations
import pytest
from graphify.paths import (
_is_test_path,
disambiguate_ambiguous_candidates,
)
@pytest.mark.parametrize(
"path",
[
# test dir segments
"tests/foo.py",
"src/tests/foo.py",
"test/foo.go",
"spec/foo.rb",
"specs/foo.rb",
"app/__tests__/foo.js",
"a/b/TESTS/foo.py", # case-insensitive segment
# test filename conventions
"src/test_service.py",
"pkg/service_test.go",
"src/service.test.ts",
"src/service.spec.ts",
"src/service_spec.rb",
"ps/Module.Tests.ps1",
"java/FooTest.java",
"java/FooTests.java",
"cs/FooTests.cs",
# windows separators
"src\\tests\\foo.py",
"src\\service_test.py",
],
)
def test_is_test_path_positive(path: str) -> None:
assert _is_test_path(path) is True, path
@pytest.mark.parametrize(
"path",
[
"",
"latest.py",
"contest.py",
"src/contest.py",
"src/greatest/x.py",
"src/service.py",
"lib/helper.go",
"src/attestation.py", # "test" only as substring, not a segment
"src/testimony.py", # filename starts with "test" but no underscore
"src/contest/x.py", # "contest" is not "test"
"src/greatest.cs", # ends with "test" but not "Tests.cs"
"src/protest.java", # not "*Test.java"
"config/manifest.json",
],
)
def test_is_test_path_negative(path: str) -> None:
assert _is_test_path(path) is False, path
def test_disambiguate_drops_test_candidate_for_nontest_call_site() -> None:
winner = disambiguate_ambiguous_candidates(
["src", "mock"],
{"src": "src/service.py", "mock": "tests/test_service.py"},
"src/caller.py",
)
assert winner == "src"
def test_disambiguate_bails_on_two_nontest_candidates() -> None:
winner = disambiguate_ambiguous_candidates(
["a", "b"],
{"a": "alpha/a.py", "b": "beta/b.py"},
"pkg/caller.py",
)
assert winner is None
def test_disambiguate_test_call_site_prefers_test_local() -> None:
winner = disambiguate_ambiguous_candidates(
["src", "local"],
{"src": "src/service.py", "local": "tests/test_service.py"},
"tests/test_service.py",
)
assert winner == "local"
def test_disambiguate_path_proximity_same_dir() -> None:
# Two non-test candidates; the one in the call site's directory wins.
winner = disambiguate_ambiguous_candidates(
["near", "far"],
{"near": "pkg/a/service.py", "far": "pkg/b/service.py"},
"pkg/a/caller.py",
)
assert winner == "near"
+90 -4
View File
@@ -100,6 +100,9 @@ def test_resolve_cross_file_raw_calls_skips_member_calls() -> None:
def test_resolve_cross_file_raw_calls_skips_ambiguous_duplicate_labels() -> None:
"""Two genuine NON-test defs of the same name: the god-node guard must still
hold even with the #1553 tie-breakers, because neither the non-test filter
nor path proximity yields a unique winner (#543/#1219 stays closed)."""
per_file = [
{
"raw_calls": [
@@ -107,20 +110,103 @@ def test_resolve_cross_file_raw_calls_skips_ambiguous_duplicate_labels() -> None
"caller_nid": "caller_run",
"callee": "log",
"is_member_call": False,
"source_file": "caller.py",
"source_file": "pkg/caller.py",
"source_location": "L2",
}
]
}
]
nodes = [
{"id": "caller_run", "label": "run()", "file_type": "code"},
{"id": "a_log", "label": "log()", "file_type": "code"},
{"id": "b_log", "label": "log()", "file_type": "code"},
{"id": "caller_run", "label": "run()", "file_type": "code", "source_file": "pkg/caller.py"},
{"id": "a_log", "label": "log()", "file_type": "code", "source_file": "alpha/a.py"},
{"id": "b_log", "label": "log()", "file_type": "code", "source_file": "beta/b.py"},
]
assert resolve_cross_file_raw_calls(per_file, nodes, []) == []
def test_resolve_cross_file_raw_calls_real_edge_survives_test_mock() -> None:
"""A real cross-file call must resolve to the SRC definition even when a
same-named TEST mock exists in the corpus (#1553)."""
per_file = [
{
"raw_calls": [
{
"caller_nid": "caller_run",
"callee": "save",
"is_member_call": False,
"source_file": "src/caller.py",
"source_location": "L2",
}
]
}
]
nodes = [
{"id": "caller_run", "label": "run()", "file_type": "code", "source_file": "src/caller.py"},
{"id": "src_save", "label": "save()", "file_type": "code", "source_file": "src/service.py"},
{"id": "mock_save", "label": "save()", "file_type": "code",
"source_file": "tests/test_service.py"},
]
resolved = resolve_cross_file_raw_calls(per_file, nodes, [])
assert [(e["source"], e["target"]) for e in resolved] == [("caller_run", "src_save")]
assert all(e["target"] != "mock_save" for e in resolved)
def test_resolve_cross_file_raw_calls_n_mock_scale() -> None:
"""One src def plus many same-named test stubs: exactly one edge to src."""
per_file = [
{
"raw_calls": [
{
"caller_nid": "caller_run",
"callee": "save",
"is_member_call": False,
"source_file": "src/caller.py",
"source_location": "L2",
}
]
}
]
nodes = [
{"id": "caller_run", "label": "run()", "file_type": "code", "source_file": "src/caller.py"},
{"id": "src_save", "label": "save()", "file_type": "code", "source_file": "src/service.py"},
{"id": "m1", "label": "save()", "file_type": "code", "source_file": "tests/foo_test.py"},
{"id": "m2", "label": "save()", "file_type": "code", "source_file": "spec/bar.Tests.ps1"},
{"id": "m3", "label": "save()", "file_type": "code", "source_file": "test/baz_test.go"},
{"id": "m4", "label": "save()", "file_type": "code", "source_file": "__tests__/q.test.js"},
]
resolved = resolve_cross_file_raw_calls(per_file, nodes, [])
assert [(e["source"], e["target"]) for e in resolved] == [("caller_run", "src_save")]
def test_resolve_cross_file_raw_calls_call_site_is_test_prefers_test_local() -> None:
"""A test file calling save() with both a src def and a test-local def present
resolves to the test-local def (call-site-is-test symmetry, #1553)."""
per_file = [
{
"raw_calls": [
{
"caller_nid": "test_caller",
"callee": "save",
"is_member_call": False,
"source_file": "tests/test_service.py",
"source_location": "L5",
}
]
}
]
nodes = [
{"id": "test_caller", "label": "test_it()", "file_type": "code",
"source_file": "tests/test_service.py"},
{"id": "src_save", "label": "save()", "file_type": "code", "source_file": "src/service.py"},
{"id": "test_save", "label": "save()", "file_type": "code",
"source_file": "tests/test_service.py"},
]
resolved = resolve_cross_file_raw_calls(per_file, nodes, [])
targets = [e["target"] for e in resolved]
assert targets == ["test_save"]
assert "src_save" not in targets
def test_resolve_cross_file_raw_calls_skips_existing_pair() -> None:
per_file = [
{