fix: enforce the 3-member hyperedge invariant at every persistence boundary by egarcia74 · Pull Request #3298 · Graphify-Labs/graphify

egarcia74 · 2026-09-02T18:26:39Z

Establishes the 3-member hyperedge invariant and applies it at every path that persists hyperedges. Enforcement is one shared gate rather than a check per site, because the audit in the issue found twelve independent writers. Review found a thirteenth, and a fourteenth site applies it on the read side, so a pre-gate cache entry cannot replay forever.

Summary

Enforce Graphify's documented hyperedge contract: a hyperedge models a group relationship, so it needs at least three distinct, resolvable members. A pair belongs in the ordinary edge set.

The original bug: valid 3+ member input could degrade to a pair once unresolved members were pruned, and then persist in the semantic cache and in graph.json. Fixing it properly meant fixing the shape problem underneath first — alias keys, object members, duplicate ids, numeric/string-equivalent ids and unusable ids all have to be settled before a count can mean anything.

The one gate, and every boundary that applies it

graphify.build.canonical_hyperedge(he) settles the shape, on a copy, so no caller's data is mutated:

fold the members / node_ids aliases onto nodes
coerce object members {"id": "a"} and numeric ids to canonical scalar ids, deduped in order
reject a non-list nodes, and members that can never name a node (None, "", booleans)
keep it only if three distinct members remain

gate_hyperedges(hyperedges, nodes) and gate_hyperedges_against_graph(hyperedges, G) add membership on top, and every writer goes through one of them. Membership is deliberately not in canonical_hyperedge: it needs the id-space bridge below, and having two membership rules for one invariant is how several of the defects listed further down arose. Boundaries with no node set — the semantic cache, the pre-manifest-stamp gate — call canonical_hyperedge directly.

Member refs are coerced so they can be compared at all (7 and "7" are one member, not two), which means every id set a member is compared against has to be coerced too — and whatever survives has to be handed back in the id space the caller is about to persist, or the written file names members no node has. Both halves of that are now in one place: _id_map maps a coerced id back to the id its container actually carries, and a single _gate core takes that prebuilt map. Getting this wrong at individual sites accounted for five of the defects below.

flowchart TD
    LLM["semantic extraction<br/>LLM or cache replay"]
    SKILL["merge-chunks skill<br/>raw JSON extend"]
    DISK[("existing graph.json")]
    OUT[("graph.json")]
    CACHE[("semantic cache")]

    LLM --> SEM["sem_result"]
    SKILL -->|"② save_semantic_cache<br/>both write gates"| CACHE
    SEM -->|"②"| CACHE
    CACHE -.->|"⑭ replay<br/>canonicalize on read"| SEM

    SEM -->|"① before manifest stamping<br/>shape + cardinality"| MERGED["merged"]
    MERGED --> MODE{"--no-cluster ?"}

    MODE -->|"no"| BUILD["build()"]
    BUILD -->|"③ normalize before dedup"| DEDUP["deduplicate_entities"]
    DEDUP -->|"④ all four exit paths"| BFJ["build_from_json"]
    BFJ -->|"⑤ distinct count, reject malformed"| BM["build_merge"]
    DISK --> BM
    BM -->|"⑥ final gate, prune or not"| TOJSON["to_json"]
    TOJSON -->|"⑦ last boundary"| OUT

    MODE -->|"yes"| RAW["cli raw write path"]
    RAW -->|"⑧ vs final node ids"| OUT
    DISK -->|"⑨ exclusion-only prune<br/>both JSON slots"| OUT

    DISK --> PREFIX["prefix_graph_for_global"]
    PREFIX -->|"⑩ normalize before prefixing"| ATTACH["attach_hyperedges"]
    ATTACH -->|"⑪ vs composed graph"| OUT

    UPD["graphify update --no-cluster<br/>watch._rebuild_code"] -->|"⑫ vs candidate node ids"| OUT
    DISK --> MD["git merge<br/>cli merge-driver"]
    MD -->|"⑬ vs composed nodes"| OUT
    DISK --> UPD

#	Where	What it catches
①	`cli` pre-merge / pre-stamp	a doc whose only output is an under-cardinality group being stamped as extracted
②	`cache.save_semantic_cache` ×2	alias-keyed groups silently dropped; duplicate-, null- and numeric-padded groups cached as valid
③	`build.build`	alias-keyed group reaching dedup un-rewired
④	`dedup.deduplicate_entities`	three early returns skipped the hyperedge pass; unusable entries counted as members
⑤	`build.build_from_json`	re-key, doc-twin and `norm_to_id` remaps collapsing two ids into one; malformed shapes persisted verbatim
⑥	`build.build_merge`	carried group degraded by an incremental update, prune or no prune
⑦	`export.to_json`	the public writer took `G.graph["hyperedges"]` verbatim
⑧	`cli` raw `--no-cluster` block	never reaches `build_from_json` / `to_json` at all
⑨	`cli._prune_graph_json_sources`	members left dangling by a prune; stale nested slot
⑩	`build.prefix_graph_for_global`	unprefixed members discarded on cross-repo merge
⑪	`export.attach_hyperedges`	`merge-graphs` metadata never sees `build_from_json`
⑫	`watch._rebuild_code` raw writer	`graphify update --no-cluster` carried a legacy pair through every rebuild
⑬	`cli merge-driver`	git-merged graph attrs serialized straight back, so a legacy branch's pair or dangling group survives
⑭	`cache.check_semantic_cache` (read)	an entry written before the gate existed replaying as a hit forever

Defects found during review

The first pass fixed the reported symptom at three sites. Review across CodeRabbit, Codex and Copilot then found defects at the boundaries that pass had missed — several of them introduced by the fix itself, which is the honest cost of adding a cross-cutting invariant to twelve separate write paths:

Write paths the first pass missed

watch's raw writer had no gate at all. _rebuild_code spreads result into the candidate JSON with hyperedges included, and _reconcile_existing_graph evicts only by source and dangling members, so graphify update --no-cluster preserved a legacy pair on every rebuild.
export.to_json took G.graph["hyperedges"] verbatim — public API, and the function that actually writes the file.
The raw --no-cluster path and the exclusion-only prune both write graph.json without ever building a graph.

Defects introduced by the fix, found in review

to_json mutated the caller's graph. node_link_data returns the same graph-attrs dict G owns, so writing the sorted list edited the caller's object. Harmless while it only reordered; once filtered it would have deleted their hyperedges.
A nested-only hyperedge slot was wiped. to_json persists both slots; a node_link_data-only file has just the nested one. The prune read the empty top level, found nothing, then overwrote the nested slot with that empty result.
merge-graphs discarded groups by member shape. prefix_graph_for_global prefixed only a canonical nodes list, so alias-keyed and object-member groups kept unprefixed ids while every node gained a repo:: prefix, and the attach gate then dropped them.
A malformed node id aborted the prune with TypeError: unhashable type — the build path deliberately tolerates a persisted list/dict id.
The member usability rule drifted four times between the bare and object branches: None, then bare booleans, then object-wrapped booleans, then the numeric coercion itself, so [7, "7", "b"] passed as a group and collapsed to a pair on replay. Both branches now share one _is_usable_member_ref predicate and the same _coerce_id coercion.
dedup counted positions, not usable members — [None, None, None] cleared the minimum.
build_from_json persisted malformed shapes ({"nodes": "a,b,c"}, a bare string) that its own gate rejects, into metadata the report, wiki, html exporters and watch all read back.
Optimistic manifest stamping counted a hyperedge as output before the gate ran.
The gate compared ids in one space and wrote them back in another. Member refs are coerced (7 → "7") so they can be compared at all, but the writers persist their node records unchanged. Coercing only the comparison side left graph.json holding nodes [7, 8, 9] and members ["7", "8", "9"] — a dangling reference of exactly the shape fix(cache): prune edges/hyperedges referencing never-written node groups #1916 removed, written by the gate that exists to prevent it. It took three rounds to find every site: first the node-list path, then the graph-container path, then to_json, then the three writers that persist node records (the raw --no-cluster writer, watch's raw writer, and the graph.json pruner). The two spaces are now bridged in one place — _id_map maps a coerced id back to the id its container actually carries, and a single _gate core indexes it for every survivor.
The gate rebuilt its id map per candidate, twice. Gating one hyperedge at a time re-walked every node for each group: measured 1060 _coerce_id calls for 50 nodes and 20 groups where linear is 110. This turned merge-graphs from linear into O(nodes × hyperedges) on exactly the thousands-of-groups corpora the invariant was added for. Fixed once in prefix_graph_for_global, then reintroduced one commit later in attach_hyperedges via a helper that built its map internally — so the map is no longer built inside any per-candidate path, and both call sites gate whole lists. A _coerce_id counter now guards each.
Four more comparisons disagreed with the gate about what resolves. Same cause as the prefixer above, in four more places: semantic_cleanup's member filter and watch's reconciliation each dropped a group the gate keeps, and both of the cache's dangling prunes — the skipped-node one and scope_semantic_result's replay one — did the opposite, keeping a group whose member named a node deliberately never written. Under-pruning is the harder direction to notice, because nothing looks missing. Rather than fix four comparisons, there is now one definition of what a member names: _member_keys yields the two lookup keys in priority order, resolve_member_ref returns the raw id for callers writing members back out, and member_in_id_space answers yes/no for callers that only decide. semantic_cleanup's filter is deleted in favour of calling the gate outright.
The cross-repo prefixer disagreed with the gate about what resolves. Once the gate learned to resolve a member that drifted in casing or punctuation, prefix_graph_for_global still keyed its relabel table on the coerced spelling alone, so drifted members stayed unprefixed while every node gained the repo:: prefix and attach_hyperedges discarded a group the gate calls valid. merge-graphs lost it silently. The relabel table is keyed through _id_map now, so both halves resolve identically.
Two collision cases from expanding the key space. Adding normalized lookup keys to the shared id sets broke two things downstream. The cache's duplicate-attribution step subtracts one id set from another, so a skipped node foo_bar and a distinct written node Foo-Bar shared the key foo_bar and the skipped node vanished from the set, leaving a member that names it undetected. And with distinct nodes 7 and "7", a member 7 coerced to "7" and was restored onto the string node — silently rebound from the node it named to a colliding one, which is worse than a dangling member because it looks correct. Duplicate attribution is now decided in the exact space before aliases are added, and a member's own raw form wins over the map's choice.
A non-dict graph value aborted the exclusion-only prune. Reading the nested hyperedge slot as (data.get("graph") or {}).get(...) raises AttributeError on a str, list or int, and the surrounding try covers only the JSON load, so the exception left the function and the whole prune stopped. The nested-slot sync a few lines below already guarded with isinstance, making this an inconsistency inside one function — on a hand-editable graph.json, the least trusted input in the feature.
A raw set was trusted as already coerced, so numeric ids behaved differently from a list or a graph: members became "7" and then failed membership against {7, 8, 9}, dropping a valid group. Removed with the node_ids parameter above, with a test across set, frozenset and list so the exemption cannot return as an optimization.
Two membership rules for one invariant. Found by auditing my own change rather than by a reviewer. Once the gate took over membership resolution, canonical_hyperedge's node_ids parameter had no production caller left — verified by AST — while its semantics had diverged from the gate's: exact matching only, no normalized fallback, members returned coerced rather than in the container's id space. The weaker rule was reachable only from tests, and it was the one this description previously advertised as the gate. Removed, so membership has exactly one implementation; the tests that covered the parameter now exercise the path production actually uses.
The raw gate dropped groups the clustered path heals. build_from_json rewires a member that drifted in casing or punctuation through norm_to_id, so a group naming Foo-Bar for node foo_bar survives on the clustered path. The raw --no-cluster and watch writers never reach it, so the exact-membership test in the new gate discarded a perfectly resolvable group — silently lost by the gate added to protect it. Members now resolve through the normalized space too, with exact keys always winning and a second dedupe pass so two refs onto one node cannot pass as a group.
Direct deduplicate_entities callers got their groups mutated into dangling references. With numeric node ids and nothing to merge, the remap pass coerced every member to "7" while the returned node records kept 7. Introduced by consolidating dedup's four exits: before that, the empty-remap return skipped the hyperedge pass entirely. Members are restored to the returned nodes' id space now; unresolved members are left alone, because that function remaps and does not gate membership.
The git merge-driver had no gate at all. It composes two graph.json files and serializes the result directly, reaching neither build_from_json nor to_json, so a legacy branch's pair or dangling group was written straight back. nx.compose also takes graph attributes from one side only, so the valid groups on the current side were already being discarded before serialization — pre-existing compose behaviour, left alone deliberately rather than folded into this PR.
The cache's skipped-node prune compared raw ids against coerced members. save_semantic_cache drops a group that names a node from a deliberately skipped source (fix(cache): prune edges/hyperedges referencing never-written node groups #1916), but collected those ids raw while the members had already been coerced, so "7" in {7} was False. Same group and same skipped source, member id changed: a string id was pruned, a numeric id was cached and dangled on every replay. Edge endpoints reach the cache raw and were matching a raw set, so they are coerced at the lookup now too.
The cache gated writes but not reads. An entry written before the gate existed, holding only a two-member group, replayed as a hit forever; check_semantic_cache now canonicalizes on read, so a fragment with no nodes and no surviving group is a miss.
semantic_cleanup built its surviving-id set raw while members are coerced, so a valid group over numeric node ids lost every member before build_from_json could heal the ids. Third site with that same root cause.
to_json raised TypeError before writing anything when a caller left the hyperedge slot as None, losing the whole export rather than just the hyperedges.
A cache test had gone vacuous. Its two-member fixture was dropped by the cardinality gate before the skipped-node prune it exists to test ever ran, so it passed for the wrong reason.

Validation

Check	Result
CI `test (3.10)` / `test (3.12)`	pass
CI `skillgen-check` / `security-scan`	pass
The 14 test files this PR touches	615 passed, 3 skipped
Full suite (local)	5,319 passed, 56 skipped
`ruff check graphify/`	pass
`bandit -ll` over changed files	no issues
CodeRabbit CLI, full PR diff	0 findings
Docstring coverage, functions touched	100% (190)

7 production files, 14 test files. Local runs also show 6 failures in tests/test_ollama*.py; these are environmental (an exported OPENAI_API_KEY with the openai package absent) and reproduce identically on an untouched checkout. CI does not set that variable and passes.

Known shortcomings

Reviewed and consciously accepted rather than overlooked. Each was measured, not assumed.

A dangling-member group leaves a doc stamped, and the #2927 heal cannot clear it

Pinned by test_dangling_member_hyperedge_only_doc_stamps_then_re_queues_from_cache.

A group can pass shape and cardinality — three distinct, well-formed member ids — while none of those ids resolves to a node. Membership cannot be checked before stamping or caching (see below), so the doc is stamped on the strength of it, and the graph gate then drops the group. Observed across two runs:

RUN1: llm_calls=1  stamped=True  hyperedges=[]
RUN2: llm_calls=1  stamped=True  hyperedges=[]
      "re-queuing 1 manifest-stamped semantic file(s) ... (#2927)"
      "semantic cache: 1 hit / 0 miss"

The heal does fire and re-queue the doc, but the group was already cached, so the re-queue is served from cache, the group is dropped again, and the doc is re-stamped. The heal therefore re-fires on every subsequent run without ever resolving.

Impact: a recurring #2927 re-queue line and a re-queue that never completes. No LLM spend (it is a cache hit) and no invalid data in graph.json. Noise, not corruption.

Fixing it needs a design change, not another gate — either stamp after the graph is built so the real node set is available, or have the heal invalidate the cache entry it re-queues, which trades a free loop for a paid one. Both are larger than this PR. An earlier revision of this description claimed the heal recovered this in one run; that was wrong and is corrected here.

A third instance of the same limitation: with --no-cluster and no changed, deleted or stale files, extract prints "no incremental changes detected" and exits before any gate runs, so a pre-upgrade pair already on disk survives indefinitely and keeps its doc counted as output. Healing it would mean rewriting graph.json on a run the tool has just reported as a no-op, which is a migration of pre-existing files rather than an application of this invariant — and the same argument would apply to every legacy shape the gates reject, not just hyperedges. Raised by review and answered rather than fixed; happy to include the migration if maintainers would rather have it here.

Why membership is not validated before stamping or caching

Not an oversight — a raw member id absent from the merged node set can still resolve to a surviving node, because build_from_json applies _semantic_id_remap and norm_to_id afterwards. Measured:

pre-build view (raw ids):        canonical_hyperedge(he, raw_ids) -> DROPPED
build-time reality (norm_to_id): graph.json -> [{"id":"grp","nodes":["m_foo","m_bar","m_baz"]}]

Gating on raw ids would re-dispatch docs whose groups actually survive, on every run — a permanent cost regression, worse than the noise above. On the cache path there are two further blockers: fragments are stored per file and a group may legitimately span files, so validating against a fragment's own nodes would drop every cross-file group; and llm._checkpoint_chunk calls save_semantic_cache mid-extraction, where no merged node set exists at all. CodeRabbit independently reached the same conclusion and withdrew its finding here.

Smaller gaps, all pre-existing

to_graphml serializes G.graph verbatim, so a sub-minimum group in graph metadata still reaches a GraphML export. graph.json is gated; GraphML is not.
dedupe_nodes raises TypeError on a persisted malformed list/dict node id, before the raw --no-cluster gate is reached. The CLI prune path now tolerates such ids; this one still aborts.
merge_raw_extraction writes its internal _unverified_semantic_shrink key into graph.json and never pops it.

Structural note

The invariant is enforced at thirteen write boundaries plus one read boundary rather than one choke point, because hyperedges are persisted from several places that do not share a code path. The shared canonical_hyperedge gate makes each site one line, but nothing structurally prevents a future writer from missing it — watch's raw writer and the git merge-driver were exactly that omission, found only by review — as were to_json, the raw --no-cluster block and the exclusion-only prune. Two of the fourteen boundaries were found by reviewers rather than by the audit, which is the measured version of this concern rather than a hypothetical one. The sharper measurement is the id-space rule underneath it: twelve sites compared member refs against node ids, and each time the cause was adding a coercion or resolution rule for one comparison without asking which other places compare the same values. An earlier revision of this description said eight of those twelve defects were created by this change. That was wrong, and the correction matters because it overstated the damage: checked out at v8 and run directly, semantic_cleanup already persisted a two-member group, the cross-repo prefixer already wrote a dangling one, and both cache prunes already failed to prune. Roughly half were pre-existing defects of this same family, surfaced because this change built the one shared rule that makes them visible; the other half — listed below — were genuinely mine. There is now exactly one implementation of that rule (_member_keys, reached through _id_map, resolve_member_ref and member_in_id_space), which is what makes a thirteenth site structurally hard rather than merely unlikely. A single persistence funnel would be the durable fix.

Notes for review

Applies cleanly — branched from v8 @ 33362d9, which is current v8 head at the time of writing.
Reviewed by CodeRabbit, Codex and GitHub Copilot across several rounds on my fork (fix: enforce minimum hyperedge cardinality egarcia74/graphify#3 and Your project was featured on AI Digital Crew 🎉 #6: 46 threads resolved (40 on Wroked out examples missing graph.html #3, 6 on Your project was featured on AI Digital Crew 🎉 #6), 1 left open by design — see the no-change exit under Known shortcomings). Roughly half the defects they found were in this fix rather than the original code; those are listed above rather than quietly folded in, because they are the argument for the structural note at the end.
Commits are authored by me with Co-Authored-By: Claude trailers — the work was done with AI assistance and the history says so.
Squashed to a single commit. The branch reached 33 commits, many of them fixing defects in earlier commits of the same branch, which is not a series worth reading. The content is unchanged — verified byte-identical to the 33-commit version before the force-push. I tried splitting it into a layered series first; build.py alone leaves 24 tests failing because the boundaries genuinely depend on the shared gate, so a multi-commit series would have had broken intermediate states. One honest commit beat five fabricated ones.

graphify-labs

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.

Formal verification. 2 change(s) alter behavior, breaking input(s) attached.

Behavior changes: \_coerce\_hyperedge\_member\_refs changes behavior, here is the input that shows it.

The verifier found a concrete input on which \_coerce\_hyperedge\_member\_refs behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"he":"\{'n': 0, 's': 'x', 'l': \[1, 2\]\}","members":"\[1, 2, 3\]"\}, the old code produced \[1, 2, 3\] but the new code produces \['1', '2', '3'\]. Paste that input straight into a regression test.

Behavior changes: attach\_hyperedges changes behavior, here is the input that shows it.

The verifier found a concrete input on which attach\_hyperedges behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\('a', \{'label': 'A', 'kind': 'fn'\}\), \('b', \{'label': 'B'\}\)\]\), \_g\.add\_edges\_from\(\[\('a', 'b', \{'weight': 2, 'kind': 'calls'\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","hyperedges":"\[0, 0, 0, 0\]"\}, the old code produced raises AttributeError but the new code produces None. Paste that input straight into a regression test.

Graphify review — findings

Enforces a minimum of three distinct members for every hyperedge across all persistence boundaries, so a group relationship that prunes down to a pair or singleton is dropped rather than written to graph.json as a degraded or dangling edge. Routes shape settling and cardinality through a single canonical_hyperedge gate (with an optional node_ids/graph filter) and a shared _is_usable_member_ref rule that coerces and dedupes numeric ids identically in both member branches, closing a cache/replay mismatch where junk or duplicate members inflated a pair into a cached "group" that vanished on reload. Adds a build_merge final cardinality gate that runs whether or not a prune fired, and canonicalizes members before dedup in build so alias-keyed hyperedges get rewired onto survivors instead of passed through un-remapped.

Worth a look

Top-level import creates a build/dedup circular import — graphify/dedup.py:15 · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
New build import creates a circular import with build's dedup dependency — graphify/dedup.py:15 · Escalate · high
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
build_from_json now drops previously legal single-member hyperedges — graphify/build.py:1417 · Escalate · medium · 2 independent checks
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
_normalize_hyperedge_members called on non-dict hyperedge in build() — graphify/build.py:1502 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
build() assumes combined['hyperedges'] key exists — graphify/build.py:1502 · Escalate · medium
- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 3367 functions depend on the 1606 functions this change touches.

Health — this change adds coupling hotspots:

new: extract() — 520 callers, 43 callees
new: _rebuild_code() — 114 callers, 51 callees
new: build_from_json() — 195 callers, 18 callees
new: detect() — 108 callers, 15 callees
new: deduplicate_entities() — 67 callers, 21 callees
new: build_merge() — 65 callers, 14 callees
new: save_semantic_cache() — 64 callers, 10 callees
new: to_obsidian() — 36 callers, 13 callees
…and 68 more — each is listed as a finding

Verification — 3367 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

verification_scope: 2854 function(s) in the blast radius were not formally verified this run

Formal verification