prep + fixes: shared graph-json loader, --graph=PATH parsing, watch root, hyperedge pruning, weight validation - #3022
Open
StuartMVG wants to merge 4 commits into
Open
prep + fixes: shared graph-json loader, --graph=PATH parsing, watch root, hyperedge pruning, weight validation#3022StuartMVG wants to merge 4 commits into
StuartMVG 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.
StuartMVG
marked this pull request as ready for review
August 24, 2026 17:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.