Files
graphify/tests/test_non_string_node_ids.py
Rishet Mehra a2dc3d9ce2 fix(build): coerce non-string node ids so a numeric id cannot abort the build
A backend can emit {"id": 10} where the schema says {"id": "10"}. Every id
consumer downstream assumes str, so one numeric id aborted the whole run at
the final merge step, after the full extraction had already been paid for.

There are three crash sites, not the one in the report:

  - dedup._pick_winner's _CHUNK_SUFFIX.search(n["id"]) raises
    "expected string or bytes-like object, got 'int'" (the reported
    traceback), but only when the node lands in a duplicate or fuzzy group
  - build_from_json's sorted(node_set) raises "'<' not supported between
    instances of 'str' and 'int'" for a lone node with nothing to dedup
  - the same sort, reached via the fuzzy-group path

A str() cast at the reported line therefore does not fix the common case:
a single numeric-id node never reaches _pick_winner. Coerce at ingest
instead, in _coerce_non_string_ids, called from build() before dedup (which
keys on id) and from build_from_json (the direct entry used by cli.py,
watch.py and diagnostics.py, which never goes through build()). The nested
call on the build() path is an idempotent no-op.

Edge endpoints and hyperedge members are coerced alongside the nodes on
purpose: coercing node ids alone would renumber 10 to "10" and leave every
edge pointing at the vanished 10, trading a loud crash for a silently
disconnected graph. The legacy from/to aliases are included because dedup
reads them directly (#803).

Only bool-free numeric scalars are coerced. A None, list or dict id is left
for validate_extraction to report, since str(None) == "None" would fabricate
an id that no edge references.

Closes #2326
2026-08-01 11:36:30 +01:00

137 lines
4.7 KiB
Python

"""Non-string node ids from LLM backends must not crash the build (#2326).
A backend can emit ``{"id": 10}`` where the schema says ``{"id": "10"}``. Every
id consumer downstream assumes ``str``, so an int id used to abort the whole
build in three different places. These tests pin the crash sites and the
edge/hyperedge linkage that a node-only coercion would silently break.
"""
import networkx as nx
import pytest
from graphify.build import build, build_from_json
def _node(nid, label, **kw):
return {
"id": nid,
"label": label,
"file_type": "concept",
"source_file": "a.py",
**kw,
}
def _edge(src, tgt):
return {"source": src, "target": tgt, "relation": "uses", "confidence": "EXTRACTED"}
def test_pick_winner_survives_int_id_in_duplicate_group():
"""dedup._pick_winner regex-searched the raw id (the issue's traceback).
Driven through ``build`` because that is dedup's only production caller, so
``build`` is where the coercion has to land for this path to be fixed.
"""
ext = {"nodes": [_node(10, "Alpha"), _node("alpha_c1", "Alpha")], "edges": []}
G = build([ext], dedup=True)
assert all(isinstance(nid, str) for nid in G.nodes)
def test_build_accepts_a_single_int_id_node_with_no_duplicate():
"""build_from_json's sorted(node_set) crashed even with nothing to dedup."""
ext = {"nodes": [_node(10, "Alpha"), _node("b", "Beta")], "edges": [_edge(10, "b")]}
G = build([ext], dedup=True)
assert "10" in G.nodes
assert 10 not in G.nodes
def test_int_id_endpoints_stay_connected_after_coercion():
"""Coercing node ids without coercing endpoints would orphan the edge."""
ext = {"nodes": [_node(10, "Alpha"), _node(20, "Beta")], "edges": [_edge(10, 20)]}
G = build([ext], dedup=True)
assert G.has_edge("10", "20")
def test_int_id_survives_a_fuzzy_dedup_group():
ext = {
"nodes": [_node(10, "PaymentProcessor"), _node("b", "PaymentProcessors")],
"edges": [_edge(10, "b")],
}
G = build([ext], dedup=True)
assert all(isinstance(nid, str) for nid in G.nodes)
def test_float_id_is_coerced_too():
ext = {"nodes": [_node(1.5, "Alpha"), _node("b", "Beta")], "edges": [_edge(1.5, "b")]}
G = build([ext], dedup=True)
assert G.has_edge("1.5", "b")
def test_legacy_from_to_endpoints_are_coerced():
"""dedup reads the legacy from/to aliases (#803), so they need it as well."""
ext = {
"nodes": [_node(10, "Alpha"), _node("b", "Beta")],
"edges": [{"from": 10, "to": "b", "relation": "uses", "confidence": "EXTRACTED"}],
}
G = build([ext], dedup=True)
assert G.has_edge("10", "b")
def test_hyperedge_members_are_coerced_with_their_nodes():
ext = {
"nodes": [_node(10, "Alpha"), _node("b", "Beta")],
"edges": [],
"hyperedges": [{"id": "he1", "label": "grp", "nodes": [10, "b"]}],
}
G = build([ext], dedup=True)
members = G.graph["hyperedges"][0]["nodes"]
assert members == ["10", "b"]
def test_build_from_json_coerces_on_the_direct_entry():
"""Reloading a persisted graph does not go through build()/dedup."""
G = build_from_json({"nodes": [_node(10, "Alpha")], "edges": []})
assert list(G.nodes) == ["10"]
def test_numeric_endpoint_with_no_matching_node_matches_the_string_case():
"""A numeric endpoint with no node of its own must behave like a string one.
Both are dangling references, which build_from_json drops — the point is that
coercion makes the int indistinguishable from the str, rather than crashing
or leaving a half-typed endpoint behind.
"""
def graph_for(target):
G = build_from_json(
{"nodes": [_node("a", "Alpha")], "edges": [_edge("a", target)]}
)
return sorted(G.nodes), sorted(G.edges)
assert graph_for(99) == graph_for("99")
@pytest.mark.parametrize("bad", [None, ["x"], {"k": "v"}])
def test_non_scalar_ids_are_left_for_validation(bad):
"""Only numeric scalars are coerced; str(None) == 'None' would be a lie."""
from graphify.build import _coerce_non_string_ids
ext = {"nodes": [{"id": bad, "label": "Alpha"}], "edges": []}
_coerce_non_string_ids(ext)
assert ext["nodes"][0]["id"] == bad
def test_bool_id_is_not_coerced():
from graphify.build import _coerce_non_string_ids
ext = {"nodes": [{"id": True, "label": "Alpha"}], "edges": []}
_coerce_non_string_ids(ext)
assert ext["nodes"][0]["id"] is True
def test_string_ids_are_untouched():
"""Regression guard: the normal path must be byte-identical."""
ext = {"nodes": [_node("a", "Alpha"), _node("b", "Beta")], "edges": [_edge("a", "b")]}
G = build([ext], dedup=True)
assert isinstance(G, nx.Graph)
assert set(G.nodes) == {"a", "b"}
assert G.has_edge("a", "b")