From 2c7cbb653051ad11637ebfb0719c3d11ae92a1ac Mon Sep 17 00:00:00 2001 From: dschwartzi Date: Wed, 24 Jun 2026 05:52:42 +0300 Subject: [PATCH] Fix: validate_extraction crashes on non-hashable node id / edge endpoint validate_extraction() is documented to return a list of error strings ("empty list means valid"), but raised TypeError: unhashable type: 'list' when a node id -- or an edge source/target -- was a non-hashable value such as a list. This occurs in practice when an LLM extraction subagent emits malformed JSON like {"id": ["foo", "bar"], ...}. Crash sites: the node_ids set comprehension and the `edge[...] not in node_ids` membership tests. Because build_from_json() validates at its start, a single malformed node aborted the entire build, losing an otherwise-complete extraction of a large corpus. build_from_json() itself would also raise (G.add_node() and the `not in node_set` test) if the validator were bypassed. - validate.py: build node_ids during the node pass, adding only hashable ids; report a non-hashable id/endpoint as an error string instead of crashing. All existing messages and the dangling-edge checks are preserved. - build.py: skip dict nodes with a missing/non-hashable id and edges with non-hashable endpoints (stderr warning). Non-dict nodes are deliberately left to raise so the multigraph diagnostic still observes shape errors. - tests: 3 cases in test_validate.py and 2 in test_build.py. --- graphify/build.py | 34 ++++++++++++++++++++++++++-- graphify/validate.py | 33 +++++++++++++++++++++++----- tests/test_build.py | 37 +++++++++++++++++++++++++++++++ tests/test_validate.py | 50 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 7 deletions(-) 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)