prep + fixes: shared graph-json loader, --graph=PATH parsing, watch root, hyperedge pruning, weight validation - #3022
prep + fixes: shared graph-json loader, --graph=PATH parsing, watch root, hyperedge pruning, weight validation#3022StuartMVG wants to merge 4 commits into
Conversation
…s multi-repo paths merge-graphs and the global graph each carried their own copy of "load node-link JSON, normalize the legacy edges key, stash direction markers" — and each copy had a different subset of the Graphify-Labs#738/Graphify-Labs#2261/Graphify-Labs#2309/Graphify-Labs#2484 fixes. build.py gains two shared helpers: - load_graph_json(path, *, preserve_type, directed, preserve_direction): applies the graph-file size cap, normalizes the legacy "edges" spelling (Graphify-Labs#738), validates structure (malformed input becomes a single actionable ValueError instead of a NetworkX traceback), preserves stored direction via _src/_tgt without clobbering pre-existing markers (Graphify-Labs#2261, Graphify-Labs#2309), and restores top-level-only hyperedges (Graphify-Labs#2484/Graphify-Labs#2485). Directed/multi inputs are coerced to a plain Graph by default so nx.compose never sees mixed types (Graphify-Labs#1606); preserve_type opts out. - merge_prefixed_into(G, prefixed): the external-library dedup-by-label merge lifted verbatim from global_add, with incident edges rewired onto the shared node and remap-introduced self-loops skipped. global_graph.py shrinks onto both helpers; cli.py merge-graphs shrinks onto load_graph_json(preserve_direction=True), now rejecting a malformed input graph with exit 1. The Graphify-Labs#3007 shared-type hook and the compose/hyperedge handling are untouched. New direct tests in tests/test_load_graph_json.py pin the loader contract: every validation branch, both key spellings, the direction modes, marker preservation, hyperedge restoration, type coercion, the size cap, and the external-dedup merge.
…less --graph query, path, explain, and affected each carried their own copy of the --graph option loop, in two different parse styles, and only affected ever handled the --graph=PATH form. On the other three the token fell through the loop's else arm and was silently dropped: the explicitly selected graph was ignored and the command ran against the default graph with no warning. A valueless --graph (trailing, or the empty --graph=) was silently dropped by all four — the same silent-selection-loss class. It now exits 2 with "error: --graph requires a path". The empty form cannot fall through to the file check instead: an empty path resolves to the cwd, a directory, and only query/affected guard that with a .json suffix check — path and explain would crash reading it. One shared pre-pass (_parse_graph_option) replaces the four inline loops. It strips only the graph tokens and returns the rest in order, so each command's own flag loop (--budget/--context, --depth/--relation, --directed/--undirected) is unchanged. The returned graph_given flag is unused today but part of the contract for a future option that needs mutual-exclusion against an explicit --graph. tests/test_graph_option_cli.py pins the contract across all four commands: the space form (characterization — passed before this change), the = form, the valueless error, and that per-command flags still parse after the pre-pass.
… weights Three independent small fixes: - watch.py: the update-path rebuild called build_from_json without root, so absolute source_file paths from semantic fragments were persisted machine-absolute where `graphify build` writes root-relative ones (Graphify-Labs#932). Pass project_root through, aligning the two build paths. - cli.py: extract _filter_payload_sources from _prune_graph_json_sources and, while there, drop hyperedges whose MEMBERS were removed, not only those whose own source_file is stale. Previously a hyperedge could outlive every node it references, persisting dangling member ids in graph.json. - cluster.py: _partition passed edge attrs to Leiden/Louvain unvalidated, so a non-numeric, NaN, infinite, or negative weight in a hand-edited or LLM-produced graph.json reached the partitioner as-is. Repair to 1.0 — the same `not isfinite or < 0` contract build_from_json already enforces when normalizing edge attrs (a path graphs loaded from disk bypass). Each fix carries a pinning test: the watch test fails without the root argument (verified by reverting), the hyperedge test exercises the new member check, and the weight tests cover all four invalid classes plus a valid weight passing through unchanged.
3ef1005 to
f03ff1b
Compare
![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
Honors --graph=PATH on query/path/explain by routing all four query surfaces through a shared _parse_graph_option, which also makes a valueless --graph or empty --graph= exit 2 with an actionable message instead of silently falling back to the default graph. Threads the project root through graphify watch/update rebuilds so absolute source_file paths from semantic fragments get relativized like graphify build, and prunes hyperedges that reference a removed node rather than only those owned by the stale file. Repairs non-numeric, NaN, infinite, or negative edge weight values to 1.0 before community detection so hand-edited or LLM-produced graph.json files match the builder's normalization.
Worth a look
- load_graph_json preserve_direction stashes _src/_tgt but does not restore endpoint order —
graphify/build.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- --graph consumes following flag token as its path value —
graphify/cli.py:111· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Pre-stripping --graph masks a missing --budget value —
graphify/cli.py:1303· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Concurrent global_add can lose graph updates —
graphify/global_graph.py:110· 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 — 1871 functions depend on the 874 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 114 callers, 50 callees - new:
build_from_json()— 192 callers, 18 callees - new:
build_merge()— 62 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 43 more — each is listed as a finding
Verification — 1871 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: 1664 function(s) in the blast radius were not formally verified this run
Formal verification
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: 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 \_prune\_graph\_json\_sources.
The verifier did not have enough to check \_prune\_graph\_json\_sources, 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 `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_partition (not a proof).
The verifier ran both versions of \_partition 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 global\_add.
The verifier did not have enough to check global\_add, 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 `source_path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_load\_global\_graph.
The verifier did not have enough to check \_load\_global\_graph, 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 1 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be 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: parameter `watch_path` is annotated `Path` — outside the synthesizable primitive/collection set
· 51 more finding(s) on lines outside this diff (see the check run).
1 participant
First extracted PR from the #2134 split: the Track A prep refactor and standalone correctness fixes. It deliberately introduces no cluster spec or commands and no user-facing multigraph mode.
Scope: four focused, independently reviewable commits across 11 files. Of 813 added lines, 482 are regression tests.
Why
merge-graphsand the global graph had three local copies of "load node-link JSON, normalize the legacyedgeskey, preserve direction." Those copies had drifted across the fixes for #738, #2261, #2309, and #2484. The duplicated CLI parsers had drifted too: onlyaffectedrecognized--graph=PATH.This PR consolidates those contracts and separates several standalone fixes from the cluster and multigraph feature work in #2134.
What changed
1. Shared graph-json loader
build.pygains two shared helpers, and the existing global-graph andmerge-graphscallers shrink onto them:load_graph_json(path, *, preserve_type, directed, preserve_direction)applies the graph-file size cap, normalizes legacyedgestolinks, validates malformed structure with an actionableValueError, preserves_src/_tgtmarkers without clobbering existing direction, restores top-level-only hyperedges, and defaults to a plainGraphso mixed graph types cannot breaknx.compose(merge-graphs: unhandled NetworkXError on mixed directed/multigraph inputs, and exit code 0 on crash #1606).merge_prefixed_into(G, prefixed)centralizes the external-library dedup-by-label merge previously embedded inglobal_add.The existing #3007 shared-type hook and the compose/hyperedge behavior in
merge-graphsare unchanged. Direct loader tests now cover malformed payloads, legacy keys, direction preservation, hyperedge restoration, simple-graph coercion, and keyed parallel edges in bothMultiGraphandMultiDiGraph.2. Consistent
--graphparsingquery,path, andexplainsilently ignored--graph=PATHand queried the default graph; onlyaffectedparsed that form.A shared
_parse_graph_optionpre-pass now supports both--graph PATHand--graph=PATHacross all four commands. A trailing--graphwas previously ignored by all four commands. Empty--graph=was ignored by three and reached a less-useful file-type error inaffected. Both valueless forms now exit 2 witherror: --graph requires a path.Each command still parses its own flags afterward. The existing space-separated behavior is pinned with characterization tests alongside the new equals-form and error cases.
3. Standalone correctness fixes
project_rootintobuild_from_json, so absolute semanticsource_filevalues are normalized the same way as a direct build.hyperedgesand nestedgraph.hyperedges, are filtered, and a nested-only change bypasses the no-change write guard._partitionrepairs non-numeric, NaN, infinite, or negative weights to1.0before Leiden/Louvain sees them. This matches the existingbuild_from_jsoncontract for graphs that passed through the builder.Compatibility and non-goals
--clusterselection lands here.--multigraphor--no-multigraphCLI surface lands here.edgespayloads remain supported.Local verification
3ef1005: 5,088 passed, 11 skipped, 1 known pre-existing order-sensitive failure.test_label_communities_batches_when_over_batch_sizereceived the same batch sizes in a different order ([100, 50, 100]instead of[100, 100, 50]); neither that test nor its implementation differs from the base branch.uv run --frozen python -m tools.skillgen --check: 134 artifacts clean.git diff --check: clean.--graphforms and exit-2 cases across all four commands;merge-graphsdirection and hyperedge preservation;global adddedup and skip-unchanged behavior.Follow-up slices
C1, the cluster-spec and member-resolution slice, is independent of this loader. B1, the multigraph build core, consumes it. C2 will later combine C1 with this loader for member composition; B1 is additionally required for C2's multi mode.