mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-19 20:15:42 +00:00
fix(build): route carried hyperedges through the dedup survivor remap (#3102)
A hyperedge carried from a prior graph.json kept its original member ids, so when dedup merged one of those members away the hyperedge dangled. Feed carried hyperedges into build() on the base chunk so deduplicate_entities remaps their members onto the survivor at the same time as edge endpoints; a member that resolves to nothing is dropped (the whole hyperedge if none survive), and a hyperedge re-emitted by a new chunk is not duplicated. Materialise new_chunks so the added second iteration is safe.
This commit is contained in:
committed by
safishamsi
parent
bad832f225
commit
3378ae8fef
+31
-13
@@ -1649,6 +1649,9 @@ def build_merge(
|
||||
graph to inherit from. An explicit True/False always overrides the on-disk
|
||||
flag.
|
||||
"""
|
||||
# Iterated more than once below (source sets, the hyperedge carry, the
|
||||
# build itself), so a one-shot iterator must be materialised first.
|
||||
new_chunks = list(new_chunks)
|
||||
graph_path = Path(graph_path if graph_path is not None else _default_graph_json())
|
||||
_loaded = _load_existing_graph(graph_path)
|
||||
if _loaded is not None:
|
||||
@@ -1726,11 +1729,6 @@ def build_merge(
|
||||
file=sys.stderr,
|
||||
)
|
||||
|
||||
base = [{"nodes": existing_nodes, "edges": existing_edges}] if had_graph else []
|
||||
|
||||
all_chunks = base + list(new_chunks)
|
||||
G = build(all_chunks, directed=directed, dedup=dedup, dedup_llm_backend=dedup_llm_backend, root=root)
|
||||
|
||||
# Prune set for deleted source files — both the raw form (matches nodes that
|
||||
# kept absolute source_file) and the normalised relative form (matches nodes
|
||||
# relativised by _norm_source_file at build time). .resolve() (via _eff_root)
|
||||
@@ -1797,12 +1795,25 @@ def build_merge(
|
||||
# deleted (#1574). build() only sees the new chunks' hyperedges, so without
|
||||
# this every --update collapses the graph's hyperedge set down to just the
|
||||
# changed files'. Re-extracted files' prior hyperedges are dropped (their new
|
||||
# version is already in G — replace-per-source, like nodes/edges); deleted
|
||||
# files' are dropped via prune_set. id-dedup (attach_hyperedges) so a carried
|
||||
# hyperedge never duplicates one the new chunks re-emitted. Mirrors watch.py,
|
||||
# which already preserves existing hyperedges across a rebuild.
|
||||
# version is already in the new chunks — replace-per-source, like
|
||||
# nodes/edges); deleted files' are dropped via prune_set; id-dedup so a
|
||||
# carried hyperedge never duplicates one the new chunks re-emitted. Mirrors
|
||||
# watch.py, which already preserves existing hyperedges across a rebuild.
|
||||
#
|
||||
# The carried set rides INTO build() on the base chunk rather than being
|
||||
# attached to G afterwards (#3102): entity dedup rewires every edge endpoint
|
||||
# and every hyperedge member it sees onto the survivor (#2805), but a
|
||||
# hyperedge attached after the fact kept naming the merged-away node — a
|
||||
# dangling member with no backing node in the written graph.
|
||||
carried_hyperedges: list[dict] = []
|
||||
if existing_hyperedges:
|
||||
carried = []
|
||||
carried = carried_hyperedges
|
||||
_new_hyperedge_ids = {
|
||||
he.get("id")
|
||||
for chunk in new_chunks
|
||||
for he in (chunk.get("hyperedges") or [])
|
||||
if isinstance(he, dict) and he.get("id")
|
||||
}
|
||||
for he in existing_hyperedges:
|
||||
if not isinstance(he, dict):
|
||||
continue
|
||||
@@ -1815,10 +1826,17 @@ def build_merge(
|
||||
continue # semantically re-extracted — replaced by the new chunk's version
|
||||
if _prune_match(sf):
|
||||
continue # deleted — pruned
|
||||
if he.get("id") and he.get("id") in _new_hyperedge_ids:
|
||||
continue # the new chunks re-emitted it — theirs wins
|
||||
carried.append(he)
|
||||
if carried:
|
||||
from graphify.export import attach_hyperedges
|
||||
attach_hyperedges(G, carried)
|
||||
|
||||
base = (
|
||||
[{"nodes": existing_nodes, "edges": existing_edges, "hyperedges": carried_hyperedges}]
|
||||
if had_graph else []
|
||||
)
|
||||
|
||||
all_chunks = base + list(new_chunks)
|
||||
G = build(all_chunks, directed=directed, dedup=dedup, dedup_llm_backend=dedup_llm_backend, root=root)
|
||||
|
||||
# Prune nodes and edges from deleted source files
|
||||
if prune_sources:
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
"""Carried-forward hyperedges must follow the dedup survivor remap (#3102).
|
||||
|
||||
build_merge() carries hyperedges from unchanged files across an incremental
|
||||
rebuild (#1574). They used to be attached to G AFTER build() and entity
|
||||
dedup had finished, so while every edge endpoint was rewired onto the dedup
|
||||
survivor (#2805), a carried hyperedge kept naming the merged-away node — a
|
||||
dangling member in graph.json with no backing node.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
from graphify.build import build_from_json, build_merge
|
||||
from graphify.export import to_json
|
||||
|
||||
# `alpha_a` and `alpha_concept_long_variant_id` label-dedup into one node.
|
||||
NODES = [
|
||||
{"id": "alpha_a", "label": "Alpha Concept", "file_type": "concept", "source_file": "notes/a.md"},
|
||||
{"id": "alpha_concept_long_variant_id", "label": "alpha_concept", "file_type": "concept",
|
||||
"source_file": "notes/b.md"},
|
||||
{"id": "beta_node", "label": "Beta", "file_type": "concept", "source_file": "notes/group.md"},
|
||||
{"id": "gamma_node", "label": "Gamma", "file_type": "concept", "source_file": "notes/group.md"},
|
||||
]
|
||||
EDGES = [{"source": "alpha_concept_long_variant_id", "target": "beta_node", "relation": "references",
|
||||
"confidence": "EXTRACTED", "confidence_score": 1.0, "source_file": "notes/b.md"}]
|
||||
HYPEREDGE = {"id": "the_group", "label": "The Group",
|
||||
"nodes": ["alpha_concept_long_variant_id", "beta_node", "gamma_node"],
|
||||
"relation": "participate_in", "confidence": "EXTRACTED", "confidence_score": 1.0,
|
||||
"source_file": "notes/group.md"}
|
||||
UNRELATED_CHUNK = {
|
||||
"nodes": [{"id": "delta_node", "label": "Delta", "file_type": "concept", "source_file": "notes/d.md"}],
|
||||
"edges": [], "hyperedges": [],
|
||||
}
|
||||
|
||||
|
||||
def _baseline(tmp_path: Path) -> Path:
|
||||
"""A graph written WITHOUT dedup, so the pair is still two nodes on disk
|
||||
and the hyperedge names the variant — the shape an older build leaves."""
|
||||
G = build_from_json({"nodes": NODES, "edges": EDGES, "hyperedges": [HYPEREDGE]})
|
||||
p = tmp_path / "graph.json"
|
||||
to_json(G, {0: list(G.nodes)}, str(p))
|
||||
return p
|
||||
|
||||
|
||||
def _hyperedges(G):
|
||||
return {he["id"]: he for he in G.graph.get("hyperedges", [])}
|
||||
|
||||
|
||||
def test_a_carried_hyperedge_is_remapped_onto_the_dedup_survivor(tmp_path):
|
||||
G = build_merge([UNRELATED_CHUNK], _baseline(tmp_path))
|
||||
survivors = set(G.nodes)
|
||||
assert "alpha_concept_long_variant_id" not in survivors # merged away
|
||||
he = _hyperedges(G)["the_group"]
|
||||
assert set(he["nodes"]) <= survivors, f"dangling members: {set(he['nodes']) - survivors}"
|
||||
assert "alpha_a" in he["nodes"] # onto the survivor, not just dropped
|
||||
assert {"beta_node", "gamma_node"} <= set(he["nodes"])
|
||||
|
||||
|
||||
def test_the_written_graph_has_no_dangling_hyperedge_member(tmp_path):
|
||||
G = build_merge([UNRELATED_CHUNK], _baseline(tmp_path))
|
||||
out = tmp_path / "merged.json"
|
||||
to_json(G, {0: list(G.nodes)}, str(out), force=True)
|
||||
data = json.loads(out.read_text(encoding="utf-8"))
|
||||
ids = {n["id"] for n in data["nodes"]}
|
||||
for he in data.get("hyperedges", []):
|
||||
assert set(he["nodes"]) <= ids, f"{he['id']} names a node that is not in the graph"
|
||||
|
||||
|
||||
def test_edges_and_hyperedges_agree_on_the_survivor(tmp_path):
|
||||
"""The edge endpoint and the hyperedge member came from the same
|
||||
merged-away node; both must now name the same survivor."""
|
||||
G = build_merge([UNRELATED_CHUNK], _baseline(tmp_path))
|
||||
edge_ends = {u for u, v in G.edges} | {v for u, v in G.edges}
|
||||
assert "alpha_a" in edge_ends
|
||||
assert "alpha_a" in _hyperedges(G)["the_group"]["nodes"]
|
||||
|
||||
|
||||
def test_a_hyperedge_re_emitted_by_the_new_chunk_is_not_duplicated(tmp_path):
|
||||
fresh = {"nodes": [{"id": "beta_node", "label": "Beta", "file_type": "concept",
|
||||
"source_file": "notes/group.md"},
|
||||
{"id": "gamma_node", "label": "Gamma", "file_type": "concept",
|
||||
"source_file": "notes/group.md"}],
|
||||
"edges": [],
|
||||
"hyperedges": [{**HYPEREDGE, "nodes": ["beta_node", "gamma_node"], "label": "The Group v2"}]}
|
||||
G = build_merge([fresh], _baseline(tmp_path))
|
||||
hes = [he for he in G.graph.get("hyperedges", []) if he["id"] == "the_group"]
|
||||
assert len(hes) == 1
|
||||
assert hes[0]["label"] == "The Group v2" # the re-extracted version wins
|
||||
|
||||
|
||||
def test_a_pruned_sources_hyperedge_is_still_dropped(tmp_path):
|
||||
G = build_merge([UNRELATED_CHUNK], _baseline(tmp_path), prune_sources=["notes/group.md"])
|
||||
assert "the_group" not in _hyperedges(G)
|
||||
|
||||
|
||||
def test_an_unchanged_hyperedge_with_no_dedup_involved_is_carried_verbatim(tmp_path):
|
||||
nodes = [n for n in NODES if n["id"] != "alpha_a"] # nothing to dedup now
|
||||
G0 = build_from_json({"nodes": nodes, "edges": EDGES, "hyperedges": [HYPEREDGE]})
|
||||
p = tmp_path / "g.json"
|
||||
to_json(G0, {0: list(G0.nodes)}, str(p))
|
||||
G = build_merge([UNRELATED_CHUNK], p)
|
||||
assert set(_hyperedges(G)["the_group"]["nodes"]) == set(HYPEREDGE["nodes"])
|
||||
Reference in New Issue
Block a user