diff --git a/graphify/build.py b/graphify/build.py index 5af6023..75e2d33 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -182,8 +182,25 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat print(f"[graphify] Extraction warning ({len(real_errors)} issues): {real_errors[0]}", file=sys.stderr) G: nx.Graph = nx.DiGraph() if directed else nx.Graph() for node in extraction.get("nodes", []): - if "source_file" in node: - node["source_file"] = _norm_source_file(node["source_file"], _root) + # Skip dict nodes with a missing or non-hashable id (e.g. a list emitted + # by a buggy LLM extraction) so NetworkX add_node never raises + # TypeError: unhashable type. Non-dict nodes are deliberately left to + # raise as before, so callers that probe build for shape errors (e.g. + # the multigraph diagnostic) still observe the malformed shape. + if isinstance(node, dict): + if "id" not in node: + continue + try: + hash(node["id"]) + except TypeError: + print( + f"[graphify] WARNING: skipping node with non-hashable id " + f"{node['id']!r} (must be a string).", + file=sys.stderr, + ) + continue + if "source_file" in node: + node["source_file"] = _norm_source_file(node["source_file"], _root) G.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"}) node_set = set(G.nodes()) @@ -276,6 +293,19 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat if "source" not in edge or "target" not in edge: continue src, tgt = edge["source"], edge["target"] + # Skip edges with non-hashable endpoints (e.g. a list emitted by a buggy + # LLM extraction) so the `not in node_set` membership test below never + # raises TypeError: unhashable type. The validator already reported these. + try: + hash(src) + hash(tgt) + except TypeError: + print( + f"[graphify] WARNING: skipping edge with non-hashable endpoint " + f"(source={src!r}, target={tgt!r}).", + file=sys.stderr, + ) + continue # Remap mismatched IDs via normalization before dropping the edge. if src not in node_set: src = norm_to_id.get(_normalize_id(src), src) diff --git a/graphify/validate.py b/graphify/validate.py index 5f6bad3..bab3ddc 100644 --- a/graphify/validate.py +++ b/graphify/validate.py @@ -17,6 +17,12 @@ def validate_extraction(data: dict) -> list[str]: errors: list[str] = [] + # Collected during the node pass so the edge pass can reuse it. Only + # hashable ids land here; a non-hashable id (e.g. a list emitted by a + # malformed LLM extraction) is reported as an error rather than crashing + # the validator on set construction. + node_ids: set = set() + # Nodes if "nodes" not in data: errors.append("Missing required key 'nodes'") @@ -30,6 +36,15 @@ def validate_extraction(data: dict) -> list[str]: for field in REQUIRED_NODE_FIELDS: if field not in node: errors.append(f"Node {i} (id={node.get('id', '?')!r}) missing required field '{field}'") + if "id" in node: + try: + hash(node["id"]) + except TypeError: + errors.append( + f"Node {i} has non-hashable id {node['id']!r} - id must be a string" + ) + else: + node_ids.add(node["id"]) if "file_type" in node and node["file_type"] not in VALID_FILE_TYPES: errors.append( f"Node {i} (id={node.get('id', '?')!r}) has invalid file_type " @@ -43,7 +58,6 @@ def validate_extraction(data: dict) -> list[str]: elif not isinstance(edge_list, list): errors.append("'edges' must be a list") else: - node_ids = {n["id"] for n in data.get("nodes", []) if isinstance(n, dict) and "id" in n} for i, edge in enumerate(edge_list): if not isinstance(edge, dict): errors.append(f"Edge {i} must be an object") @@ -56,10 +70,19 @@ def validate_extraction(data: dict) -> list[str]: f"Edge {i} has invalid confidence '{edge['confidence']}' " f"- must be one of {sorted(VALID_CONFIDENCES)}" ) - if "source" in edge and node_ids and edge["source"] not in node_ids: - errors.append(f"Edge {i} source '{edge['source']}' does not match any node id") - if "target" in edge and node_ids and edge["target"] not in node_ids: - errors.append(f"Edge {i} target '{edge['target']}' does not match any node id") + for endpoint in ("source", "target"): + if endpoint not in edge: + continue + val = edge[endpoint] + try: + unmatched = bool(node_ids) and val not in node_ids + except TypeError: + errors.append( + f"Edge {i} {endpoint} {val!r} is non-hashable - must be a string" + ) + continue + if unmatched: + errors.append(f"Edge {i} {endpoint} '{val}' does not match any node id") return errors diff --git a/tests/test_build.py b/tests/test_build.py index 6789f04..aeedeee 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -677,3 +677,40 @@ def test_build_merge_rejects_oversized_existing_graph(monkeypatch, tmp_path): monkeypatch.setattr("graphify.security._MAX_GRAPH_FILE_BYTES", 8) with pytest.raises(ValueError, match="exceeds"): build_merge([], graph_path, dedup=False) + + +def test_build_from_json_skips_non_hashable_node_id(): + # A malformed LLM extraction can emit a list-valued id; build_from_json must + # skip it (NetworkX add_node would otherwise raise unhashable type) and still + # build the graph from the well-formed nodes. + extraction = { + "nodes": [ + {"id": "a", "label": "A", "file_type": "code", "source_file": "a.py"}, + {"id": ["x", "y"], "label": "B", "file_type": "code", "source_file": "b.py"}, + {"label": "C", "file_type": "code", "source_file": "c.py"}, # missing id + ], + "edges": [], + } + G = build_from_json(extraction) + assert set(G.nodes()) == {"a"} + + +def test_build_from_json_skips_edge_with_non_hashable_endpoint(): + # A list-valued edge endpoint must be skipped rather than crash the + # `not in node_set` membership test. The well-formed edge survives. + extraction = { + "nodes": [ + {"id": "a", "label": "A", "file_type": "code", "source_file": "a.py"}, + {"id": "b", "label": "B", "file_type": "code", "source_file": "b.py"}, + ], + "edges": [ + {"source": "a", "target": ["b", "c"], "relation": "calls", + "confidence": "INFERRED", "source_file": "a.py"}, + {"source": "a", "target": "b", "relation": "imports", + "confidence": "EXTRACTED", "source_file": "a.py"}, + ], + } + G = build_from_json(extraction) + assert G.number_of_nodes() == 2 + assert G.number_of_edges() == 1 + assert G.has_edge("a", "b") diff --git a/tests/test_validate.py b/tests/test_validate.py index 396e90c..ea865fa 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -85,3 +85,53 @@ def test_assert_valid_raises_on_errors(): def test_assert_valid_passes_silently(): assert_valid(VALID) # should not raise + + +def test_non_hashable_node_id_reported_not_raised(): + # A malformed LLM extraction can emit a list-valued id. The validator must + # report it as an error string (its documented contract) rather than crash + # with TypeError: unhashable type on set construction. + data = { + "nodes": [ + {"id": "n1", "label": "A", "file_type": "code", "source_file": "a.py"}, + {"id": ["x", "y"], "label": "B", "file_type": "code", "source_file": "b.py"}, + ], + "edges": [], + } + errors = validate_extraction(data) + assert any("non-hashable id" in e for e in errors) + + +def test_non_hashable_edge_endpoint_reported_not_raised(): + # A list-valued edge source/target must be reported, not crash the + # membership test against the node-id set. + data = { + "nodes": [ + {"id": "n1", "label": "A", "file_type": "code", "source_file": "a.py"}, + {"id": "n2", "label": "B", "file_type": "code", "source_file": "b.py"}, + ], + "edges": [ + {"source": "n1", "target": ["n2", "n3"], "relation": "calls", + "confidence": "INFERRED", "source_file": "a.py"}, + ], + } + errors = validate_extraction(data) + assert any("target" in e and "non-hashable" in e for e in errors) + + +def test_non_hashable_node_id_does_not_mask_valid_ids(): + # The valid node id must still be collected so a legitimately-dangling edge + # is still flagged even when a sibling node has a bad id. + data = { + "nodes": [ + {"id": "n1", "label": "A", "file_type": "code", "source_file": "a.py"}, + {"id": {"oops": 1}, "label": "B", "file_type": "code", "source_file": "b.py"}, + ], + "edges": [ + {"source": "n1", "target": "ghost", "relation": "calls", + "confidence": "EXTRACTED", "source_file": "a.py"}, + ], + } + errors = validate_extraction(data) + assert any("non-hashable id" in e for e in errors) + assert any("target" in e and "ghost" in e for e in errors)