mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 08:46:43 +00:00
fix(dedup): preserve same-source node attributes
This commit is contained in:
+5
-4
@@ -957,10 +957,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}
|
||||
|
||||
+36
-8
@@ -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():
|
||||
|
||||
+78
-3
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user