fix(js): keep import edges to declared npm packages

graphify/extract.py

@@ -1366,7 +1366,7 @@ def _rescue_js_dynamic_imports(path: Path, result: dict) -> None:
             resolution = _resolve_rescued_specifier(path, raw, aliases, base_url)
             if resolution is None:
                 continue
-            node_id, _stub_sf, resolved_file = resolution
+            node_id, _stub_sf, resolved_file, _is_external = resolution
             # AST-captured already: same resolved target id, same resolved
             # on-disk file, or the engine's ref-namespaced external id.
             if node_id in deferred_ids or _make_id("ref", raw) in deferred_ids:
@@ -1508,10 +1508,10 @@ def _resolve_rescued_specifier(
     raw: str,
     aliases,
     base_url,
-) -> "tuple[str, str, Path | None] | None":
+) -> "tuple[str, str, Path | None, bool] | None":
     """Resolve a regex-rescued import specifier the way ``_import_js`` does.
 
-    Returns ``(node_id, stub_source_file, resolved_file)`` — ``resolved_file``
+    Returns ``(node_id, stub_source_file, resolved_file, is_external)`` — ``resolved_file``
     is the target as a real on-disk file, or None when the specifier is
     external or dangling. Returns None when no target can be minted at all
     (empty bare-import segment). Split out of :func:`_emit_rescued_import` so
@@ -1524,7 +1524,7 @@ def _resolve_rescued_specifier(
             Path(os.path.normpath(path.parent / raw))
         )
         resolved_file = resolved if resolved is not None and resolved.is_file() else None
-        return _make_id(str(resolved)), str(resolved), resolved_file
+        return _make_id(str(resolved)), str(resolved), resolved_file, False
     # Check tsconfig.json path aliases (e.g. "$lib/" -> "src/lib/",
     # "@/" -> "src/") before treating as external. Mirrors _import_js
     # logic so alias imports resolve to the same file node IDs the
@@ -1534,13 +1534,15 @@ def _resolve_rescued_specifier(
         resolved_alias = _resolve_js_module_path(resolved_alias)
         resolved_file = (resolved_alias if resolved_alias is not None
                          and resolved_alias.is_file() else None)
-        return _make_id(str(resolved_alias)), str(resolved_alias), resolved_file
-    # Bare/scoped import (node_modules) - use last segment;
-    # build_from_json drops as external if no matching node exists.
-    module_name = raw.split("/")[-1]
+        return _make_id(str(resolved_alias)), str(resolved_alias), resolved_file, False
+    # Bare/scoped import (node_modules) - anchor to the PACKAGE, not the path
+    # leaf: `lodash/fp/map` is a dependency on `lodash`, and `@a/utils` and
+    # `@b/utils` are different packages that share a leaf.
+    parts = raw.split("/")
+    module_name = "/".join(parts[:2]) if raw.startswith("@") and len(parts) >= 2 else parts[0]
     if not module_name:
         return None
-    return _make_id(module_name), raw, None
+    return _make_id(module_name), module_name, None, True
 
 
 def _emit_rescued_import(
@@ -1573,7 +1575,7 @@ def _emit_rescued_import(
     resolution = _resolve_rescued_specifier(path, raw, aliases, base_url)
     if resolution is None:
         return
-    node_id, stub_source_file, resolved_file = resolution
+    node_id, stub_source_file, resolved_file, is_external = resolution
     edge = {
         "source": file_node_id, "target": node_id,
         "relation": relation, "confidence": "EXTRACTED",
@@ -1589,11 +1591,28 @@ def _emit_rescued_import(
         # Edge target already a real node - just add the edge, don't add a node.
         result.setdefault("edges", []).append(edge)
         return
-    result.setdefault("nodes", []).append({
+    stub: dict = {
         "id": node_id, "label": raw,
         "file_type": "code", "source_file": stub_source_file,
         "confidence": "EXTRACTED",
-    })
+    }
+    if is_external:
+        # A bare/scoped specifier names a MODULE, not a path: the same package
+        # imported from N files is one node, and it is the same package a
+        # manifest in the corpus declares under that name. Mark it the way Swift
+        # module anchors are marked (#1327) — `file_type=code` keeps build.py
+        # validation happy, `type=module` exempts it from id-disambiguation — so
+        # it collapses with that declaration instead of being salted apart from
+        # it. Without this the salt renames the node and every importing edge is
+        # left on the dead id, so a declared dependency silently loses all of its
+        # inbound edges while an undeclared one keeps them (#3084).
+        stub["type"] = "module"
+        # For an external, `stub_source_file` is the package name (see
+        # _resolve_rescued_specifier) — and the node IS that package, so name it
+        # that way. `lodash/fp/map` and `lodash/debounce` are one `lodash` node,
+        # and neither subpath should end up titling it.
+        stub["label"] = stub_source_file
+    result.setdefault("nodes", []).append(stub)
     result.setdefault("edges", []).append(edge)
     existing_ids.add(node_id)
 

tests/test_js_dynamic_imports.py

@@ -275,3 +275,127 @@ def test_dynamic_import_is_traversed_by_affected():
     g.add_edge("importer", "dep", relation="dynamic_import")
     hits = affected_nodes(g, "dep", depth=1)
     assert any(h.node_id == "importer" for h in hits)
+
+
+def test_declared_npm_dependency_keeps_its_import_edge(tmp_path, monkeypatch):
+    """A package declared in package.json must not lose its inbound import edges.
+
+    The bare specifier mints an external stub whose id is the package name; the
+    manifest in the same corpus produces a node under that same name. Two nodes,
+    one id, different source_files -> _disambiguate_colliding_node_ids salted both
+    apart, and the importing edge — which carries neither salt's source key —
+    was left on the now-dead id and dropped at build. The perverse result was
+    that an UNDECLARED package kept its edge (nothing to collide with) while a
+    properly declared dependency became invisible (#3084).
+
+    The stub is a module anchor, like the Swift ones (#1327), so it collapses onto
+    the declaration instead of being salted away from it.
+    """
+    _write(tmp_path / "package.json",
+           '{"name": "mre", "version": "1.0.0", "dependencies": {"postgres": "^3.4.0"}}\n')
+    _write(tmp_path / "app.mts",
+           "const postgres = (await import('postgres')).default;\n"
+           "export async function connect(url: string): Promise<void> {\n"
+           "  postgres(url, { max: 1 });\n"
+           "}\n")
+
+    monkeypatch.chdir(tmp_path)
+    result = extract([Path("app.mts"), Path("package.json")], cache_root=tmp_path / ".cache")
+
+    node_ids = {n["id"] for n in result["nodes"]}
+    pg_edges = [e for e in result["edges"]
+                if e["relation"] in ("dynamic_import", "imports_from")
+                and "postgres" in e["target"]]
+    assert pg_edges, "the dynamic import of a declared dependency must emit an edge"
+    for edge in pg_edges:
+        assert edge["target"] in node_ids, (
+            f"edge target {edge['target']!r} has no node — it would be dropped at build"
+        )
+
+
+def test_undeclared_npm_dependency_still_keeps_its_edge(tmp_path, monkeypatch):
+    """The no-manifest case must keep working: nothing to collapse onto, but the
+    stub still anchors the edge."""
+    _write(tmp_path / "app.mts",
+           "const postgres = (await import('postgres')).default;\n"
+           "export async function connect(url: string): Promise<void> {\n"
+           "  postgres(url, { max: 1 });\n"
+           "}\n")
+
+    monkeypatch.chdir(tmp_path)
+    result = extract([Path("app.mts")], cache_root=tmp_path / ".cache")
+
+    node_ids = {n["id"] for n in result["nodes"]}
+    pg_edges = [e for e in result["edges"]
+                if e["relation"] in ("dynamic_import", "imports_from")
+                and "postgres" in e["target"]]
+    assert pg_edges
+    for edge in pg_edges:
+        assert edge["target"] in node_ids
+
+
+def test_unresolved_relative_import_is_not_marked_a_module(tmp_path, monkeypatch):
+    """Only bare/scoped specifiers are module anchors.
+
+    An unresolved RELATIVE specifier names a path, not a module, so it must stay
+    subject to id-disambiguation — two `./helper` stubs in different directories
+    are different files and must not collapse onto one node.
+    """
+    _write(tmp_path / "app.mts", "const h = (await import('./missing-helper')).default;\n")
+
+    monkeypatch.chdir(tmp_path)
+    result = extract([Path("app.mts")], cache_root=tmp_path / ".cache")
+
+    for node in result["nodes"]:
+        if "missing" in str(node.get("label", "")) or "missing" in node["id"]:
+            assert node.get("type") != "module", (
+                "an unresolved relative import must not be exempted from disambiguation"
+            )
+
+
+def test_scoped_packages_sharing_a_leaf_stay_distinct(tmp_path, monkeypatch):
+    """`@a/utils` and `@b/utils` are different packages that share a path leaf.
+
+    The external stub is anchored to the package name, not the last segment.
+    Anchoring to the leaf gave both the id `utils`, and because a module anchor
+    is exempt from id-disambiguation they then merged into one node — so an
+    import of `@scope-a/utils` resolved to a node labelled `@scope-b/utils`.
+    """
+    _write(tmp_path / "a.mts", "const x = (await import('@scope-a/utils')).default;\nexport const A = 1;\n")
+    _write(tmp_path / "b.mts", "const y = (await import('@scope-b/utils')).default;\nexport const B = 2;\n")
+
+    monkeypatch.chdir(tmp_path)
+    result = extract([Path("a.mts"), Path("b.mts")], cache_root=tmp_path / ".cache")
+
+    stubs = {n["id"]: n for n in result["nodes"] if n.get("type") == "module"}
+    assert len(stubs) == 2, f"the two packages must not share a node: {sorted(stubs)}"
+    assert {n["label"] for n in stubs.values()} == {"@scope-a/utils", "@scope-b/utils"}
+
+    node_ids = {n["id"] for n in result["nodes"]}
+    targets = {e["target"] for e in result["edges"] if e["relation"] == "dynamic_import"}
+    assert len(targets) == 2, f"each import must reach its own package: {targets}"
+    for t in targets:
+        assert t in node_ids
+
+
+def test_subpath_imports_collapse_onto_their_package(tmp_path, monkeypatch):
+    """`lodash/fp/map` is a dependency on `lodash`, not on `map`.
+
+    Every subpath of one package is one package node, and it is named for the
+    package — not for whichever specifier happened to be extracted first.
+    """
+    _write(tmp_path / "a.mts", "const x = (await import('lodash/fp/map')).default;\nexport const A = 1;\n")
+    _write(tmp_path / "b.mts", "const y = (await import('lodash/debounce')).default;\nexport const B = 2;\n")
+
+    monkeypatch.chdir(tmp_path)
+    result = extract([Path("a.mts"), Path("b.mts")], cache_root=tmp_path / ".cache")
+
+    # One stub dict is emitted per importing file; they carry the same id and so
+    # collapse to a single node at build. What matters is that the id and the
+    # name are the package's, whichever subpath minted them.
+    stubs = [n for n in result["nodes"] if n.get("type") == "module"]
+    assert {n["id"] for n in stubs} == {"lodash"}, "both subpaths are the same package"
+    assert {n["label"] for n in stubs} == {"lodash"}, "the node is the package, not one of its subpaths"
+
+    targets = {e["target"] for e in result["edges"] if e["relation"] == "dynamic_import"}
+    assert targets == {"lodash"}