fix(extract): gate JS/TS cross-file calls on import evidence to kill phantom cross-package edges (#1659)
When a callee had exactly one same-named definition repo-wide, the cross-file resolver emitted a `calls` edge at INFERRED/0.8 even with no import path between caller and callee. On a monorepo this fabricated dependencies: a 14-package repo showed `platform`/`sidecar` depending on `registry-protocol` purely because it exported generically-named symbols that unresolved calls collapsed onto. JS/TS modules have no implicit cross-module scope, so a cross-file call is real only if the caller imported it. Direct JS/TS cross-file `calls` attribution is now gated on import evidence and left unresolved otherwise. Scoped to direct calls: other languages keep the #1553 single-candidate resolution (C/C++ headers, Ruby autoload, same-package implicit scope), and the indirect_call path (already INFERRED + callable-gated) is untouched. Also hardens caller/candidate -> file mapping to resolve via the node's `source_file` string (identifying the file node by its basename label) instead of `relative_to(root.resolve())`, which threw on a path-resolution/symlink mismatch and fell back to a non-matching absolute id — spuriously failing import evidence. This both makes the new gate safe and fixes legitimate cross-file calls being mislabeled INFERRED instead of EXTRACTED. Full suite: 2898 passed, 3 skipped. Verified via CLI on the reporter's repro (phantom dropped) and a control (imported call resolves EXTRACTED). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
983da3c15f
commit
62b8eb1416
@@ -2,6 +2,10 @@
|
||||
|
||||
Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases)
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Fix: a JS/TS call with no local definition and no import no longer binds to a same-named export in an unrelated package (#1659, thanks @leonaburime-ucla). When a callee had exactly one same-named definition repo-wide, the cross-file resolver emitted a `calls` edge at INFERRED/0.8 even with no import path between the two files. On a monorepo this fabricated dependencies: a 14-package repo showed `platform` and `sidecar` depending on `registry-protocol` purely because it exported generically-named symbols (`*Schema`, etc.) that unresolved calls collapsed onto. JS/TS modules have no implicit cross-module scope, so a cross-file call is real only if the caller imported it — direct JS/TS cross-file `calls` attribution is now gated on import evidence and left unresolved otherwise. Other languages keep the single-candidate resolution (C/C++ headers, Ruby autoload, same-package implicit scope legitimately call across files without an explicit import), and the `indirect_call` path (already INFERRED and callable-gated) is unchanged. As part of the fix, caller→file mapping for import-evidence now uses the raw call's `source_file` string, so a path-resolution/symlink mismatch can no longer spuriously fail evidence and mislabel a real cross-file call.
|
||||
|
||||
## 0.9.6 (2026-07-04)
|
||||
|
||||
- Fix: Ruby plain modules and `Struct.new` / `Class.new` / `Data.define` constant assignments now get container nodes (#1640, thanks @krishnateja7). The extractor only created nodes for `class Foo`, so `module Foo` (utility/`module_function` modules), `Foo = Struct.new(...) do ... end`, `Foo = Class.new(StandardError)`, and `Result = Data.define(...)` produced no node at all — their methods hung off the file via `contains` with dot-less labels, and no edge could ever target them. `module` is now a container type (methods attach via `method` like a class, nested modules included), and a constant assignment whose RHS is one of those factories synthesizes a class node named after the constant, attaches block-defined methods to it, and emits an `inherits` edge for `Class.new(Super)`. Plain constant assignments (`MAX = 100`, `X = Foo.new`) are untouched.
|
||||
|
||||
+45
-3
@@ -16344,9 +16344,20 @@ def extract(
|
||||
elif e.get("relation") == "imports_from":
|
||||
file_to_module_imports.setdefault(e["source"], set()).add(e["target"])
|
||||
|
||||
# Map each node back to its containing file_id so we can ask
|
||||
# Map each node back to its containing file node id so we can ask
|
||||
# "did the caller's file import the callee's file?"
|
||||
# Use relativized paths to match how file node IDs were remapped above (#502).
|
||||
# A node and its file node share the exact same ``source_file`` string, and a
|
||||
# file node is the one whose label is the basename (``add_node(file_nid,
|
||||
# path.name)``). Resolving file membership by that shared string is robust
|
||||
# against the path-resolution/symlink mismatch that makes
|
||||
# ``relative_to(root.resolve())`` throw and fall back to a non-matching
|
||||
# absolute-derived id — which would spuriously fail import evidence and (with
|
||||
# the #1659 JS/TS gate below) drop a legitimately-imported call.
|
||||
sf_to_file_nid: dict[str, str] = {}
|
||||
for n in all_nodes:
|
||||
sf = n.get("source_file")
|
||||
if sf and n.get("label") == Path(str(sf)).name:
|
||||
sf_to_file_nid.setdefault(str(sf), n["id"])
|
||||
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
|
||||
@@ -16357,6 +16368,12 @@ def extract(
|
||||
if not sf:
|
||||
continue
|
||||
nid_to_source_file[n["id"]] = str(sf)
|
||||
fnid = sf_to_file_nid.get(str(sf))
|
||||
if fnid is not None:
|
||||
nid_to_file_nid[n["id"]] = fnid
|
||||
continue
|
||||
# Fallback (no file node found for this source_file): derive it the old
|
||||
# way from the relativized path.
|
||||
sf_path = Path(sf)
|
||||
try:
|
||||
sf_rel = sf_path.relative_to(root) if sf_path.is_absolute() else sf_path
|
||||
@@ -16372,6 +16389,10 @@ def extract(
|
||||
(e["source"], e["target"]) for e in all_edges
|
||||
if e.get("relation") in ("calls", "indirect_call")
|
||||
}
|
||||
# JS/TS/JSX modules have no implicit cross-module scope: a call into another
|
||||
# file is real ONLY if the caller imported it. So a cross-file call from one
|
||||
# of these files with no import evidence is gated below (#1659).
|
||||
_JS_TS_CALL_SUFFIXES = (".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs")
|
||||
for rc in all_raw_calls:
|
||||
callee = rc.get("callee", "")
|
||||
if not callee:
|
||||
@@ -16392,7 +16413,15 @@ def extract(
|
||||
if not candidates:
|
||||
continue
|
||||
caller = rc["caller_nid"]
|
||||
caller_file_nid = nid_to_file_nid.get(caller)
|
||||
# Resolve the caller's file via the raw_call's own source_file string,
|
||||
# which is stable regardless of any caller_nid remap. An indirect
|
||||
# callback's caller_nid is the file node, whose id may have been
|
||||
# relativized after the raw_call was recorded, so a caller_nid lookup can
|
||||
# miss and (with the #1659 gate) drop a legitimately-imported callback.
|
||||
caller_file_nid = (
|
||||
sf_to_file_nid.get(str(rc.get("source_file", "")))
|
||||
or nid_to_file_nid.get(caller)
|
||||
)
|
||||
imported_symbols = file_to_symbol_imports.get(caller_file_nid, set())
|
||||
imported_modules = file_to_module_imports.get(caller_file_nid, set())
|
||||
|
||||
@@ -16469,6 +16498,19 @@ def extract(
|
||||
"weight": 1.0,
|
||||
})
|
||||
continue
|
||||
# #1659: a JS/TS DIRECT call with no import evidence is almost always an
|
||||
# unrelated same-named export in a package that was never imported — a
|
||||
# phantom cross-package edge (a 14-package monorepo had `platform` and
|
||||
# `sidecar` shown as depending on `registry-protocol` purely because it
|
||||
# exported generically-named symbols). JS/TS modules have no implicit
|
||||
# cross-module scope, so leave it unresolved rather than binding by name
|
||||
# alone. Other languages keep the #1553 single-candidate resolution:
|
||||
# C/C++ headers, Ruby autoload, and same-package implicit scope
|
||||
# legitimately call across files without an explicit import. Scoped to
|
||||
# direct calls: the indirect_call path above is already conservative
|
||||
# (INFERRED, callable-target-gated) and independent of import evidence.
|
||||
if not has_import_evidence and str(rc.get("source_file", "")).endswith(_JS_TS_CALL_SUFFIXES):
|
||||
continue
|
||||
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
|
||||
|
||||
@@ -879,9 +879,11 @@ def test_cross_file_call_promoted_to_extracted_with_import_evidence(tmp_path):
|
||||
assert call_edges[0]["confidence_score"] == 1.0
|
||||
|
||||
|
||||
def test_cross_file_call_remains_inferred_without_import_evidence(tmp_path):
|
||||
"""A cross-file `calls` edge must stay INFERRED when there is no import
|
||||
edge — name collision alone is insufficient evidence."""
|
||||
def test_js_cross_file_call_without_import_emits_no_edge(tmp_path):
|
||||
"""A JS/TS call with no local definition and no import must NOT bind to a
|
||||
same-named export in another file (#1659). JS/TS modules have no implicit
|
||||
cross-module scope, so name collision alone is not a real call — it used to
|
||||
produce a phantom INFERRED edge that fabricated cross-package dependencies."""
|
||||
caller = tmp_path / "caller.js"
|
||||
callee = tmp_path / "lib.js"
|
||||
# Caller does NOT require lib — same-name function happens to exist elsewhere
|
||||
@@ -898,8 +900,7 @@ def test_cross_file_call_remains_inferred_without_import_evidence(tmp_path):
|
||||
and nodes[e["source"]]["label"] == "run()"
|
||||
and nodes[e["target"]]["label"] == "doUnique()"
|
||||
]
|
||||
assert len(call_edges) == 1
|
||||
assert call_edges[0]["confidence"] == "INFERRED"
|
||||
assert call_edges == [], f"unimported cross-file JS call should not resolve: {call_edges}"
|
||||
|
||||
|
||||
def test_python_qualified_class_method_call_resolves_extracted(tmp_path):
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
"""#1659 — a JS/TS call with no local definition and no import must not bind to
|
||||
a same-named export in an unrelated package that was never imported.
|
||||
|
||||
JS/TS modules have no implicit cross-module scope: a call into another file is
|
||||
real only if the caller imported it. The cross-file resolver used to fall back
|
||||
to any lone same-named export repo-wide and emit a `calls` edge at INFERRED/0.8,
|
||||
so on a monorepo a package that exports generically-named symbols (`*Schema`,
|
||||
`validate`, ...) appeared depended-on by packages that import nothing from it.
|
||||
|
||||
The fix gates JS/TS cross-file call attribution on import evidence; other
|
||||
languages keep the #1553 single-candidate resolution (headers, autoload,
|
||||
same-package implicit scope legitimately call across files with no import).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from graphify.extract import extract
|
||||
|
||||
|
||||
def _write(path: Path, text: str) -> Path:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(text, encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
def _calls(files: list[Path], base: Path) -> set[tuple[str, str, str]]:
|
||||
r = extract(files, cache_root=base, parallel=False)
|
||||
lbl = {n["id"]: n["label"] for n in r["nodes"]}
|
||||
return {
|
||||
(lbl.get(e["source"], ""), lbl.get(e["target"], ""), e.get("confidence"))
|
||||
for e in r["edges"] if e["relation"] == "calls"
|
||||
}
|
||||
|
||||
|
||||
def test_unimported_cross_package_call_emits_no_edge(tmp_path: Path) -> None:
|
||||
_write(tmp_path / "pkg-a/src/index.ts",
|
||||
"declare function validate(x: number): boolean;\n"
|
||||
"export function run(x: number): boolean { return validate(x); }\n")
|
||||
_write(tmp_path / "pkg-b/src/index.ts",
|
||||
"export function validate(name: string): boolean { return name.length > 0; }\n")
|
||||
calls = _calls(sorted(tmp_path.rglob("*.ts")), tmp_path)
|
||||
assert not any("run" in s and "validate" in t for s, t, _ in calls), calls
|
||||
|
||||
|
||||
def test_many_files_do_not_collapse_onto_one_export(tmp_path: Path) -> None:
|
||||
# The real-world symptom: N packages importing nothing all showed edges to a
|
||||
# single package that exported a generically-named symbol.
|
||||
_write(tmp_path / "proto/index.ts",
|
||||
"export function encode(x: string): string { return x; }\n")
|
||||
for i in range(4):
|
||||
_write(tmp_path / f"svc{i}/index.ts",
|
||||
"declare function encode(x: string): string;\n"
|
||||
f"export function use{i}(x: string) {{ return encode(x); }}\n")
|
||||
calls = _calls(sorted(tmp_path.rglob("*.ts")), tmp_path)
|
||||
assert not any("encode" in t for _s, t, _ in calls), calls
|
||||
|
||||
|
||||
def test_imported_cross_file_call_still_resolves(tmp_path: Path) -> None:
|
||||
# A real import must still resolve (and be promoted to EXTRACTED).
|
||||
_write(tmp_path / "a.ts",
|
||||
'import { validate } from "./b";\n'
|
||||
"export function run(x: number) { return validate(x); }\n")
|
||||
_write(tmp_path / "b.ts",
|
||||
"export function validate(name: string): boolean { return name.length > 0; }\n")
|
||||
calls = _calls([tmp_path / "a.ts", tmp_path / "b.ts"], tmp_path)
|
||||
resolved = [c for c in calls if "run" in c[0] and "validate" in c[1]]
|
||||
assert resolved, calls
|
||||
assert resolved[0][2] == "EXTRACTED"
|
||||
|
||||
|
||||
def test_same_file_call_unaffected(tmp_path: Path) -> None:
|
||||
_write(tmp_path / "s.ts",
|
||||
"function helper() { return 1; }\n"
|
||||
"export function main() { return helper(); }\n")
|
||||
calls = _calls([tmp_path / "s.ts"], tmp_path)
|
||||
assert any("main" in s and "helper" in t for s, t, _ in calls), calls
|
||||
|
||||
|
||||
def test_non_js_single_candidate_cross_file_still_resolves(tmp_path: Path) -> None:
|
||||
# The gate is JS/TS-only. Ruby (autoload, no require) legitimately calls a
|
||||
# lone same-named function across files without an import — keep the #1553
|
||||
# single-candidate resolution for it.
|
||||
_write(tmp_path / "helper.rb", "def transform(data)\n data.upcase\nend\n")
|
||||
_write(tmp_path / "main.rb", "def handle(v)\n transform(v)\nend\n")
|
||||
calls = _calls([tmp_path / "main.rb", tmp_path / "helper.rb"], tmp_path)
|
||||
assert any("handle" in s and "transform" in t for s, t, _ in calls), calls
|
||||
Reference in New Issue
Block a user