mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-21 04:56:18 +00:00
fix(hyperedge): accept members/node_ids alias keys for the member list (#1561)
A hyperedge's member list is canonically keyed `nodes`, but producers (LLM/subagent drift, externally-supplied graph.json) sometimes emit `members` or `node_ids` — graphify only read `nodes`, so those hyperedges silently lost their members, and semantic_cleanup's prune dropped them entirely. Normalize the member key to `nodes` at one ingest chokepoint in build_from_json (and in semantic_cleanup, which runs pre-build), deduping and warning, so every downstream consumer sees the canonical key. Mirrors the existing from/to edge-endpoint aliasing. Reported by @askalot-io. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c865a3c9b0
commit
bd885cc97d
@@ -53,6 +53,56 @@ _FILE_TYPE_SYNONYMS = {
|
||||
}
|
||||
|
||||
|
||||
# Hyperedge member lists are canonically keyed `nodes` (see graphify/llm.py
|
||||
# extraction spec), but LLM/subagent drift and externally-supplied graph.json
|
||||
# sometimes emit `members` or `node_ids`. _normalize_hyperedge_members folds
|
||||
# those aliases into `nodes` at ingest so every downstream consumer reads one
|
||||
# canonical key — mirroring the `from`/`to` edge-endpoint tolerance below.
|
||||
_HE_MEMBER_ALIASES = ("members", "node_ids")
|
||||
|
||||
|
||||
def _normalize_hyperedge_members(he: object) -> None:
|
||||
"""Canonicalize a hyperedge's member list onto the `nodes` key, in place.
|
||||
|
||||
If `nodes` is already a list it wins (canonical), and only stray alias keys
|
||||
are dropped. Otherwise the first alias (`members`, then `node_ids`) that is a
|
||||
list is moved to `nodes`, deduped preserving order, with a single stderr
|
||||
WARNING naming the hyperedge id and alias used. Leftover alias keys are
|
||||
always removed so downstream code never re-reads them.
|
||||
"""
|
||||
if not isinstance(he, dict):
|
||||
return
|
||||
if not isinstance(he.get("nodes"), list):
|
||||
for alias in _HE_MEMBER_ALIASES:
|
||||
val = he.get(alias)
|
||||
if isinstance(val, list):
|
||||
seen: set = set()
|
||||
deduped: list = []
|
||||
for ref in val:
|
||||
try:
|
||||
is_dupe = ref in seen
|
||||
except TypeError:
|
||||
is_dupe = False # unhashable ref: keep it, validator flags it
|
||||
if is_dupe:
|
||||
continue
|
||||
try:
|
||||
seen.add(ref)
|
||||
except TypeError:
|
||||
pass
|
||||
deduped.append(ref)
|
||||
he["nodes"] = deduped
|
||||
print(
|
||||
f"[graphify] WARNING: hyperedge "
|
||||
f"'{he.get('id', '?')}' uses field '{alias}' instead of "
|
||||
f"'nodes'; normalizing.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
break
|
||||
# Drop any leftover alias keys regardless of which branch ran above.
|
||||
for alias in _HE_MEMBER_ALIASES:
|
||||
he.pop(alias, None)
|
||||
|
||||
|
||||
def _norm_source_file(p: str | None, root: str | None = None) -> str | None:
|
||||
"""Normalize path separators and relativize absolute paths.
|
||||
|
||||
@@ -279,6 +329,14 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
if ft and ft not in {"code", "document", "paper", "image", "rationale", "concept"}:
|
||||
node["file_type"] = _FILE_TYPE_SYNONYMS.get(ft, "concept")
|
||||
|
||||
# Canonicalize hyperedge member lists (#1561): producers sometimes key the
|
||||
# member list `members`/`node_ids` instead of `nodes`. Fold aliases onto
|
||||
# `nodes` here — BEFORE validation and the semantic-rekey loop below — so
|
||||
# every downstream consumer (rekey, source_file relativize, to_json) reads
|
||||
# one canonical key, the same way edge endpoints alias from/to at build.
|
||||
for he in extraction.get("hyperedges", []) or []:
|
||||
_normalize_hyperedge_members(he)
|
||||
|
||||
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]
|
||||
|
||||
+1
-1
@@ -679,7 +679,7 @@ def to_html(
|
||||
if raw_hyperedges:
|
||||
remapped = []
|
||||
for he in raw_hyperedges:
|
||||
he_members = he.get("nodes") or he.get("members") or []
|
||||
he_members = he.get("nodes", [])
|
||||
comm_ids, seen = [], set()
|
||||
for nid in he_members:
|
||||
c = node_to_community.get(nid)
|
||||
|
||||
@@ -12,6 +12,8 @@ import json
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
from .build import _normalize_hyperedge_members
|
||||
|
||||
# Labels longer than this many characters, or containing >= this many words,
|
||||
# are candidates for being sentence-like rationale text rather than entity names.
|
||||
_RATIONALE_MIN_CHARS = 80
|
||||
@@ -101,6 +103,10 @@ def validate_semantic_fragment(fragment: object) -> list[str]:
|
||||
if not isinstance(he, dict):
|
||||
errors.append(f"hyperedges[{i}] must be an object")
|
||||
continue
|
||||
# Fold alias member keys (members/node_ids) onto `nodes` (#1561) so
|
||||
# an alias-keyed hyperedge isn't rejected here for "nodes must be a
|
||||
# list" before it ever reaches build's normalization.
|
||||
_normalize_hyperedge_members(he)
|
||||
_validate_semantic_id(errors, f"hyperedges[{i}].id", he.get("id"))
|
||||
he_nodes = he.get("nodes")
|
||||
if not isinstance(he_nodes, list):
|
||||
@@ -265,6 +271,10 @@ def sanitize_semantic_fragment(fragment: dict) -> dict:
|
||||
for he in hyperedges:
|
||||
if not isinstance(he, dict):
|
||||
continue
|
||||
# Fold alias member keys (members/node_ids) onto `nodes` (#1561) so an
|
||||
# alias-keyed hyperedge isn't silently dropped below for a missing
|
||||
# `nodes` list before build can canonicalize it.
|
||||
_normalize_hyperedge_members(he)
|
||||
he_nodes = he.get("nodes")
|
||||
if not isinstance(he_nodes, list):
|
||||
continue
|
||||
|
||||
@@ -233,3 +233,96 @@ def test_report_skips_hyperedges_section_when_key_missing():
|
||||
G = build_from_json(extraction)
|
||||
report = _make_report(G)
|
||||
assert "## Hyperedges" not in report
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 7. Hyperedge member-key alias normalization (#1561)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _alias_extraction():
|
||||
"""Three hyperedges, one per member-key spelling: nodes / members / node_ids."""
|
||||
return {
|
||||
"nodes": [
|
||||
{"id": "a", "label": "A", "file_type": "code", "source_file": "m.py"},
|
||||
{"id": "b", "label": "B", "file_type": "code", "source_file": "m.py"},
|
||||
{"id": "c", "label": "C", "file_type": "code", "source_file": "m.py"},
|
||||
],
|
||||
"edges": [],
|
||||
"hyperedges": [
|
||||
{"id": "he_nodes", "label": "canon", "nodes": ["a", "b", "c"]},
|
||||
{"id": "he_members", "label": "alias1", "members": ["a", "b", "c"]},
|
||||
{"id": "he_node_ids", "label": "alias2", "node_ids": ["a", "b", "c"]},
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
def test_build_normalizes_member_aliases_to_nodes():
|
||||
G = build_from_json(_alias_extraction())
|
||||
hes = {he["id"]: he for he in G.graph["hyperedges"]}
|
||||
for hid in ("he_nodes", "he_members", "he_node_ids"):
|
||||
assert hes[hid]["nodes"] == ["a", "b", "c"], hid
|
||||
# alias keys are dropped post-normalization
|
||||
assert "members" not in hes[hid]
|
||||
assert "node_ids" not in hes[hid]
|
||||
|
||||
|
||||
def test_build_dedups_alias_members_preserving_order():
|
||||
extraction = {
|
||||
"nodes": [
|
||||
{"id": "a", "label": "A", "file_type": "code", "source_file": "m.py"},
|
||||
{"id": "b", "label": "B", "file_type": "code", "source_file": "m.py"},
|
||||
],
|
||||
"edges": [],
|
||||
"hyperedges": [{"id": "h", "label": "x", "members": ["a", "a", "b"]}],
|
||||
}
|
||||
G = build_from_json(extraction)
|
||||
assert G.graph["hyperedges"][0]["nodes"] == ["a", "b"]
|
||||
assert "members" not in G.graph["hyperedges"][0]
|
||||
|
||||
|
||||
def test_build_canonical_nodes_wins_over_alias():
|
||||
extraction = {
|
||||
"nodes": [
|
||||
{"id": "a", "label": "A", "file_type": "code", "source_file": "m.py"},
|
||||
{"id": "b", "label": "B", "file_type": "code", "source_file": "m.py"},
|
||||
{"id": "x", "label": "X", "file_type": "code", "source_file": "m.py"},
|
||||
],
|
||||
"edges": [],
|
||||
"hyperedges": [
|
||||
{"id": "h", "label": "x", "nodes": ["a", "b"], "members": ["x"]},
|
||||
],
|
||||
}
|
||||
G = build_from_json(extraction)
|
||||
he = G.graph["hyperedges"][0]
|
||||
assert he["nodes"] == ["a", "b"] # canonical untouched
|
||||
assert "members" not in he # stray alias dropped
|
||||
|
||||
|
||||
def test_build_rekeys_alias_keyed_hyperedge_members():
|
||||
"""Alias normalization must run BEFORE the semantic id-remap loop so a
|
||||
`members`-keyed hyperedge's refs get rekeyed alongside `nodes`-keyed ones."""
|
||||
# Non-AST node whose id uses the OLD short stem (`mod_foo`) for source_file
|
||||
# pkg/mod.py -> new canonical stem pkg_mod -> remap mod_foo => pkg_mod_foo.
|
||||
extraction = {
|
||||
"nodes": [
|
||||
{"id": "mod_foo", "label": "foo", "file_type": "code", "source_file": "pkg/mod.py"},
|
||||
{"id": "mod_bar", "label": "bar", "file_type": "code", "source_file": "pkg/mod.py"},
|
||||
],
|
||||
"edges": [],
|
||||
"hyperedges": [
|
||||
{"id": "h", "label": "x", "members": ["mod_foo", "mod_bar"]},
|
||||
],
|
||||
}
|
||||
G = build_from_json(extraction)
|
||||
he = G.graph["hyperedges"][0]
|
||||
assert he["nodes"] == ["pkg_mod_foo", "pkg_mod_bar"]
|
||||
|
||||
|
||||
def test_build_warns_once_per_aliased_hyperedge(capsys):
|
||||
build_from_json(_alias_extraction())
|
||||
err = capsys.readouterr().err
|
||||
# one warning each for the two alias hyperedges, none for the nodes-keyed one
|
||||
assert err.count("normalizing") == 2
|
||||
assert "he_members" in err and "members" in err
|
||||
assert "he_node_ids" in err and "node_ids" in err
|
||||
assert "he_nodes" not in err
|
||||
|
||||
@@ -342,3 +342,36 @@ def test_sanitize_rationale_only_propagates_through_rationale_for_edges():
|
||||
assert "tree-sitter" in ids["rationale_target"].get("rationale", "")
|
||||
# unrelated_target should NOT have rationale leaked from the `references` edge
|
||||
assert "rationale" not in ids["unrelated_target"]
|
||||
|
||||
|
||||
def test_sanitize_keeps_members_keyed_hyperedge(capsys):
|
||||
"""#1561: a `members`-keyed hyperedge with >=2 surviving members must be
|
||||
KEPT (normalized to `nodes`), not silently dropped before build."""
|
||||
fragment = {
|
||||
"nodes": [
|
||||
{"id": "real_a", "label": "A", "file_type": "code"},
|
||||
{"id": "real_b", "label": "B", "file_type": "code"},
|
||||
],
|
||||
"edges": [],
|
||||
"hyperedges": [
|
||||
{"id": "grp", "label": "Group", "members": ["real_a", "real_b"]},
|
||||
],
|
||||
}
|
||||
out = sc.sanitize_semantic_fragment(fragment)
|
||||
assert len(out["hyperedges"]) == 1
|
||||
he = out["hyperedges"][0]
|
||||
assert he["id"] == "grp"
|
||||
assert he["nodes"] == ["real_a", "real_b"]
|
||||
assert "members" not in he
|
||||
|
||||
|
||||
def test_validate_accepts_node_ids_keyed_hyperedge():
|
||||
"""#1561: an alias-keyed hyperedge must not be rejected for a missing
|
||||
`nodes` list — validate normalizes first."""
|
||||
fragment = _valid_fragment()
|
||||
fragment["nodes"].append({"id": "second", "label": "Second", "file_type": "code"})
|
||||
fragment["hyperedges"] = [
|
||||
{"id": "grp", "label": "G", "node_ids": ["module_func", "second"]}
|
||||
]
|
||||
errors = sc.validate_semantic_fragment(fragment)
|
||||
assert errors == []
|
||||
|
||||
Reference in New Issue
Block a user