fix(build): sweep external-import nodes a prune leaves orphaned (#2807) - #2809
fix(build): sweep external-import nodes a prune leaves orphaned (#2807)#2809abhay-codes07 wants to merge 1 commit into
Conversation
…hify-Labs#2807) prune_sources matches nodes on source_file. Extractors create a per-file node for each IMPORTED EXTERNAL symbol -- `Path` from pathlib, `Counter` from collections, `Graph` from networkx -- and those carry no source_file, because they are defined outside the corpus. Every edge they have points at symbols in the one file they were created for, so pruning that file left them at degree 0. Nothing could ever collect them. Deletions go through deleted_files, exclusions through excluded_files (Graphify-Labs#1908) and _stale_graph_sources (Graphify-Labs#1909), and all three match on source_file. A node without one is outside all of them, so the residue is permanent and grows with every prune. They are not inert while they sit there: GRAPH_REPORT.md opens with a node count read off the graph, to_obsidian writes one note per node, and the wiki and graph.json carry them too -- so a vault keeps a `Path.md` for a file the corpus no longer contains. On graphify's own package, pruning graphify/callflow_html.py removed 137 of its 139 nodes and stranded graphify_callflow_html_py_path (label `Path`) and graphify_callflow_html_py_counter (label `Counter`). With this change all 139 go and no orphan is left behind. The sweep is scoped three ways so it cannot become a general garbage collector: - only nodes with NO source_file (one that has a real source_file is prunable through the normal path and must not be second-guessed here); - only nodes THIS prune isolated (a source-less node already isolated beforehand is a different question and survives -- graphify's own graph has one); - only inside the prune branch, so a plain merge never touches isolated nodes. A source-less node with no edges names nothing and connects nothing, so dropping it loses no information. An isolated node that DOES have a source_file is left alone even though it looks equally lonely -- b.py's symbol whose only call went into a pruned a.py is still a real symbol in a file that still exists, and there is a test pinning that distinction.
![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
Adds an orphan sweep to build_merge's prune_sources branch that removes source-less nodes left at degree 0 by the prune (external-import nodes like Path/Counter), while snapshotting _isolated_before so nodes already isolated pre-prune are spared and counting swept nodes into n_nodes. Adds tests/test_prune_sweeps_orphans.py covering the strand-with-file case, shared/still-referenced externals, pre-existing isolates, source-ful isolates, no-prune and no-match cases.
Worth a look
- Prune now deletes arbitrary sourceless nodes it orphans —
graphify/build.py:1815· 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.
- Orphan sweep ignores live hyperedge references —
graphify/build.py:1815· 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 — 1009 functions depend on the 101 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 95 callers, 51 callees - new:
build_from_json()— 156 callers, 18 callees - new:
build_merge()— 44 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 - new:
extract_corpus_parallel()— 26 callers, 10 callees - …and 38 more — each is listed as a finding
Verification — 1009 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: 653 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
· 1 grounded finding(s) anchored inline below; 45 more finding(s) on lines outside this diff (see the check run).
| return build_from_json(_extraction(nodes, edges)) | ||
|
|
||
|
|
||
| def _prune(G, tmp_path, sources): |
There was a problem hiding this comment.
_prune()
7 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Fixes #2807.
The bug
prune_sourcesmatches nodes onsource_file. Extractors create a per-file node for each imported external symbol —Pathfrom pathlib,Counterfrom collections,Graphfrom networkx — and those carry nosource_file, because they are defined outside the corpus. Every edge they have points at symbols in the one file they were created for.So pruning that file leaves them at degree 0, and nothing can ever collect them: deletions go through
deleted_files, exclusions throughexcluded_files(#1908) and_stale_graph_sources(#1909), and all three match onsource_file. A node without one is outside every path graphify has for removing stale content, so the residue is permanent and grows with each prune.On graphify's own package:
137 of the file's 139 nodes go; 2 stay forever, and the prune line reports
137 node(s)and looks complete. With this change all 139 go.They are not inert while they sit there.
GRAPH_REPORT.mdopens with a node count read straight off the graph,to_obsidianwrites one note per node, and the wiki andgraph.jsoncarry them too — so a vault keeps aPath.mdfor a file the corpus no longer contains.Scope
The sweep is narrowed three ways, so it cannot quietly become a general garbage collector:
source_file. One that has a realsource_fileis prunable through the normal path and must not be second-guessed here.The distinction I want to flag for review, because it is the one that looks inconsistent at a glance: an isolated node that does have a
source_fileis deliberately kept. In the test fixture,b.py's symbol whose only call went into a pruneda.pyends up at degree 0 — but it is a real symbol in a file that still exists, so pruning it would be data loss, and it stays prunable through the normal path ifb.pyever goes. There is a test pinning that asymmetry so it reads as intentional rather than as a missed case.A source-less node with no edges names nothing and connects nothing, so dropping it loses no information; keeping it inflates every count that reads the graph.
Tests
tests/test_prune_sweeps_orphans.py(8 tests). Three fail without the fix — the external-import node going with its file, no source-less orphan remaining, and pruning every file actually emptying the graph.The other five exist to stop the sweep over-reaching: a shared external node still referenced by a surviving file must live, a pre-existing isolate must live, an isolated node with a
source_filemust live, a merge with no prune must change nothing, and a prune that matches nothing must sweep nothing.One of those caught me while writing this: my first version asserted "no orphans remain at all" and failed, correctly, because
mod_b_keepis isolated but real. The assertion was wrong, not the fix — I narrowed it to source-less orphans and added the surviving-real-symbol case as an explicit test rather than loosening it silently.Validation
Windows 11, Python 3.12, branched off
4fca621(0.9.44).20 failed, 4469 passed->21 failed, 4476 passed. The +8 are the new tests.test_incremental_mtime_collision.py::test_same_size_rewrite_in_one_tick_is_requeued, which this PR does not touch: it depends on two writes landing in the same filesystem timestamp tick, and this run took 10:58 against the usual ~6:30, so the machine was loaded. It passes in isolation, and it has surfaced the same way on two unrelated branches of mine. Flagging it rather than quietly reporting 20.