diff --git a/graphify/build.py b/graphify/build.py index bfe8fe9..7e7ed61 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -124,6 +124,39 @@ def _normalize_hyperedge_members(he: object) -> None: he.pop(alias, None) +def _fold_node_aliases(node: dict) -> None: + """Fold legacy node field aliases onto canonical keys, in place (#2194). + + ``name`` -> ``label`` and ``path`` -> ``source_file``. Uses an empty-check + (not mere key presence) so a node carrying ``label: ""``/``None`` next to a + real ``name`` is healed too. When the canonical field already holds a value + it wins and the alias key is left untouched. Without this fold an alias-only + node enters the graph with no label/source_file: it fails validation, gets + ``norm_label == ""`` (invisible to query/explain), and is excluded from every + label-keyed merge/dedup — a permanent ghost that ``graphify update`` + re-feeds through build_from_json forever. + """ + if not node.get("label") and isinstance(node.get("name"), str) and node["name"]: + node["label"] = node.pop("name") + if not node.get("source_file") and isinstance(node.get("path"), str) and node["path"]: + node["source_file"] = node.pop("path") + + +def _fold_edge_aliases(edge: dict) -> None: + """Fold legacy edge field aliases onto canonical keys, in place (#2194). + + ``type`` -> ``relation``. A ``confidence_score`` float with no ``confidence`` + enum backfills ``confidence: "INFERRED"`` — never EXTRACTED (alias recovery + is not provenance) and never a threshold mapping of the float. The + ``confidence_score`` key itself is NOT popped: it is a legitimate companion + field that the edge loop sanitizes and to_json round-trips. + """ + if not edge.get("relation") and isinstance(edge.get("type"), str) and edge["type"]: + edge["relation"] = edge.pop("type") + if not edge.get("confidence") and edge.get("confidence_score") is not None: + edge["confidence"] = "INFERRED" + + def _norm_source_file(p: str | None, root: str | None = None) -> str | None: """Normalize path separators and relativize absolute paths. @@ -395,7 +428,23 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict: if norm_nid == new_stem or norm_nid.startswith(new_stem + "_"): continue new_id: str | None = None - for old_stem in _old_file_stems(rel): + old_forms = _old_file_stems(rel) + # #2197: on Windows, detect() can emit an ABSOLUTE source_file, and a + # semantic fragment's id derived from that absolute path (e.g. + # d_projects_myrepo_docs_dataflow) matches neither the canonical + # relative stem nor the legacy short forms above — so while source_file + # itself is healed by _norm_source_file, the id would ghost against the + # existing graph's docs_dataflow. When the raw path was absolute and + # relativized under root, treat the raw-absolute stem as one more + # old-stem form — the semantic-side twin of extract.py's absolute-form + # id registration. It is the longest form, so it goes first (greedy + # prefix stripping, same ordering rule as _old_file_stems). + sf_raw = str(sf).replace("\\", "/") + if sf_raw != sf_norm and os.path.isabs(sf_raw): + abs_stem = make_id(_file_stem(Path(sf_raw))) + if abs_stem and abs_stem != new_stem and abs_stem not in old_forms: + old_forms.insert(0, abs_stem) + for old_stem in old_forms: if old_stem == new_stem: continue # already canonical for this form if norm_nid == old_stem: @@ -518,6 +567,11 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat file=sys.stderr, ) node["source_file"] = node.pop("source") + # Fold the remaining legacy node aliases (`name`->`label`, + # `path`->`source_file`, #2194) before validation and before the + # semantic-rekey / ghost-merge passes below, all of which key on + # label/source_file and would otherwise skip the node entirely. + _fold_node_aliases(node) # Default missing/None file_type to "concept" so legacy graph.json # entries (and stub nodes preserved by `_rebuild_code` from older # graphify versions that didn't always populate file_type) don't @@ -536,11 +590,33 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat for he in extraction.get("hyperedges", []) or []: _normalize_hyperedge_members(he) + # Fold legacy edge field aliases (`type`->`relation`, + # `confidence_score`->`confidence`, #2194) BEFORE validation. The existing + # from/to endpoint fold lives in the edge loop further down, which runs + # after validate_extraction — too late for fields the validator requires. + for edge in extraction.get("edges", []): + if isinstance(edge, dict): + _fold_edge_aliases(edge) + errors = validate_extraction(extraction) # Dangling edges (stdlib/external imports) are expected - only warn about real schema errors. real_errors = [e for e in errors if "does not match any node id" not in e] if real_errors: - print(f"[graphify] Extraction warning ({len(real_errors)} issues): {real_errors[0]}", file=sys.stderr) + # Break the warning down by cause (#2194): a mixed batch used to surface + # only real_errors[0], hiding every other failure mode. Group on the + # "missing required field 'X'" suffix and report per-cause counts plus + # one example each, so the operator sees the full shape of the damage. + by_cause: dict[str, list[str]] = {} + for err in real_errors: + m = re.search(r"missing required field '[^']*'", err) + by_cause.setdefault(m.group(0) if m else "other schema issue", []).append(err) + breakdown = "; ".join( + f"{len(errs)}x {cause} (e.g. {errs[0]})" for cause, errs in by_cause.items() + ) + print( + f"[graphify] Extraction warning ({len(real_errors)} issues): {breakdown}", + file=sys.stderr, + ) # Deterministic semantic re-key (#1504/#1509): the node-ID stem is now the # full repo-relative path (docs/v1/api/README.md -> docs_v1_api_readme), but # the semantic cache is UNVERSIONED, so a cached/LLM fragment can still carry @@ -972,6 +1048,13 @@ def build( combined["input_tokens"] += ext.get("input_tokens", 0) combined["output_tokens"] += ext.get("output_tokens", 0) if dedup and combined["nodes"]: + # 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 + # build_from_json's own fold has healed the persisted graph.json. + for n in combined["nodes"]: + if isinstance(n, dict): + _fold_node_aliases(n) combined["nodes"], combined["edges"] = deduplicate_entities( combined["nodes"], combined["edges"], communities={}, dedup_llm_backend=dedup_llm_backend, diff --git a/tests/test_build.py b/tests/test_build.py index 3d8582c..0851ba7 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -135,6 +135,145 @@ def test_legacy_edge_from_to_canonicalized(): assert G.number_of_edges() == 1 +def test_legacy_node_name_path_aliases_folded(): + """#2194: nodes carrying `name`/`path` instead of `label`/`source_file` must + be canonicalized before validation, not enter the graph as label-less + ghosts. After build the canonicalized dict also passes validation.""" + from graphify.validate import validate_extraction + ext = {"nodes": [{"id": "n1", "name": "Foo", "path": "a/b.md", "file_type": "concept"}], + "edges": [], "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext) + attrs = G.nodes["n1"] + assert attrs["label"] == "Foo" + assert attrs["source_file"] == "a/b.md" + assert "name" not in attrs + assert "path" not in attrs + # build_from_json canonicalizes in place; the extraction dict must now be + # schema-valid (no missing-field errors for the alias node). + assert not [e for e in validate_extraction(ext) if "missing required field" in e] + + +def test_legacy_edge_type_confidence_score_aliases_folded(): + """#2194: edges carrying `type`/`confidence_score` instead of + `relation`/`confidence` fold to canonical fields. Recovery confidence is + INFERRED (never EXTRACTED — alias recovery is not provenance) and the + companion confidence_score float is retained, not popped.""" + ext = {"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", "type": "references", + "confidence_score": 0.9, "source_file": "a.py"}], + "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext) + data = edge_data(G, "n1", "n2") + assert data["relation"] == "references" + assert data["confidence"] == "INFERRED" + assert data["confidence_score"] == 0.9 + assert "type" not in data + + +def test_node_alias_canonical_field_wins(): + """#2194: when both the canonical field and its alias are present, the + canonical value wins and the alias key is left untouched.""" + ext = {"nodes": [{"id": "n1", "label": "Real", "name": "Alias", + "file_type": "code", "source_file": "a.py"}], + "edges": [], "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext) + assert G.nodes["n1"]["label"] == "Real" + assert G.nodes["n1"]["name"] == "Alias" # preserved, not consumed + + +def test_alias_node_ghost_merges_into_ast_twin(): + """#2194: an alias-only semantic node (name/path) must participate in the + AST/LLM ghost merge once folded — same label and file as an AST node with a + different id collapses into the AST node instead of surviving as a ghost.""" + ext = {"nodes": [ + {"id": "src_foo_helper", "label": "helper", "file_type": "code", + "source_file": "src/foo.py", "_origin": "ast", "source_location": "L10"}, + {"id": "helper_ghost", "name": "helper", "path": "src/foo.py", + "file_type": "code"}, + ], "edges": [], "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext) + assert "src_foo_helper" in G.nodes + assert "helper_ghost" not in G.nodes + assert G.number_of_nodes() == 1 + + +def test_alias_node_gets_nonempty_norm_label(tmp_path): + """#2194: a recovered alias node must serialize with a non-empty norm_label + so query/explain can find it.""" + from graphify.export import to_json + ext = {"nodes": [{"id": "n1", "name": "Foo", "path": "a/b.md", "file_type": "concept"}], + "edges": [], "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext) + out = tmp_path / "graph.json" + assert to_json(G, {}, str(out)) + data = json.loads(out.read_text()) + node = next(n for n in data["nodes"] if n["id"] == "n1") + assert node["norm_label"] == "foo" + + +def test_extraction_warning_breakdown_by_cause(capsys): + """#2194: a mixed batch of schema errors must report per-cause counts, not + just the first error.""" + ext = {"nodes": [ + {"id": "n1", "label": "A", "file_type": "code", "source_file": "a.py"}, + {"id": "n2", "label": "B", "file_type": "code", "source_file": "b.py"}, + # two nodes missing label (and carrying no name alias) + {"id": "x1", "file_type": "code", "source_file": "x.py"}, + {"id": "x2", "file_type": "code", "source_file": "x.py"}, + ], "edges": [ + # three edges missing relation (and carrying no type alias) + {"source": "n1", "target": "n2", "confidence": "EXTRACTED", "source_file": "a.py"}, + {"source": "n2", "target": "n1", "confidence": "EXTRACTED", "source_file": "a.py"}, + {"source": "n1", "target": "x1", "confidence": "EXTRACTED", "source_file": "a.py"}, + ], "input_tokens": 0, "output_tokens": 0} + build_from_json(ext) + err = capsys.readouterr().err + assert "2x missing required field 'label'" in err + assert "3x missing required field 'relation'" in err + + +def test_absolute_derived_semantic_ids_rekeyed(tmp_path): + """#2197: a semantic fragment whose ids were derived from an ABSOLUTE + source_file (Windows detect() emits them) must re-key to the canonical + repo-relative stem instead of ghosting against the existing graph.""" + from graphify.ids import make_id + (tmp_path / "docs").mkdir() + abs_sf = str(tmp_path / "docs" / "DATAFLOW.md") + abs_stem = make_id(str(tmp_path / "docs" / "DATAFLOW")) + ext = {"nodes": [ + {"id": abs_stem, "label": "DATAFLOW.md", "file_type": "document", + "source_file": abs_sf}, + {"id": f"{abs_stem}_pipeline", "label": "Pipeline", "file_type": "concept", + "source_file": abs_sf}, + ], "edges": [ + {"source": abs_stem, "target": f"{abs_stem}_pipeline", "relation": "describes", + "confidence": "INFERRED", "source_file": abs_sf, "weight": 1.0}, + ], "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext, root=tmp_path) + assert "docs_dataflow" in G.nodes + assert "docs_dataflow_pipeline" in G.nodes + assert abs_stem not in G.nodes + assert G.nodes["docs_dataflow"]["source_file"] == "docs/DATAFLOW.md" + assert G.has_edge("docs_dataflow", "docs_dataflow_pipeline") + + +def test_absolute_derived_semantic_ids_rekeyed_backslash(tmp_path): + """#2197 (separator variant): the same absolute-derived-id fragment with + backslash separators in source_file re-keys identically.""" + from graphify.ids import make_id + (tmp_path / "docs").mkdir() + abs_sf = str(tmp_path / "docs" / "DATAFLOW.md").replace("/", "\\") + abs_stem = make_id(str(tmp_path / "docs" / "DATAFLOW")) + ext = {"nodes": [ + {"id": f"{abs_stem}_pipeline", "label": "Pipeline", "file_type": "concept", + "source_file": abs_sf}, + ], "edges": [], "input_tokens": 0, "output_tokens": 0} + G = build_from_json(ext, root=tmp_path) + assert "docs_dataflow_pipeline" in G.nodes + assert G.nodes["docs_dataflow_pipeline"]["source_file"] == "docs/DATAFLOW.md" + + def test_source_file_backslash_normalized(): """Windows backslash paths and POSIX paths for the same file must produce one node.""" extraction = { diff --git a/tests/test_validate.py b/tests/test_validate.py index ea865fa..013fa87 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -87,6 +87,27 @@ def test_assert_valid_passes_silently(): assert_valid(VALID) # should not raise +def test_legacy_aliases_valid_after_build_canonicalization(): + # #2194: build_from_json folds legacy aliases (name->label, + # path->source_file, type->relation, confidence_score->confidence) in + # place BEFORE validation, so an alias-only extraction that fails + # validation raw is fully schema-valid after canonicalization. + from graphify.build import build_from_json + data = { + "nodes": [ + {"id": "n1", "name": "Foo", "path": "a/b.md", "file_type": "concept"}, + {"id": "n2", "label": "Bar", "file_type": "code", "source_file": "bar.py"}, + ], + "edges": [ + {"source": "n1", "target": "n2", "type": "references", + "confidence_score": 0.9, "source_file": "a/b.md"}, + ], + } + assert any("missing required field" in e for e in validate_extraction(data)) + build_from_json(data) + assert validate_extraction(data) == [] + + 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