fix: excuse dedup-explained node drops from the #479 shrink guard - #3623
seonghobae wants to merge 1 commit into
Conversation
…ink guard build_from_json's Graphify-Labs#1145 ghost-merge pass collapses non-AST duplicate nodes (e.g. a manifest package node and its AST-canonical twin sharing the same source_file/label) into their AST canonical counterpart. That is a legitimate, data-preserving node-count reduction, but to_json's Graphify-Labs#479 shrink guard could not distinguish it from an unverified shrink and refused to overwrite graph.json even when nothing was actually lost — reproduced on a 23,934-node graph.json where reloading via build_from_json produced 23,933 NetworkX nodes (crate:mlsirm-core merged into its AST twin pkg_mlsirm_core, zero incident edges dropped) and cluster-only refused the write. build_from_json now records the collapsed-node count as G.graph["_ghost_dedup_count"]; to_json's shrink guard treats a node-count drop as safe when it is fully explained by that count, and still refuses when any part of the drop is unexplained. Fixes downstream report at ContextualWisdomLab/fast-mlsirm#1847. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXYRNTJhYLB2bYveuaFe74
![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.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_match\_cited\_file changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_match\_cited\_file 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 \{"cited":"'\./'","doc\_file":"'\.mdx'","source\_files":"\{1, 2, 3\}"\}, the old code produced None but the new code produces raises AttributeError. Paste that input straight into a regression test.
Graphify review — findings
Teaches the to_json shrink guard to distinguish legitimate node-count drops from data loss: build_from_json now records on G.graph["_ghost_dedup_count"] how many ghost nodes it collapsed into their (source_file, label) AST twins, and the guard subtracts that count before deciding. A drop fully explained by dedup proceeds; any unexplained remainder still refuses to overwrite (unless force=True), and the warning now reports the explained/unexplained split.
No blocking issues surfaced. 7 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1459 functions depend on the 414 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 123 callers, 53 callees - new:
build_from_json()— 209 callers, 19 callees - new:
build_merge()— 76 callers, 14 callees - new:
to_obsidian()— 38 callers, 14 callees - new:
to_json()— 58 callers, 7 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
build()— 52 callers, 6 callees - new:
_call_claude_cli()— 33 callers, 9 callees - …and 42 more — each is listed as a finding
Verification — 1459 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: 1055 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
92 of 283 test file(s) selected (33%) via static blast radius.
tests/test_analyze.py— impacttests/test_atomic_canvas_export.py— impacttests/test_atomic_writes.py— impacttests/test_benchmark.py— impacttests/test_benchmark_raw_graph.py— impacttests/test_build.py— impact, changed-testtests/test_build_merge_dedup_scope.py— impacttests/test_build_merge_hyperedges_and_prune.py— impacttests/test_build_merge_shrink_guard.py— impacttests/test_carried_hyperedge_remap.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_claude_cli_backend.py— impacttests/test_cli_export.py— impacttests/test_cluster.py— impacttests/test_community_labels_skill.py— impacttests/test_confidence.py— impacttests/test_corrupt_graph_json.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_dedup.py— impacttests/test_dedup_remaps_hyperedges.py— impacttests/test_definition_file_portability.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_evidence_binding.py— impacttests/test_export.py— impact, changed-testtests/test_export_control_characters.py— impacttests/test_export_path_length.py— impacttests/test_extract.py— impacttests/test_falkordb_integration.py— impacttests/test_file_label_disambiguation.py— impacttests/test_global_add_tag_inference.py— impacttests/test_global_graph.py— impacttests/test_go_qualified_resolution.py— impacttests/test_god_nodes_exclude_hubs.py— impacttests/test_hyperedge_member_shapes.py— impacttests/test_hyperedge_roundtrip.py— impacttests/test_hypergraph.py— impacttests/test_image_vision.py— impacttests/test_import_self_loops.py— impacttests/test_issue_3472_source_file_collision.py— impacttests/test_java_type_resolution.py— impacttests/test_languages.py— impacttests/test_llm_backends.py— impacttests/test_llm_parser.py— impacttests/test_llm_parser_reasoning.py— impacttests/test_loose_sibling_import_resolution.py— impacttests/test_lua_import.py— impacttests/test_manifest_ingest.py— impact- … and 42 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Behavior changes: \_match\_cited\_file changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_match\_cited\_file 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 \{"cited":"'\./'","doc\_file":"'\.mdx'","source\_files":"\{1, 2, 3\}"\}, the old code produced None but the new code produces raises AttributeError. Paste that input straight into a regression test.
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, 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: no capturable inputs from the test suite; property tier: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify prefix\_graph\_for\_global.
The verifier did not have enough to check prefix\_graph\_for\_global, 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: no capturable inputs from the test suite; property tier: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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: no capturable inputs from the test suite; property tier: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly IndexError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in to\_json (not a proof).
The verifier ran both versions of to\_json 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\_elixir.
The verifier did not have enough to check extract\_elixir, 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: no capturable inputs from the test suite; property tier: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_code\_span\_mention (not a proof).
The verifier ran both versions of \_code\_span\_mention 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\_markdown.
The verifier did not have enough to check extract\_markdown, 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: no capturable inputs from the test suite; property tier: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_rust.
The verifier did not have enough to check extract\_rust, 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: no capturable inputs from the test suite; property tier: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in resolve\_markdown\_mentions (not a proof).
The verifier ran both versions of resolve\_markdown\_mentions 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 resolve\_ruby\_member\_calls.
The verifier did not have enough to check resolve\_ruby\_member\_calls, 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: no capturable inputs from the test suite; property tier: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
Could not verify: Could not verify \_rebuild\_code.
The verifier did not have enough to check \_rebuild\_code, 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: no capturable inputs from the test suite; property tier: parameter `watch_path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_reconcile\_markdown\_links.
The verifier did not have enough to check \_reconcile\_markdown\_links, 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: no capturable inputs from the test suite; property tier: parameter `project_root` is annotated `Path` — outside the synthesizable primitive/collection set
· 50 more finding(s) on lines outside this diff (see the check run).
1 participant
Summary
build_from_json's bug: AST and semantic extractors generate different node IDs for the same symbol → god nodes duplicated, cluster cohesion degraded #1145 ghost-merge pass collapses non-AST duplicate nodes (e.g. a manifest-derived package node and its AST-canonical twin sharing the same(source_file, label)) into the canonical AST node. This is a legitimate, data-preserving node-count reduction.to_json's Prevent partial-chunk overwrite in --update: pre-build node-count assertion + build_merge() helper #479 shrink guard could not distinguish this from an unverified shrink, and refused to overwritegraph.jsoneven though nothing was actually lost.build_from_jsonnow records the collapsed-node count onG.graph["_ghost_dedup_count"].to_json's shrink guard treats a node-count drop as safe when it is fully explained by that count, and still refuses when any part of the drop is unexplained (regression-tested).Reproduction (evidence from downstream)
Reported downstream at ContextualWisdomLab/fast-mlsirm#1847: a persisted
graph.jsonhad 23,934 nodes;build_from_json(raw)on the identical extraction reloaded 23,933 NetworkX nodes (crate:mlsirm-coremerged into its AST twinpkg_mlsirm_core, zero incident edges dropped).cluster-only . --no-viz --no-labelon this unchanged graph refused the write:No data was actually lost — this was the intended
(source_file, label)canonicalization merge, not a shrink.Test plan
test_ghost_merge_records_dedup_count(build.py) asserting the merge count is recorded.test_to_json_allows_dedup_explained_shrinkandtest_to_json_still_refuses_shrink_beyond_dedup_count(export.py) — the latter guards that an unexplained loss on top of a dedup-explained one still refuses.pytest tests/test_build.py tests/test_export.py tests/test_manifest_ingest.py tests/test_build_merge_shrink_guard.py tests/test_hollow_chunks_arm_shrink_guard.py tests/test_incomplete_build_guard.py tests/test_unverified_semantic_shrink.py tests/test_no_dedup_flag.py tests/test_cli_export.py— 260 passed.🤖 Generated with Claude Code