diff --git a/graphify/build.py b/graphify/build.py index 4c9a9ba29..7b360d21f 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -862,6 +862,27 @@ def edge_datas(G: nx.Graph, u: str, v: str) -> list[dict]: return [raw] +def has_parallel_links(links: "list[dict]", directed: bool = True) -> bool: + """True when any two link dicts share the same endpoints. + + Used to keep the serialized ``multigraph`` flag consistent with the links + actually stored (#4500): a simple-graph write-back can restore parallel + links from ``original_links`` while ``node_link_data`` derived + ``multigraph: false`` from the in-memory graph, and readers that honour + the flag then collapse the parallel edges on load. + """ + seen: set = set() + for link in links or []: + if not isinstance(link, dict): + continue + endpoints = (link.get("source"), link.get("target")) + key = endpoints if directed else frozenset(endpoints) + if key in seen: + return True + seen.add(key) + return False + + def dedupe_nodes(nodes: list[dict]) -> list[dict]: """Collapse nodes sharing an ``id``, last-writer-wins on attributes. diff --git a/graphify/export.py b/graphify/export.py index e61416ec6..12d114404 100644 --- a/graphify/export.py +++ b/graphify/export.py @@ -15,7 +15,7 @@ import networkx as nx from networkx.readwrite import json_graph from graphify.security import sanitize_label from graphify.analyze import _node_community_map -from graphify.build import edge_data +from graphify.build import edge_data, has_parallel_links from graphify.paths import ( os_replace_with_fallback, stem_filename_budget, @@ -404,6 +404,11 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *, if isinstance(link, dict) and link.get("source") in node_ids and link.get("target") in node_ids ] + # The flag above was derived from the in-memory simple graph, but the + # restored links may hold parallel edges (#3999); keep the serialized + # metadata consistent with the links or readers collapse them (#4500). + if has_parallel_links(data["links"], directed=bool(data.get("directed", True))): + data["multigraph"] = True for link in data["links"]: if "confidence_score" not in link: conf = link.get("confidence", "EXTRACTED") diff --git a/graphify/serve.py b/graphify/serve.py index 4b5c0e81b..638aa07e0 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -14,7 +14,7 @@ from typing import NamedTuple import networkx as nx from networkx.readwrite import json_graph from graphify.security import sanitize_label, check_graph_file_size_cap, _CONTROL_CHAR_RE -from graphify.build import edge_data, edge_datas +from graphify.build import edge_datas, has_parallel_links from graphify.paths import default_graph_json as _default_graph_json try: @@ -61,6 +61,13 @@ def _read_graph(graph_path: str) -> nx.Graph: # not mistake the override for graph truth. _logical_directed = bool(data.get("directed", False)) data = {**data, "directed": True} + # Repair files whose links hold parallel edges while the serialized flag says + # otherwise (#4500): older writers restored original_links verbatim while + # node_link_data derived "multigraph": false from the simple graph. Forced the + # same way "directed" is forced above, so the load keeps every relationship + # instead of collapsing parallel edges. + if has_parallel_links(data.get("links"), directed=True): + data = {**data, "multigraph": True} try: from graphify.build import graph_has_legacy_ids as _legacy if _legacy(data.get("nodes", [])): @@ -1028,8 +1035,9 @@ def _complete_induced_edges(G: nx.Graph, visited: set[str], edges_seen: list[tup Dedup keys on the ordered pair for directed graphs and the unordered pair otherwise: on a DiGraph `u->v` and `v->u` are genuinely distinct edges (mutual recursion, circular imports), and collapsing them would drop a real - one. On a multigraph parallel edges collapse to one entry, matching the - renderer, which already shows only the first (`_subgraph_to_text`). + one. On a multigraph the traversal still records one entry per pair; the + renderer expands it into one line per parallel edge (`_subgraph_to_text`, + #4500). Traversal edges keep their discovery order; completions are appended after. """ @@ -1200,39 +1208,42 @@ def _subgraph_to_text(G: nx.Graph, nodes: set[str], edges: list[tuple], token_bu lines.append(line) for u, v in edges: if u in nodes and v in nodes: - raw = G[u][v] - d = next(iter(raw.values()), {}) if isinstance(G, (nx.MultiGraph, nx.MultiDiGraph)) else raw - # (u, v) is BFS/DFS visit order, not necessarily the true edge - # direction: on an undirected graph G.neighbors() walks callers - # and callees alike, so a caller->callee edge renders backwards - # whenever the callee is visited first. _src/_tgt (stashed on the - # edge data by the `query` CLI loader) carry the real direction; - # fall back to (u, v) for graphs/edges that don't set them. - src = d.get("_src", u) - tgt = d.get("_tgt", v) - # Guard against a stray/dangling _src/_tgt (hand-edited or adversarial - # graph.json): only trust them when they name exactly this edge's - # endpoints, else fall back to (u, v). Without this, G.nodes[src] - # would KeyError on an unknown id (#2080 review). - if {src, tgt} != {u, v}: - src, tgt = u, v - context = d.get("context") - context_suffix = f" context={sanitize_label(str(context))}" if context else "" - # The relation SITE (call/import/reference line in the source's - # file), not a def line — so "who calls X" cites a clickable call - # location, not the caller's def (#BUG1). - _loc = str(d.get("source_location") or "") - at_suffix = ( - f" at={sanitize_label(str(d.get('source_file') or ''))}:{sanitize_label(_loc)}" - if _loc else "" - ) - line = ( - f"EDGE {sanitize_label(G.nodes[src].get('label', src))} " - f"--{sanitize_label(str(d.get('relation', '')))} " - f"[{sanitize_label(str(d.get('confidence', '')))}{context_suffix}]--> " - f"{sanitize_label(G.nodes[tgt].get('label', tgt))}{at_suffix}" - ) - lines.append(line) + # One EDGE line per relationship: on a multigraph a pair can + # carry several edges (e.g. contains + calls), each with its + # own metadata (#4500). The traversal records one entry per + # pair; expanding here keeps every relationship visible. + for d in edge_datas(G, u, v): + # (u, v) is BFS/DFS visit order, not necessarily the true edge + # direction: on an undirected graph G.neighbors() walks callers + # and callees alike, so a caller->callee edge renders backwards + # whenever the callee is visited first. _src/_tgt (stashed on the + # edge data by the `query` CLI loader) carry the real direction; + # fall back to (u, v) for graphs/edges that don't set them. + src = d.get("_src", u) + tgt = d.get("_tgt", v) + # Guard against a stray/dangling _src/_tgt (hand-edited or adversarial + # graph.json): only trust them when they name exactly this edge's + # endpoints, else fall back to (u, v). Without this, G.nodes[src] + # would KeyError on an unknown id (#2080 review). + if {src, tgt} != {u, v}: + src, tgt = u, v + context = d.get("context") + context_suffix = f" context={sanitize_label(str(context))}" if context else "" + # The relation SITE (call/import/reference line in the source's + # file), not a def line — so "who calls X" cites a clickable call + # location, not the caller's def (#BUG1). + _loc = str(d.get("source_location") or "") + at_suffix = ( + f" at={sanitize_label(str(d.get('source_file') or ''))}:{sanitize_label(_loc)}" + if _loc else "" + ) + line = ( + f"EDGE {sanitize_label(G.nodes[src].get('label', src))} " + f"--{sanitize_label(str(d.get('relation', '')))} " + f"[{sanitize_label(str(d.get('confidence', '')))}{context_suffix}]--> " + f"{sanitize_label(G.nodes[tgt].get('label', tgt))}{at_suffix}" + ) + lines.append(line) output = "\n".join(lines) if len(output) > char_budget: cut_at = output[:char_budget].rfind("\n") @@ -2079,6 +2090,42 @@ def _community_header(cid: int, community_name) -> str: return base +def _neighbor_lines(G: nx.Graph, nid: str, rel_filter: str = "") -> list[str]: + """One output line per relationship incident to ``nid``. + + Parallel edges between the same pair each render as their own line with + their own metadata (#4500); ``edge_data()`` would return only the first. + """ + def _edge_at(d: dict) -> str: + # Edge location = the relation SITE (call/import line) in the source + # node's file, not a def line (#BUG1). + loc = str(d.get("source_location") or "") + return ( + f" at={sanitize_label(str(d.get('source_file') or ''))}:{sanitize_label(loc)}" + if loc else "" + ) + lines: list[str] = [] + for nb in G.successors(nid): + for d in edge_datas(G, nid, nb): + rel = d.get("relation", "") + if rel_filter and rel_filter not in rel.lower(): + continue + lines.append( + f" --> {sanitize_label(G.nodes[nb].get('label', nb))} " + f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]{_edge_at(d)}" + ) + for nb in G.predecessors(nid): + for d in edge_datas(G, nb, nid): + rel = d.get("relation", "") + if rel_filter and rel_filter not in rel.lower(): + continue + lines.append( + f" <-- {sanitize_label(G.nodes[nb].get('label', nb))} " + f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]{_edge_at(d)}" + ) + return lines + + def _build_server(graph_path: str): """Build the configured low-level MCP Server (shared by every transport). @@ -2380,32 +2427,7 @@ def _build_server(graph_path: str): if err: return err lines = [f"Neighbors of {sanitize_label(G.nodes[nid].get('label', nid))}:"] - def _edge_at(d: dict) -> str: - # Edge location = the relation SITE (call/import line) in the source - # node's file, not a def line (#BUG1). - loc = str(d.get("source_location") or "") - return ( - f" at={sanitize_label(str(d.get('source_file') or ''))}:{sanitize_label(loc)}" - if loc else "" - ) - for nb in G.successors(nid): - d = edge_data(G, nid, nb) - rel = d.get("relation", "") - if rel_filter and rel_filter not in rel.lower(): - continue - lines.append( - f" --> {sanitize_label(G.nodes[nb].get('label', nb))} " - f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]{_edge_at(d)}" - ) - for nb in G.predecessors(nid): - d = edge_data(G, nb, nid) - rel = d.get("relation", "") - if rel_filter and rel_filter not in rel.lower(): - continue - lines.append( - f" <-- {sanitize_label(G.nodes[nb].get('label', nb))} " - f"[{sanitize_label(str(rel))}] [{sanitize_label(str(d.get('confidence', '')))}]{_edge_at(d)}" - ) + lines.extend(_neighbor_lines(G, nid, rel_filter)) budget = int(arguments.get("token_budget", 2000)) return _cut_lines_to_budget( lines, budget, "Narrow with relation_filter or use get_node for a specific symbol" diff --git a/tests/test_build.py b/tests/test_build.py index 1ce7834e2..e27351a4b 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -2081,3 +2081,34 @@ def test_method_ghost_ambiguous_same_name_in_one_file_is_not_merged_3705(): assert not G.has_edge("doc", "svc_a_save_state") assert not G.has_edge("doc", "svc_b_save_state") + + +def test_has_parallel_links_detects_repeated_pairs(): + from graphify.build import has_parallel_links + links = [ + {"source": "a", "target": "b", "relation": "contains"}, + {"source": "a", "target": "b", "relation": "calls"}, + {"source": "b", "target": "c", "relation": "calls"}, + ] + assert has_parallel_links(links) is True + + +def test_has_parallel_links_false_without_repeats(): + from graphify.build import has_parallel_links + links = [ + {"source": "a", "target": "b", "relation": "calls"}, + {"source": "b", "target": "c", "relation": "calls"}, + ] + assert has_parallel_links(links) is False + assert has_parallel_links([]) is False + assert has_parallel_links(None) is False + + +def test_has_parallel_links_antiparallel_is_not_parallel_when_directed(): + from graphify.build import has_parallel_links + links = [ + {"source": "a", "target": "b", "relation": "calls"}, + {"source": "b", "target": "a", "relation": "calls"}, + ] + assert has_parallel_links(links, directed=True) is False + assert has_parallel_links(links, directed=False) is True diff --git a/tests/test_cli_export.py b/tests/test_cli_export.py index 722b031c7..884019111 100644 --- a/tests/test_cli_export.py +++ b/tests/test_cli_export.py @@ -1196,3 +1196,61 @@ def test_cluster_only_preserves_parallel_edges_across_an_id_remap(tmp_path): ("claude_claude_alpha", "pkg_beta", "mentions"), ("claude_claude_alpha", "pkg_beta", "references"), ], pairs + + +def test_to_json_declares_multigraph_when_original_links_have_parallel_edges(tmp_path): + """#4500: to_json restored original_links (which may hold parallel edges, + #3999) while node_link_data derived "multigraph": false from the simple + in-memory graph. The flag must match the links actually written.""" + from graphify.build import build_from_json + from graphify.cluster import cluster + from graphify.export import to_json + + extraction = { + "directed": True, + "nodes": [ + {"id": "a", "label": "App()", "file_type": "code", "source_file": "src/App.tsx"}, + {"id": "b", "label": "applySeed()", "file_type": "code", "source_file": "src/App.tsx"}, + ], + "links": [ + {"source": "a", "target": "b", "relation": "contains", "confidence": "EXTRACTED", + "source_file": "src/App.tsx", "source_location": "80"}, + {"source": "a", "target": "b", "relation": "calls", "confidence": "EXTRACTED", + "source_file": "src/App.tsx", "source_location": "163"}, + ], + } + G = build_from_json(extraction, directed=True) + communities = cluster(G) + out = tmp_path / "graph.json" + assert to_json(G, communities, str(out), original_links=extraction["links"]) + + data = json.loads(out.read_text()) + assert data["multigraph"] is True + pairs = sorted((l["source"], l["target"], l["relation"]) for l in data["links"]) + assert pairs == [("a", "b", "calls"), ("a", "b", "contains")], pairs + + +def test_to_json_keeps_multigraph_false_without_parallel_edges(tmp_path): + """#4500: the flag repair must not flip graphs that have no parallel edges.""" + from graphify.build import build_from_json + from graphify.cluster import cluster + from graphify.export import to_json + + extraction = { + "directed": True, + "nodes": [ + {"id": "a", "label": "a", "file_type": "code", "source_file": "a.py"}, + {"id": "b", "label": "b", "file_type": "code", "source_file": "a.py"}, + ], + "links": [ + {"source": "a", "target": "b", "relation": "calls", "confidence": "EXTRACTED"}, + ], + } + G = build_from_json(extraction, directed=True) + communities = cluster(G) + out = tmp_path / "graph.json" + assert to_json(G, communities, str(out), original_links=extraction["links"]) + + data = json.loads(out.read_text()) + assert data["multigraph"] is False + assert len(data["links"]) == 1 diff --git a/tests/test_serve.py b/tests/test_serve.py index 0cfe882af..224f9786b 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -34,6 +34,7 @@ from graphify.serve import ( _subgraph_to_text, _cut_lines_to_budget, _load_graph, + _neighbor_lines, _community_header, _search_tokens, _shortest_path_text, @@ -1905,3 +1906,88 @@ def test_dotted_python_symbol_same_file_tie_is_refused_by_path_endpoint(): G.add_node("m", label=".run()", source_file="pkg/mod.py", source_location="L9") nid, _, err = _resolve_path_endpoint(G, "pkg.mod.run") assert nid is None and err.startswith("Ambiguous") +# --- parallel-edge preservation (#4500) --- + +def _parallel_edge_graph_json(tmp_path): + """A graph.json like the #4500 reporter's: parallel links stored while the + serialized flag says multigraph:false.""" + p = tmp_path / "graph.json" + p.write_text(json.dumps({ + "directed": True, "multigraph": False, "graph": {}, + "nodes": [ + {"id": "src_app_app", "label": "App()"}, + {"id": "src_app_app_applyseed", "label": "applySeed()"}, + ], + "links": [ + {"source": "src_app_app", "target": "src_app_app_applyseed", + "relation": "contains", "confidence": "EXTRACTED", + "source_file": "src/App.tsx", "source_location": "80"}, + {"source": "src_app_app", "target": "src_app_app_applyseed", + "relation": "calls", "confidence": "EXTRACTED", + "source_file": "src/App.tsx", "source_location": "163"}, + ], + })) + return str(p) + + +def test_load_graph_repairs_multigraph_false_with_parallel_links(tmp_path): + G = _load_graph(_parallel_edge_graph_json(tmp_path)) + assert G.is_multigraph() + # graph_stats reports G.number_of_edges(): the reporter's 134 stored links + # became 131 loaded edges; here both must survive. + assert G.number_of_edges() == 2 + got = sorted( + d.get("relation") + for u, v, d in G.edges(data=True) + if (u, v) == ("src_app_app", "src_app_app_applyseed") + ) + assert got == ["calls", "contains"], got + + +def test_load_graph_keeps_simple_graph_simple(tmp_path): + p = tmp_path / "graph.json" + p.write_text(json.dumps({ + "directed": True, "multigraph": False, "graph": {}, + "nodes": [{"id": "a", "label": "a"}, {"id": "b", "label": "b"}], + "links": [{"source": "a", "target": "b", "relation": "calls", "confidence": "EXTRACTED"}], + })) + G = _load_graph(str(p)) + assert not G.is_multigraph() + assert G.number_of_edges() == 1 + + +def test_neighbor_lines_shows_every_parallel_relationship(tmp_path): + G = _load_graph(_parallel_edge_graph_json(tmp_path)) + lines = _neighbor_lines(G, "src_app_app") + assert len(lines) == 2, lines + assert any("[contains]" in l and "App.tsx:80" in l for l in lines), lines + assert any("[calls]" in l and "App.tsx:163" in l for l in lines), lines + # each relationship keeps its own metadata: no combined/merged edge + assert len({l for l in lines}) == 2 + + +def test_neighbor_lines_relation_filter_applies_per_edge(tmp_path): + G = _load_graph(_parallel_edge_graph_json(tmp_path)) + lines = _neighbor_lines(G, "src_app_app", rel_filter="calls") + assert len(lines) == 1, lines + assert "[calls]" in lines[0] + + +def test_subgraph_to_text_renders_every_parallel_edge(tmp_path): + G = _load_graph(_parallel_edge_graph_json(tmp_path)) + text = _subgraph_to_text( + G, + {"src_app_app", "src_app_app_applyseed"}, + [("src_app_app", "src_app_app_applyseed")], + ) + edge_lines = [l for l in text.splitlines() if l.startswith("EDGE ")] + assert len(edge_lines) == 2, edge_lines + assert any("--contains " in l and "App.tsx:80" in l for l in edge_lines), edge_lines + assert any("--calls " in l and "App.tsx:163" in l for l in edge_lines), edge_lines + + +def test_query_graph_text_surfaces_parallel_relationships(tmp_path): + G = _load_graph(_parallel_edge_graph_json(tmp_path)) + text = _query_graph_text(G, "applySeed", token_budget=2000) + assert "--contains " in text, text + assert "--calls " in text, text