diff --git a/graphify/build.py b/graphify/build.py index 9acf342bb..ec6d70f3a 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -952,10 +952,11 @@ def build( ambiguous pairs in the 75–92 Jaro-Winkler score zone. root: if given, absolute source_file paths are made relative to root (#932). - Extractions are merged in order. For nodes with the same ID, the last - extraction's attributes win (NetworkX add_node overwrites). Pass AST - results before semantic results so semantic labels take precedence, or - reverse the order if you prefer AST source_location precision to win. + With dedup disabled, extractions are merged in order and the last node's + attributes win (NetworkX add_node overwrites). With dedup enabled, nodes + sharing an ID use a deterministic survivor and retain missing attributes + from duplicate records of the same source entity. Genuine cross-file ID + collisions remain isolated and are reported. """ from graphify.dedup import deduplicate_entities combined: dict = {"nodes": [], "edges": [], "hyperedges": [], "input_tokens": 0, "output_tokens": 0} diff --git a/graphify/dedup.py b/graphify/dedup.py index 79a1bfb8a..f4de6958f 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -244,14 +244,35 @@ def _collision_rank(node: dict) -> tuple: ) +def _same_source_entity(survivor: dict, duplicate: dict) -> bool: + """True when exact-ID records came from the same source file. + + Exact IDs can also collide across files through references or slugged-path + ambiguity (#1504). Keep those records isolated rather than importing + attributes whose provenance belongs to another file. + """ + keep_file = survivor.get("source_file") or "" + lose_file = duplicate.get("source_file") or "" + return keep_file == lose_file + + +def _merge_missing_attributes(survivor: dict, duplicate: dict) -> dict: + """Keep survivor values while retaining non-conflicting duplicate data.""" + merged = dict(survivor) + for key, value in duplicate.items(): + merged.setdefault(key, value) + return merged + + def _report_id_collision(nid: str, survivor: dict, losers: list[dict]) -> None: """Report an ID collision in proportion to what dropping the loser actually costs. - Cross-reference to a defining node: same entity, edges are keyed by ID and rewire - to the survivor — nothing is lost, so say nothing. Same file, different labels: the - extractor emitted two labels for one entity and one is discarded — note it. Two - files that both encode this ID: they are distinct entities and one is genuinely - lost — warn, and point at the extraction split that keeps them apart (#1504). + Cross-reference to a defining node: the structural entity and its edges survive; + foreign-file attributes stay isolated, so no collision warning is needed. Same + file, different labels: the extractor emitted two labels for one entity and one is + discarded — note it. Two files that both encode this ID: they are distinct entities + and one is genuinely lost — warn, and point at the extraction split that keeps them + apart (#1504). """ keep_file = survivor.get("source_file") or "" keep_label = survivor.get("label") or "" @@ -316,8 +337,9 @@ def deduplicate_entities( # Pre-deduplicate: one node per ID. The survivor is the node that *defines* the # ID (its source_file is the file the ID encodes), not merely the first seen — # otherwise chunk order decides whether an entity keeps its own attributes or a - # passing cross-reference's. Warnings are then emitted for what is actually lost - # (#1504); a same-entity merge costs nothing and stays quiet. + # passing cross-reference's. Missing attributes from same-source records are + # retained so AST structure and semantic enrichment can coexist (#2091). + # Genuine cross-file ID collisions stay isolated and are reported below (#1504). seen_ids: dict[str, dict] = {} dropped: dict[str, list[dict]] = defaultdict(list) for node in nodes: @@ -331,9 +353,15 @@ def deduplicate_entities( # Smallest-ranked node wins; the min over a total order is independent # of the order nodes arrive in, so the survivor no longer depends on # chunk ordering (#1851). - seen_ids[nid] = node + seen_ids[nid] = ( + _merge_missing_attributes(node, incumbent) + if _same_source_entity(node, incumbent) + else node + ) dropped[nid].append(incumbent) else: + if _same_source_entity(incumbent, node): + seen_ids[nid] = _merge_missing_attributes(incumbent, node) dropped[nid].append(node) for nid, losers in dropped.items(): diff --git a/tests/test_dedup.py b/tests/test_dedup.py index 57fe6c192..7195226a1 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -137,6 +137,30 @@ def test_build_calls_dedup(): assert G.number_of_nodes() == 1 +def test_build_dedup_preserves_semantic_attributes(): + """The default build path must not discard semantic enrichment (#2091).""" + from graphify.build import build + ast = { + "nodes": [{"id": "src_auth_login", "label": "login", "file_type": "code", + "source_file": "src/auth.py", "_origin": "ast", + "source_location": "L42"}], + "edges": [], + } + semantic = { + "nodes": [{"id": "src_auth_login", "label": "User login handler", + "file_type": "code", "source_file": "src/auth.py", + "summary": "Authenticates a user.", "confidence_score": 0.9}], + "edges": [], + } + + node = dict(build([ast, semantic], directed=True, dedup=True).nodes["src_auth_login"]) + + assert node["label"] == "login" + assert node["source_location"] == "L42" + assert node["summary"] == "Authenticates a user." + assert node["confidence_score"] == 0.9 + + # --- #878: fuzzy dedup false merges on short/variant labels --- def test_dedup_does_not_merge_numeric_variants(tmp_path): @@ -469,14 +493,15 @@ def test_defining_file_wins_over_referencing_file(nodes, capsys): def test_reference_collision_is_silent(capsys): - """A cross-reference collapsing into the entity it references loses nothing — - edges are keyed by ID and rewire to the survivor — so it must not be reported.""" + """A cross-reference rewires silently without importing foreign-file metadata.""" edges = _make_edges("agents_make_batch_fixtures_make_batch_fixtures", "other") + referencing = dict(_REFERENCING, summary="Reference-local description.") result_nodes, result_edges = deduplicate_entities( - [_DEFINING, _REFERENCING], edges, communities={}) + [_DEFINING, referencing], edges, communities={}) assert len(result_nodes) == 1 assert len(result_edges) == 1 + assert "summary" not in result_nodes[0] captured = capsys.readouterr() assert "WARNING" not in captured.err assert "note:" not in captured.err @@ -513,6 +538,56 @@ def test_same_file_relabel_is_noted(capsys): assert "WARNING" not in captured.err +@pytest.mark.parametrize("nodes", [ + [ + {"id": "src_auth_login", "label": "login", "file_type": "code", + "source_file": "src/auth.py", "_origin": "ast", "source_location": "L42"}, + {"id": "src_auth_login", "label": "User login handler", "file_type": "code", + "source_file": "src/auth.py", "summary": "Authenticates a user.", + "confidence_score": 0.9}, + ], + [ + {"id": "src_auth_login", "label": "User login handler", "file_type": "code", + "source_file": "src/auth.py", "summary": "Authenticates a user.", + "confidence_score": 0.9}, + {"id": "src_auth_login", "label": "login", "file_type": "code", + "source_file": "src/auth.py", "_origin": "ast", "source_location": "L42"}, + ], +], ids=["ast-first", "semantic-first"]) +def test_same_id_same_entity_retains_complementary_attributes(nodes): + """Exact-ID dedup combines AST precision with semantic enrichment (#2091).""" + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + + assert len(result_nodes) == 1 + assert result_nodes[0] == { + "id": "src_auth_login", + "label": "login", + "file_type": "code", + "source_file": "src/auth.py", + "_origin": "ast", + "source_location": "L42", + "summary": "Authenticates a user.", + "confidence_score": 0.9, + } + + +def test_cross_file_id_collision_does_not_mix_attributes(capsys): + """Two files that both mint one ID remain isolated despite exact-ID dedup.""" + nodes = [ + {"id": "pkg_service_run", "label": "run", "file_type": "code", + "source_file": "pkg/service.py", "source_location": "L10"}, + {"id": "pkg_service_run", "label": "run helper", "file_type": "code", + "source_file": "pkg_service.py", "summary": "Different function."}, + ] + + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + + assert len(result_nodes) == 1 + assert result_nodes[0]["source_file"] == "pkg/service.py" + assert "summary" not in result_nodes[0] + assert "WARNING" in capsys.readouterr().err + + def test_collision_survivor_is_order_independent(): """#1851: definer + same-file relabel + cross-file reference. Across every insertion order the SAME node (source_file AND label) must survive — the