fix(dedup): rewire hyperedge members onto survivors instead of dropping them (#2805) - #2820
Conversation
…ng them (Graphify-Labs#2805) build() rewires EDGE endpoints to dedup survivors, but combined["hyperedges"] never went through the same remap -- dedup.py had no reference to hyperedges at all. The member naming a merged-away id was simply absent from the rebuilt graph. The failure is quiet in the worst way. The group loses a participant, can fall under the 3-member threshold that makes it a hyperedge at all, and leaves NO dangling reference behind -- so a referential-integrity check sees a perfectly consistent graph. On the reporter's fixture a 3-member group came back with 2. _normalize_hyperedge_members / _coerce_hyperedge_member_refs normalise member SHAPE (bare id vs object carrying one) but never resolve a member against surviving node ids, so they did not cover this. deduplicate_entities now takes an optional `hyperedges` list and rewires member ids in place, alongside the edge rewiring it already did. Optional and mutating rather than added to the return tuple so every existing two-tuple caller is unaffected; deduplicate_by_label has the same shape and is left alone since nothing routes hyperedges through it. Both member shapes are handled, other fields on object members are preserved, and member order is kept so a rebuilt graph does not churn. Two members that remap onto the SAME survivor collapse to one entry: that shrinks the group, but they were the same entity -- where the old behaviour shrank it and lost the participant as well.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Reverts the 0.9.45 changelog entries back to an unreleased 0.9.44, dropping the Go case-collision id salting (symbol_nid/_scan_declarations and the hashlib import in extractors/go.py) and the id-less-hyperedge .get("id") guard in attach_hyperedges. Adds hyperedge member rewiring to deduplicate_entities via a new _remap_hyperedge_members and a hyperedges kwarg threaded through build, so members follow dedup survivors like edge endpoints (#2805). Also re-exports _refresh_all_version_stamps from install.
Worth a look
- attach_hyperedges reverts KeyError guard for id-less hyperedges —
graphify/export.py:166· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
- Existing id-less hyperedges crash attach_hyperedges —
graphify/export.py:166· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
- KeyError revert: id-less hyperedges crash attach_hyperedges again —
graphify/export.py:166· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
- Go case-only declarations collapse to one node again —
graphify/extractors/go.py:207· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1903 functions depend on the 917 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 95 callers, 51 callees - new:
build_from_json()— 151 callers, 18 callees - new:
deduplicate_entities()— 63 callers, 21 callees - new:
build_merge()— 41 callers, 14 callees - new:
to_obsidian()— 29 callers, 12 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
to_wiki()— 41 callers, 7 callees - new:
_call_claude_cli()— 31 callers, 9 callees - …and 63 more — each is listed as a finding
Verification — 1903 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: 1701 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify build.
The verifier did not have enough to check build, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify deduplicate\_entities.
The verifier did not have enough to check deduplicate\_entities, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in attach\_hyperedges (not a proof).
The verifier ran both versions of attach\_hyperedges on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify extract\_go.
The verifier did not have enough to check extract\_go, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify install.
The verifier did not have enough to check install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_\_init\_\_.
The verifier did not have enough to check \_\_init\_\_, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
· 1 grounded finding(s) anchored inline below; 70 more finding(s) on lines outside this diff (see the check run).
| he["nodes"] = rewired | ||
|
|
||
|
|
||
| def deduplicate_entities( |
There was a problem hiding this comment.
deduplicate_entities()
fans out to 21 callees (efferent coupling); 63 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Landed in v0.9.46, just published to PyPI. Cherry-picked onto |
Fixes #2805.
Thanks @vihavald — the diagnosis was exact, including the observation that the newer behaviour is the harder one to catch. That turned out to be the most important thing about this bug.
The bug
build()rewires edge endpoints to dedup survivors.combined["hyperedges"]never went through the same remap —dedup.pycontained no reference to hyperedges at all — so the member naming a merged-away id was simply absent from the rebuilt graph.Your reproduction, verbatim:
The reason this deserves the fix rather than a note in the docs is the part you flagged: there is no dangling reference left behind. The group loses a participant, can fall under the 3-member threshold that makes it a hyperedge at all, and the resulting graph passes any referential-integrity check cleanly. Nothing on stderr, nothing in the artifact. The 0.8.29 dangling-member behaviour was at least detectable.
You were also right that
_normalize_hyperedge_members/_coerce_hyperedge_member_refsdo not cover it — they normalise member shape, never resolving a member against surviving node ids.The change
deduplicate_entitiestakes an optionalhyperedgeslist and rewires member ids in place, alongside the edge rewiring it already did.Optional and mutating rather than added to the return tuple, so every existing two-tuple caller is unaffected — that seemed the right trade against changing a public-ish signature for one caller. Say the word if you would rather it returned a triple.
deduplicate_by_labelhas the identical shape and is deliberately left alone: nothing currently routes hyperedges through it, and fixing a path that has no caller would be speculative. Worth noting as a live footgun if that changes.Handled: both member shapes (bare id and object carrying one), other fields on object members preserved, and member order kept so a rebuilt graph does not churn.
One judgement call worth review. Two members that remap onto the same survivor collapse to a single entry, so the group does shrink — but they were the same entity, which is the honest result. The old behaviour shrank the group and lost the participant. There is a test pinning this so it reads as intentional rather than as the original bug surviving in a corner.
Tests
tests/test_dedup_remaps_hyperedges.py(14 tests): the member follows the survivor, the group keeps its size, no member points at a merged-away id, object-shaped members remap too, and an untouched hyperedge is byte-identical. Then the helper directly — collapse-onto-one-survivor, order preservation, other fields kept, an empty remap changing nothing, and five malformed-hyperedge shapes that must not raise.Reverting the
build.pycall site while keeping the helper fails 3 of them. I checked it that way deliberately: reverting both files makes the module fail to import, which would have "failed" the whole file and told me nothing about which behaviour each test actually pins.Validation
Windows 11, Python 3.12, branched off
4fca621(0.9.44).20 failed, 4469 passed->21 failed, 4482 passed. The +13 are the new tests.test_incremental_mtime_collision.py::test_same_size_rewrite_in_one_tick_is_requeued, untouched by this PR. It needs two writes in one filesystem timestamp tick; this run took 9:34 against a ~6:30 baseline, so the machine was loaded. Passes in isolation. It has now surfaced this way on four unrelated branches of mine, so I will open a separate issue for the flake rather than keep footnoting it.