fix(json_config): link a package.json dependency's manifest entry to its ref node (#4569, #4572)

Applied from PR #4572 (authorship preserved); stale cache-schema bump dropped for one combined bump.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

graphify/extractors/json_config.py

@@ -234,7 +234,7 @@ def extract_json(path: Path) -> dict:
                     if ref_nid:
                         add_edge(parent_nid, ref_nid, "references", line)
 
-                elif parent_key in _DEP_KEYS and val_text:
+                elif parent_key in _DEP_KEYS and val_text.strip():
                     # Two defects fixed here, both from the #1764 shape:
                     #
                     # 1. The edge ran `key_nid -> dep_nid`, and both nodes carry
@@ -253,10 +253,34 @@ def extract_json(path: Path) -> dict:
                     #    `types` could then be collapsed onto a same-named local
                     #    module by build.py's alias index -- the #1638 failure
                     #    mode reached through a different door.
+                    #
+                    # 3. Moving that edge to the manifest also cut the only link between the
+                    #    dependency's key node (its manifest entry, reached through
+                    #    `dependencies --contains-->`) and its ref node, so the entry and
+                    #    the code that imports the package landed in different communities
+                    #    (#4569). `key_nid --declares--> dep_nid` reconnects them, and the
+                    #    entry is relabelled `name@range` so the two nodes no longer share a
+                    #    label: a same-label edge is the `react -> react` view #1764
+                    #    removed, and two `react` nodes made `explain`/`affected`
+                    #    ambiguous. The ref node keeps the bare name, which is what
+                    #    `import "react"` resolves to. The range is one line, capped at 40
+                    #    characters; a blank one links nothing (like an empty one) and the
+                    #    entry keeps its bare name.
                     dep_nid = _make_id("ref", key)
                     if dep_nid:
                         add_node(dep_nid, key, line, file_type="concept")
                         add_edge(file_nid, dep_nid, "imports", line, context="import")
+                        add_edge(key_nid, dep_nid, "declares", line)
+                        version = " ".join(val_text.split())
+                        if len(version) > 40:
+                            version = version[:37] + "..."
+                        # `add_node` kept the first node under this id (the same key under
+                        # two sibling blocks shares it), so relabel only the pair that
+                        # node's line belongs to: label and line then name one entry.
+                        for node in nodes:
+                            if node["id"] == key_nid and node["source_location"] == f"L{line}":
+                                node["label"] = f"{key}@{version}"
+                                break
 
     # Entry: find root document → object
     doc = root

tests/test_extract.py

@@ -6308,3 +6308,264 @@ def test_python_external_calls_survive_real_incremental_context(tmp_path):
     assert "incremental scan" in incremental_stdout
     assert "AST extraction on 2 code files" in incremental_stdout
     assert incremental_calls == [("ext_a_fetch_a", "requests", "L5"), ("ext_b_fetch_b", "requests", "L4")]
+
+
+# ── #4569: a package.json dependency has a manifest entry (key node) and a ref node ──
+import networkx as nx  # noqa: E402
+
+_PKG = """{
+  "name": "demo",
+  "version": "1.0.0",
+  "scripts": {"build": "vite build"},
+  "%s": {
+    "react": "^19.0.0"
+  }
+}
+"""
+_TSX = 'import React from "react";\nexport const App = () => React.createElement("div");\n'
+_DEP_BLOCKS = ("dependencies", "devDependencies", "peerDependencies", "optionalDependencies")
+
+
+def _built(tmp_path, files):
+    paths = []
+    for name, body in files.items():
+        path = tmp_path / name
+        path.parent.mkdir(parents=True, exist_ok=True)
+        path.write_text(body, encoding="utf-8")
+        paths.append(path)
+    return build_from_json(extract(paths, cache_root=tmp_path, parallel=False), directed=True)
+
+
+def _manifest(G, manifest="package.json"):
+    return next(n for n, d in G.nodes(data=True)
+                if str(d.get("source_file", "")).endswith(manifest) and str(d.get("label", "")).endswith("package.json"))
+
+
+def _entries(G, name, manifest="package.json"):
+    """A manifest's entries for dependency `name`, found by their place in the graph and not by
+    label: every node a dependency block `contains` for it (a fix may decorate the label as
+    `name@range`), plus every node the manifest `imports` under that name."""
+    def is_entry(label):
+        return label == name or str(label).startswith(name + "@")
+
+    root, found = _manifest(G, manifest), []
+    for _, block, d in G.edges(root, data=True):
+        if d.get("relation") == "contains" and G.nodes[block].get("label") in _DEP_BLOCKS:
+            found += [c for _, c, e in G.edges(block, data=True)
+                      if e.get("relation") == "contains" and is_entry(G.nodes[c].get("label"))]
+    found += [t for _, t, d in G.edges(root, data=True)
+              if d.get("relation") == "imports" and is_entry(G.nodes[t].get("label"))]
+    return list(dict.fromkeys(found))
+
+
+def _node(G, label, suffix):
+    return next(n for n, d in G.nodes(data=True)
+                if d.get("label") == label and str(d.get("source_file", "")).endswith(suffix))
+
+
+def _hops(G, a, b):
+    U = G.to_undirected()
+    return nx.shortest_path_length(U, a, b) if nx.has_path(U, a, b) else None
+
+
+def test_a_dependency_entry_is_connected_to_the_code_that_imports_the_package(tmp_path):
+    G = _built(tmp_path, {"package.json": _PKG % "dependencies", "src/main.tsx": _TSX})
+    main, entries = _node(G, "main.tsx", "main.tsx"), _entries(G, "react")
+    assert entries
+    for entry in entries:
+        hops = _hops(G, entry, main)
+        assert hops is not None and hops <= 2, f"{entry} is {hops} hops from main.tsx"
+
+
+@pytest.mark.parametrize("key", _DEP_BLOCKS)
+def test_every_dependency_block_connects_its_entries_to_the_importing_code(tmp_path, key):
+    G = _built(tmp_path, {"package.json": _PKG % key, "src/main.tsx": _TSX})
+    main = _node(G, "main.tsx", "main.tsx")
+    entries = _entries(G, "react")
+    assert entries
+    for entry in entries:
+        hops = _hops(G, entry, main)
+        assert hops is not None and hops <= 2, f"{key}: {entry} is {hops} hops away"
+
+
+def test_no_import_runs_between_two_nodes_with_the_same_label_and_no_edge_is_a_self_loop(tmp_path):
+    G = _built(tmp_path, {"package.json": _PKG % "dependencies", "src/main.tsx": _TSX})
+    for u, v, d in G.edges(data=True):
+        assert u != v, f"self-loop {u}"
+        if d.get("relation") in ("imports", "imports_from"):
+            assert G.nodes[u].get("label") != G.nodes[v].get("label"), f"{d['relation']}: {u} -> {v}"
+
+
+def test_a_dependency_named_like_a_local_module_stays_a_namespaced_external_ref(tmp_path):
+    pkg = '{\n  "name": "demo",\n  "dependencies": {\n    "utils": "^1.0.0"\n  }\n}\n'
+    G = _built(tmp_path, {"package.json": pkg, "src/utils.ts": "export const x = 1;\n"})
+    local = _node(G, "utils.ts", "utils.ts")
+    manifest = _manifest(G)
+    targets = [v for _, v, d in G.edges(manifest, data=True) if d.get("relation") == "imports"]
+    assert targets and local not in targets
+    assert all(t.startswith("ref") and G.nodes[t].get("file_type") == "concept" for t in targets), targets
+
+
+def test_the_line_that_declares_a_dependency_is_still_recorded(tmp_path):
+    G = _built(tmp_path, {"package.json": _PKG % "dependencies", "src/main.tsx": _TSX})
+    react = set(_entries(G, "react"))
+    on_a_node = any(G.nodes[n].get("source_location") == "L6" for n in react)
+    on_an_edge = any(d.get("source_location") == "L6" and str(d.get("source_file", "")).endswith("package.json")
+                     and (u in react or v in react) for u, v, d in G.edges(data=True))
+    assert on_a_node or on_an_edge
+
+
+def test_two_manifests_that_depend_on_react_each_keep_their_declaring_line(tmp_path):
+    a = _PKG % "dependencies"                      # react is on line 6
+    b = '{\n  "name": "b",\n  "dependencies": {\n    "left-pad": "1",\n    "react": "^18.0.0"\n  }\n}\n'  # line 5
+    G = _built(tmp_path, {"a/package.json": a, "a/src/index.tsx": _TSX, "b/package.json": b, "b/src/index.tsx": _TSX + "// b\n"})
+    for manifest, line in (("a/package.json", "L6"), ("b/package.json", "L5")):
+        entries = set(_entries(G, "react", manifest))
+        on_a_node = any(G.nodes[n].get("source_location") == line for n in entries)
+        on_an_edge = any(d.get("source_location") == line and str(d.get("source_file", "")).endswith(manifest)
+                         and (u in entries or v in entries) for u, v, d in G.edges(data=True))
+        assert on_a_node or on_an_edge, f"{manifest} lost its declaring line"
+
+
+def test_keys_that_are_not_dependencies_are_unchanged(tmp_path):
+    path = tmp_path / "package.json"
+    path.write_text('{\n  "name": "demo",\n  "version": "1.0.0",\n  "scripts": {"build": "vite build"}\n}\n')
+    result = extract_json(path)
+    assert sorted(n["label"] for n in result["nodes"]) == ["build", "name", "package.json", "scripts", "version"]
+    assert {e["relation"] for e in result["edges"]} == {"contains"}
+    assert len(result["edges"]) == 4
+
+
+def test_a_cache_entry_from_before_the_dependency_link_is_not_served(tmp_path, monkeypatch):
+    import graphify.cache as cache_mod
+    from graphify.cache import save_cached
+
+    path = tmp_path / "package.json"
+    path.write_text(_PKG % "dependencies", encoding="utf-8")
+    current = cache_mod._AST_CACHE_SCHEMA
+    monkeypatch.setattr(cache_mod, "_AST_CACHE_SCHEMA", 20)  # the schema before this fix (v8, 0.9.93)
+    stale = {"nodes": [{"id": "stale_sentinel", "label": "STALE", "file_type": "code",
+                        "source_file": str(path), "source_location": "L1"}], "edges": []}
+    save_cached(path, stale, tmp_path, cache_root=tmp_path)
+    monkeypatch.setattr(cache_mod, "_AST_CACHE_SCHEMA", current)
+    result = extract([path], cache_root=tmp_path, parallel=False)
+    assert "stale_sentinel" not in {n["id"] for n in result["nodes"]}
+
+
+def test_a_dependency_has_a_single_node_labelled_with_its_bare_name(tmp_path):
+    G = _built(tmp_path, {"package.json": _PKG % "dependencies", "src/main.tsx": _TSX})
+    assert len([n for n, d in G.nodes(data=True) if d.get("label") == "react"]) == 1
+
+
+# Known gap, not part of #4569: with two package.json files the manifests' ref nodes are
+# file-qualified (a_package_json_ref_react) while the code imports an external stub
+# `ref_react`, so no fix to the key/ref split can connect them. strict=True flags the day
+# that is fixed, so this xfail can be turned into a plain test.
+@pytest.mark.xfail(strict=True, reason="two-manifest corpora qualify the manifests' ref ids; the code imports a separate `ref_react` stub")
+def test_in_a_two_manifest_corpus_the_code_reaches_its_own_manifest_entry(tmp_path):
+    a = _PKG % "dependencies"
+    G = _built(tmp_path, {"a/package.json": a, "a/src/index.tsx": _TSX, "b/package.json": a, "b/src/index.tsx": _TSX + "// b\n"})
+    index = _node(G, "a/src/index.tsx", "a/src/index.tsx")
+    entries = _entries(G, "react", "a/package.json")
+    assert entries
+    for entry in entries:
+        hops = _hops(G, entry, index)
+        assert hops is not None and hops <= 3, f"{entry} is {hops} hops from a/src/index.tsx"
+
+
+def test_no_edge_of_any_relation_joins_two_nodes_with_the_same_label(tmp_path):
+    """#1764's concern was a view that reads `react -> react`; H3 only covers imports."""
+    G = _built(tmp_path, {"package.json": _PKG % "dependencies", "src/main.tsx": _TSX})
+    same = [(G.nodes[u]["label"], d.get("relation"), G.nodes[v]["label"]) for u, v, d in G.edges(data=True)
+            if G.nodes[u].get("label") == G.nodes[v].get("label")]
+    assert same == []
+
+
+def test_the_package_name_resolves_to_a_single_node_for_explain_and_affected(tmp_path):
+    from graphify.affected import resolve_seed
+    from graphify.serve import _resolve_single_node
+
+    G = _built(tmp_path, {"package.json": _PKG % "dependencies", "src/main.tsx": _TSX})
+    nid, refusal = _resolve_single_node(G, "react")   # what `graphify explain react` calls
+    assert nid is not None, refusal
+    assert resolve_seed(G, "react") is not None       # what `graphify affected react` calls
+
+
+def test_a_dev_dependency_nothing_imports_is_not_reported_isolated(tmp_path):
+    from graphify.analyze import god_nodes, surprising_connections
+    from graphify.cluster import cluster, score_all
+    from graphify.report import generate
+
+    pkg = '{\n  "name": "demo",\n  "devDependencies": {\n    "@types/react": "^19.0.0"\n  }\n}\n'
+    G = _built(tmp_path, {"package.json": pkg, "src/main.tsx": _TSX})
+    communities = cluster(G)
+    text = generate(G, communities, score_all(G, communities), {c: f"c{c}" for c in communities},
+                    god_nodes(G), surprising_connections(G, communities),
+                    {"total_files": 2, "total_words": 0, "files": {}}, {"input": 0, "output": 0}, str(tmp_path))
+    isolated = [ln for ln in text.splitlines() if "isolated node(s)" in ln]
+    assert not any("@types/react" in ln for ln in isolated), isolated
+
+
+# ── #4569: the manifest entry's label carries its version range ──
+
+def _dep_entry(tmp_path, value):
+    path = tmp_path / "package.json"
+    path.write_text(json.dumps({"name": "demo", "dependencies": {"react": value}}, indent=2) + "\n", encoding="utf-8")
+    result = extract_json(path)
+    entry = next(n for n in result["nodes"] if n["id"].endswith("_dependencies_react"))
+    return entry["label"], result
+
+
+@pytest.mark.parametrize("value,label", [
+    ("^19.0.0", "react@^19.0.0"), ("workspace:*", "react@workspace:*"), ("file:../x", "react@file:../x"),
+    ("npm:other@1.0.0", "react@npm:other@1.0.0"), ("github:user/repo#abc123", "react@github:user/repo#abc123"),
+    ("https://example.com/pkg.tgz", "react@https://example.com/pkg.tgz"),
+    ("^1.0.0   ||   ^2.0.0", "react@^1.0.0 || ^2.0.0"),
+    ("https://example.com/" + "a" * 180, "react@https://example.com/" + "a" * 17 + "..."),
+], ids=["caret", "workspace", "file", "alias", "git", "url", "spaced", "long"])
+def test_the_manifest_entry_label_carries_a_one_line_capped_version(tmp_path, value, label):
+    assert _dep_entry(tmp_path, value)[0] == label
+
+
+def test_the_ref_node_keeps_the_bare_name_whatever_the_range(tmp_path):
+    for value in ("^19.0.0", "workspace:*", "https://example.com/" + "a" * 180):
+        _, result = _dep_entry(tmp_path, value)
+        ref = [n for n in result["nodes"] if n["file_type"] == "concept"]
+        assert [(n["id"], n["label"]) for n in ref] == [("ref_react", "react")]
+
+
+@pytest.mark.parametrize("value", ["", "   "], ids=["empty", "blank"])
+def test_an_empty_or_blank_version_keeps_the_bare_name_and_makes_no_ref_or_link(tmp_path, value):
+    label, result = _dep_entry(tmp_path, value)
+    assert label == "react"
+    assert [n for n in result["nodes"] if n["file_type"] == "concept"] == []
+    assert {e["relation"] for e in result["edges"]} == {"contains"}
+
+
+def test_dependency_values_that_are_not_strings_are_extracted_as_before(tmp_path):
+    path = tmp_path / "package.json"
+    path.write_text(json.dumps({
+        "name": "demo",
+        "dependencies": {"num": 1, "obj": {"version": "1.0.0", "nested": "x"}, "arr": ["a", "b"],
+                         "nul": None, "flag": True, "empty": ""},
+        "devDependencies": {"float": 1.5}}, indent=2) + "\n", encoding="utf-8")
+    result = extract_json(path)
+    assert sorted(n["label"] for n in result["nodes"]) == sorted([
+        "package.json", "name", "dependencies", "num", "obj", "version", "nested", "arr", "nul", "flag",
+        "empty", "devDependencies", "float"])
+    assert {e["relation"] for e in result["edges"]} == {"contains"}
+    assert not [n for n in result["nodes"] if n["file_type"] == "concept"]
+
+
+def test_a_key_shared_by_two_sibling_blocks_is_labelled_with_the_range_on_its_own_line(tmp_path):
+    """Two blocks that each hold `dependencies.semver` mint one key node (its id carries only the
+    immediate parent). Its label and its line must describe the same entry."""
+    text = ('{\n  "name": "demo",\n  "a": {\n    "dependencies": {\n      "semver": "^7.5.0"\n    }\n  },\n'
+            '  "b": {\n    "dependencies": {\n      "semver": "^6.0.0"\n    }\n  }\n}\n')
+    path = tmp_path / "package.json"
+    path.write_text(text, encoding="utf-8")
+    shared = [n for n in extract_json(path)["nodes"] if n["id"].endswith("_dependencies_semver")]
+    assert len(shared) == 1
+    label, line = shared[0]["label"], int(shared[0]["source_location"][1:])
+    assert label.startswith("semver@"), label
+    assert label.split("@", 1)[1] in text.splitlines()[line - 1], (label, line)