fix(build): coerce non-string node ids so a numeric id cannot abort the build

graphify/build.py

@@ -157,6 +157,62 @@ def _fold_edge_aliases(edge: dict) -> None:
         edge["confidence"] = "INFERRED"
 
 
+def _coerce_id(value: object) -> object:
+    """Return a str for a numeric id, else the value unchanged.
+
+    ``bool`` is excluded despite subclassing ``int``: an id of ``True`` is not a
+    number the model meant to name a node, and ``"True"`` would invent a label.
+    """
+    if isinstance(value, bool) or not isinstance(value, (int, float)):
+        return value
+    return str(value)
+
+
+def _coerce_non_string_ids(extraction: dict) -> None:
+    """Coerce numeric node ids and edge/hyperedge references to str, in place (#2326).
+
+    A backend can emit ``{"id": 10}`` where the schema says ``{"id": "10"}``.
+    Every id consumer downstream assumes ``str``, so one int id aborted the build
+    in three places: ``_pick_winner``'s ``_CHUNK_SUFFIX.search(n["id"])`` raised
+    ``TypeError: expected string or bytes-like object``, and ``build_from_json``'s
+    ``sorted(node_set)`` raised ``'<' not supported between instances of 'str'
+    and 'int'`` — the latter for a lone node with nothing to dedup at all.
+    Coercing keeps the node and its edges rather than dropping either, which is
+    the same tolerate-and-heal treatment loose backend output already gets at the
+    parse chokepoint (#1631) and in the alias folds (#2194).
+
+    Endpoints and hyperedge members are coerced with the nodes, not after: a
+    node-only coercion 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`` endpoint aliases are included
+    because dedup reads them directly (#803).
+
+    Runs in BOTH ``build`` (before dedup, which keys on id) and
+    ``build_from_json`` (the direct entry that reloads a persisted graph), for
+    the same two-site reason as the ``_fold_node_aliases`` fold (#2194). It is
+    idempotent, so the nested call on the ``build`` path is a no-op.
+
+    Non-numeric non-str ids (``None``, lists, dicts) are left alone for
+    ``validate_extraction`` to report: ``str(None) == "None"`` would fabricate a
+    node id that no edge references.
+    """
+    for node in extraction.get("nodes") or ():
+        if isinstance(node, dict) and "id" in node:
+            node["id"] = _coerce_id(node["id"])
+    for edge in extraction.get("edges") or ():
+        if not isinstance(edge, dict):
+            continue
+        for key in ("source", "target", "from", "to"):
+            if key in edge:
+                edge[key] = _coerce_id(edge[key])
+    for he in extraction.get("hyperedges") or ():
+        if not isinstance(he, dict):
+            continue
+        members = he.get("nodes")
+        if isinstance(members, list):
+            he["nodes"] = [_coerce_id(ref) for ref in members]
+
+
 def _norm_source_file(p: str | None, root: str | None = None) -> str | None:
     """Normalize path separators and relativize absolute paths.
 
@@ -549,6 +605,10 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
     if "edges" not in extraction and "links" in extraction:
         extraction = dict(extraction, edges=extraction["links"])
 
+    # Numeric ids from a loose backend become str before anything keys on them
+    # (#2326) — after the links remap so aliased edges are covered too.
+    _coerce_non_string_ids(extraction)
+
     # Canonicalize legacy node/edge schema before validation.
     for node in extraction.get("nodes", []):
         if not isinstance(node, dict):
@@ -1048,6 +1108,10 @@ def build(
         combined["input_tokens"] += ext.get("input_tokens", 0)
         combined["output_tokens"] += ext.get("output_tokens", 0)
     if dedup and combined["nodes"]:
+        # Numeric ids must be str before dedup, which keys on them and would
+        # raise TypeError in _pick_winner's regex search (#2326). build_from_json
+        # coerces too, but that runs after dedup — too late for this path.
+        _coerce_non_string_ids(combined)
         # Fold legacy node field aliases before dedup (#2194): dedup runs BEFORE
         # build_from_json and keys on `label`, so a `name`/`path` alias node
         # would be invisible to it and only label-dedup one build later, after

tests/test_non_string_node_ids.py

@@ -0,0 +1,136 @@
+"""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")