Repository navigation
Conversation
Pass dedup=False into graph construction so repeated labels do not collapse distinct non-AST identities. Preserve existing AST and document-file twin reconciliation. Fixes Graphify-Labs#4019. Assisted-by: OpenAI Codex (GPT-6)
|
Thanks for the pull request, @xiehuanyi. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![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. PR-changed functions: 0/2 verified (0 proven, 0 may-equivalent, 0 distinguished) · 2 not verified (2 vacuous).
Not verified on this run: build (vacuous: never exercised), build\_from\_json (vacuous: never exercised).
Graphify review — findings
Makes --no-dedup (and dedup=False) also stop build_from_json from collapsing distinct non-AST nodes just because they share a source file and label. Separate same-named entries at different locations in one file now keep their own IDs and edges. Semantic nodes still fold into a matching AST node, document-file twin reconciliation still runs, and build now passes its dedup flag through so full builds and merges behave the same way.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1371 functions depend on the 123 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 149 callers, 56 callees - new:
build_from_json()— 231 callers, 20 callees - new:
build_merge()— 85 callers, 14 callees - new:
to_obsidian()— 41 callers, 14 callees - new:
to_json()— 61 callers, 8 callees - new:
to_wiki()— 45 callers, 8 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
build()— 56 callers, 6 callees - …and 46 more — each is listed as a finding
Verification — 1371 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: 940 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
330 of 330 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— impact, full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— impact, full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— impact, full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— impact, full-run-safetytests/test_benchmark_raw_graph.py— impact, full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_located_semantic_identity.py— impact, changed-test, full-run-safetytests/test_build_merge_dedup_scope.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, full-run-safetytests/test_build_merge_shrink_guard.py— impact, full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— impact, full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— impact, full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— impact, full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— impact, full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— impact, full-run-safetytests/test_confidence.py— impact, full-run-safetytests/test_corrupt_graph_json.py— impact, full-run-safetytests/test_cpp_method_declarations.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safety- … and 280 more
non-code file(s) changed (
README.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
README.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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.
Docs that may be stale (advisory)
ARCHITECTURE.md§ Module responsibilities (lines 13-35): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.34 (2026-08-05) (lines 547-555): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.27 (2026-07-26) (lines 619-639): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.23 (2026-07-21) (lines 669-678): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.21 (2026-07-20) (lines 689-701): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.17 (2026-07-16) (lines 734-763): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.13 (2026-07-12) (lines 824-843): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.12 (2026-07-10) (lines 844-863): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.3 (2026-06-30) (lines 998-1009): references changed symbolsbuild_from_jsonCHANGELOG.md§ 0.9.0 (2026-06-28) (lines 1036-1040): references changed symbolsbuild_from_json
…and 10 more.
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: no capturable inputs from the test suite; property tier: 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 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)
· 1 grounded finding(s) anchored inline below; 53 more finding(s) on lines outside this diff (see the check run).
|
|
||
|
|
||
| def build_from_json(extraction: dict, *, directed: bool = False, root: str | Path | None = None) -> nx.Graph: | ||
| def build_from_json(extraction: dict, *, directed: bool = False, root: str | Path | None = None, |
There was a problem hiding this comment.
build_from_json()
fans out to 20 callees (efferent coupling); 231 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Landed in v0.9.77 via an authorship-preserving cherry-pick, so your commit is on |
With
dedup=False, repeated JSON keys or document headings in one file still collapsed insidebuild_from_json: the issue fixture went from five nodes to three. An incremental merge could consequently trigger the untouched-source shrink guard even though entity deduplication was disabled.Pass the existing
build(..., dedup=False)flag through to graph construction and add the same explicit option tobuild_from_json. The same-file/same-label ghost pass now leaves distinct non-AST IDs intact when opted out. Default behavior, AST/semantic reconciliation, document-file twin reconciliation, and exact-ID canonicalization remain unchanged. Document the Python equivalents of--no-dedup.Closes #4019.
Validation:
v8, the high-levelbuild(..., dedup=False)and freshbuild_merge(..., dedup=False)regressions both fail with5 -> 3; both pass after the fix.Commands (using the shared development virtualenv):
python -m pytest -q,python -m pytest tests/test_build_located_semantic_identity.py tests/test_build.py tests/test_build_merge_dedup_scope.py tests/test_no_dedup_flag.py -q,ruff check .,pyright --pythonpath <dev-venv>/bin/python, andPYTHONPATH=$PWD python -m graphify update ..Assisted by OpenAI Codex (GPT-6).