fix(build): route carried hyperedges through the dedup survivor remap (#3102) - #3113
fix(build): route carried hyperedges through the dedup survivor remap (#3102)#3113abhay-codes07 wants to merge 2 commits into
Conversation
…Graphify-Labs#3102) build_merge() carries hyperedges from unchanged files across an incremental rebuild (Graphify-Labs#1574). They were attached to G AFTER build() and entity dedup had finished, so while every edge endpoint was rewired onto the dedup survivor (Graphify-Labs#2805), a carried hyperedge kept naming the merged-away node: a dangling member in graph.json with no backing node, and a member list that disagreed with the edges about which node the concept is. The carried set now rides into build() on the base chunk, so dedup remaps its members exactly as it remaps the new chunks' hyperedges, and build_from_json validates them against the built node set like any other. The prune-set helpers move ahead of the build call (they never depended on it); a hyperedge the new chunks re-emit under the same id is left to them, preserving the id-dedup the old attach_hyperedges step provided.
![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 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Fixes carried-forward hyperedges naming a dedup survivor's merged-away node during incremental build_merge rebuilds: the carried set now rides into build() on the base chunk instead of being attached to G afterward, so entity dedup rewires hyperedge members onto the survivor the same way it rewires edge endpoints, leaving no dangling members in the written graph. Carried hyperedges whose id the new chunks re-emit are skipped so the re-extracted version wins, while re-extracted/deleted sources are dropped via the existing semantic-remap and prune checks.
Worth a look
- build_merge consumes one-shot new_chunks before building —
graphify/build.py:1810· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Existing hyperedges cause one-shot new_chunks iterators to be consumed before build —
graphify/build.py:1807· 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 — 1109 functions depend on the 93 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 50 callees - new:
build_from_json()— 190 callers, 18 callees - new:
build_merge()— 53 callers, 13 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
to_wiki()— 41 callers, 7 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
_call_claude_cli()— 31 callers, 9 callees - …and 38 more — each is listed as a finding
Verification — 1109 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: 728 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify build\_merge.
The verifier did not have enough to check build\_merge, 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 `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set
· 46 more finding(s) on lines outside this diff (see the check run).
|
Follow-up commit on the review's iterator point: |
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 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Fixes carried-forward hyperedges naming dedup-merged-away nodes during an incremental build_merge: the preserved hyperedges from unchanged files now ride in on the base chunk fed to build, so entity dedup rewires their members onto the survivor instead of leaving a dangling member with no backing node in the written graph. Also materialises new_chunks to a list up front since it's now iterated multiple times, and skips carrying any hyperedge whose id the new chunks re-emit (their version wins).
Worth a look
- Carried-hyperedge replacement compares raw IDs before build-time ID coercion —
graphify/build.py:1811· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Carried hyperedge de-dup by id assumes new chunk hyperedges always carry an id —
graphify/build.py:1829· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Carried hyperedge re-emit dedup only skips by id, dropping non-id hyperedges from dedup and possibly duplicating them —
graphify/build.py:1829· 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 — 1109 functions depend on the 93 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 50 callees - new:
build_from_json()— 190 callers, 18 callees - new:
build_merge()— 53 callers, 13 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
to_wiki()— 41 callers, 7 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
_call_claude_cli()— 31 callers, 9 callees - …and 38 more — each is listed as a finding
Verification — 1109 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: 728 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify build\_merge.
The verifier did not have enough to check build\_merge, 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 `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set
· 46 more finding(s) on lines outside this diff (see the check run).
|
Shipped in v0.9.51 via authorship-preserving cherry-pick so you keep contributor-graph credit. Thanks @abhay-codes07! Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.51 |
Closes #3102.
The problem
build_merge()carries hyperedges forward from files that were neither re-extracted nor deleted (#1574). They were attached toGafterbuild()and entity dedup had finished. 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. The written graph then had:G.graph["hyperedges"]with no backing graph node.The reporter's synthetic repro reproduces on 0.9.50 exactly as filed:
edge_rewired True,dangling_members ['alpha_concept_long_variant_id'].The change
The carried set is computed before the build and rides into
build()on the base chunk ({"nodes": existing_nodes, "edges": existing_edges, "hyperedges": carried}), so dedup remaps its members exactly as it remaps the new chunks' hyperedges, andbuild_from_jsonvalidates them against the built node set like any other. The prune-set helpers (_prune_match, the #2446 root derivation) move ahead of the build call — they never depended onG, only on the prune sources and the existing graph. The oldattach_hyperedgesstep also id-deduped against hyperedges the new chunks re-emitted; that is preserved by leaving a same-id hyperedge to the new chunk.Nothing about which hyperedges are carried changes: semantically re-extracted sources still replace theirs, pruned sources still drop theirs.
Tests
tests/test_carried_hyperedge_remap.py— 6 tests built on the issue's data: the carried hyperedge's members are remapped onto the survivor (not merely dropped); the writtengraph.jsonhas no hyperedge member without a node; edges and hyperedges agree on the survivor; a hyperedge re-emitted by the new chunk under the same id wins and is not duplicated; a pruned source's hyperedge is still dropped; and an unchanged hyperedge with no dedup involved is carried verbatim. With the reorder reverted, 3 of 6 fail.test_build_merge_hyperedges_and_prune,test_build, thetest_build_merge*, hyperedge and dedup suites are unchanged (219 passed); the full suite matches thev8baseline.