feat(multigraph): add internal keyed MultiDiGraph build path#970
Open
hypnwtykvmpr wants to merge 21 commits into
Open
feat(multigraph): add internal keyed MultiDiGraph build path#970hypnwtykvmpr wants to merge 21 commits into
hypnwtykvmpr wants to merge 21 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces stronger security/robustness guards around loading and emitting graph data, and adds new infrastructure for multigraph support and deterministic symbol/call resolution, with extensive test coverage.
Changes:
- Enforce graph file size caps across multiple CLI/library entry points to prevent memory bombs before JSON parsing.
- Add multigraph-focused utilities (edge identity helpers, loader, capability probe, diagnostics, projections) and extend build to optionally produce keyed
MultiDiGraphs with collision diagnostics. - Add deterministic symbol resolution helpers (Python import-guided call resolver; Bash source-edge resolver) plus semantic-fragment validation/sanitization and SPDX/SCIP ingestion scaffolding.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_symbol_resolution.py | Adds comprehensive tests for symbol indexing/resolution and Bash source-edge resolution hardening. |
| tests/test_serve.py | Verifies serve._load_graph rejects oversized graph files. |
| tests/test_semantic_cleanup.py | Adds tests for semantic fragment validation + rationale/concept cleanup behavior. |
| tests/test_security.py | Adds tests for graph file size cap and recursive metadata sanitization. |
| tests/test_query_cli.py | Ensures query CLI refuses oversized graph files before parsing. |
| tests/test_projections.py | Adds tests for projection helpers and edge summarization with multigraph semantics. |
| tests/test_multigraph_diagnostics.py | Adds tests for multigraph collapse-risk diagnostics and CLI behaviors. |
| tests/test_multigraph_compat.py | Adds tests for runtime capability probing needed for keyed multigraph node-link round-trips. |
| tests/test_graph_loader.py | Adds tests for schema-aware loader (simple vs directed vs multigraph), key stripping, and repair behavior. |
| tests/test_global_graph.py | Ensures global graph workflows enforce the graph size cap. |
| tests/test_extract.py | Adds regression tests for Bash extractor edge cases + metadata sanitization. |
| tests/test_export.py | Verifies pinned vis-network URL with SRI/crossorigin in HTML exports. |
| tests/test_edge_identity.py | Adds tests for stable edge key helpers and schema key stripping. |
| tests/test_detect.py | Adds extensive tests for shebang interpreter parsing and classification. |
| tests/test_callflow_html.py | Ensures callflow HTML loader rejects oversized graph files. |
| tests/test_build.py | Extends build tests for multigraph behavior and size-cap enforcement in merge paths. |
| tests/test_benchmark.py | Ensures benchmark runner enforces graph size cap before parsing. |
| tests/conftest.py | Adds targeted warning filters for a specific test module. |
| pyproject.toml | Adds dev dependency group and basic Ruff/Pyright configuration. |
| graphify/watch.py | Enforces size cap before reading existing graph JSON during watch rebuilds. |
| graphify/tree_html.py | Enforces size cap before loading graph JSON for tree HTML output. |
| graphify/symbol_resolution.py | Introduces deterministic symbol resolution + Bash source-edge resolver utilities. |
| graphify/skill-opencode.md | Updates skill instructions and adds validated/sanitized semantic chunk merge steps. |
| graphify/skill-codex.md | Updates skill instructions and adds validated/sanitized semantic chunk merge steps. |
| graphify/serve.py | Enforces size cap in server-side graph loading. |
| graphify/semantic_cleanup.py | Adds validation and sanitizer for untrusted semantic fragments and rationale cleanup. |
| graphify/security.py | Adds graph file size cap helper and recursive bounded metadata sanitization. |
| graphify/scip_ingest.py | Adds SCIP-style JSON ingestion skeleton with defensive parsing and sanitization. |
| graphify/prs.py | Adds size cap enforcement when reading saved graphs and tolerates cap errors. |
| graphify/projections.py | Adds projection helpers for consumers needing explicit edge semantics. |
| graphify/multigraph_compat.py | Adds a behavior-based runtime capability probe for multigraph keyed node-link support. |
| graphify/graph_loader.py | Adds schema-aware graph loader for simple/directed/multigraph node-link JSON. |
| graphify/global_graph.py | Enforces size cap when reading/writing global graphs and source graphs. |
| graphify/extract.py | Hardens Bash extraction (entrypoint node, nested function calls, expansion filtering, metadata sanitization). |
| graphify/export.py | Enforces size cap on existing graph comparisons and pins vis-network with SRI. |
| graphify/edge_identity.py | Introduces stable edge key generation and schema key stripping helper. |
| graphify/diagnostics.py | Adds diagnostics for same-endpoint edge collapse risk and suppression-site scanning. |
| graphify/detect.py | Implements robust shebang interpreter parsing including complex env forms. |
| graphify/callflow_html.py | Enforces size cap before reading graph JSON and translates cap errors to CLI exits. |
| graphify/build.py | Adds optional multigraph build mode with stable keys, collision repair, and diagnostics. |
| graphify/benchmark.py | Enforces size cap before loading graph JSON for benchmarking. |
| graphify/main.py | Adds CLI helper to enforce size cap pre-parse and introduces diagnose multigraph subcommand. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
23fc4a7 to
2493d37
Compare
4fcd6d0 to
5d35dc8
Compare
…ntity + internal keyed build path
Internal, opt-in MultiDiGraph foundation. No default user-visible behavior changes.
Part 1: Projection API (graphify/projections.py)
Explicit projection helpers so consumers can declare what graph semantics they want and
what they intentionally lose:
- project_for_community (configurable weight mode: confidence or count)
- project_for_path (simple undirected topology for shortest-path)
- project_for_callflow (directed projection with optional relation filter)
- project_for_context (filter by context value)
- edge_records_between (raw edge record iteration)
- edge_summary_between (bundle/summary formatting)
- distinct_neighbor_degree (for god-node and hub thresholds)
- normalize_to_multidigraph (simple-to-multidigraph lift)
Tests in tests/test_projections.py cover MultiDiGraph fixtures plus property-style
invariants (bundle counts equal total edge records; weighted projection weight equals
multiplicity).
Part 2: Schema-aware loader + stable edge identity
Centralizes graph loading and reserves the edge "key" field as schema, not as an ordinary
attribute.
- graphify/edge_identity.py: SCHEMA_KEY_FIELD constant,
make_stable_key(relation, source_file, source_location), strip_schema_key(attrs)
- graphify/graph_loader.py: load_graph and load_graph_file handling
legacy simple JSON with "links",
legacy simple JSON with "edges",
valid multigraph node-link JSON (multigraph: true) with keyed parallel edges,
malformed multigraph JSON with missing/non-string keys (deterministic repair via
full-attribute payload hash, never silent downgrade),
conflicting schema markers.
- Profile metadata stored in G.graph["graphify_profile"] (preserved by node-link
serialization).
- Multigraph loads gated behind require_multigraph_capabilities() from PR safishamsi#956.
Tests in tests/test_graph_loader.py and tests/test_edge_identity.py cover all seven
loader scenarios plus the schema-key reservation contract.
Part 3: Internal keyed MultiDiGraph build path
Opt-in nx.MultiDiGraph build support in build_from_json and build.
- multigraph: bool = False parameter
- Stable edge keys generated after dedup/remap and source normalization.
- Serialized edge attrs cannot pass duplicate key= kwargs into G.add_edge.
- Exact duplicates collapse only with diagnostics; non-exact key collisions fire
deterministic bounded repair (full-payload hash, not identity-field-only).
- Node-link JSON written with explicit edges="links" compatibility.
- Default simple-graph output is unchanged.
Adversarial-input resilience (verified against malformed extraction inputs):
- Hashable non-string node IDs and edge endpoints are preserved.
- Unhashable node IDs and endpoints do not crash validation or build.
- Non-dict node entries and nodes missing "id" are skipped safely after validation
warns.
- Non-dict edge entries are skipped safely after validation warns.
- Explicit empty-string schema keys are preserved.
- Collision-repair keys are deterministic and do not overwrite explicit keys.
- Exact duplicate detection remains O(n) within a (source, target, key) group.
Out of scope:
- No public --multigraph CLI flag (planned for a later slice; only programmatic
activation here).
- No watch/cache/global-graph/MCP/export surface changes.
- No producer widening.
- No dedup/remap MultiDiGraph contract changes (separate concern, separate review).
Test coverage:
pytest tests/test_projections.py tests/test_graph_loader.py tests/test_edge_identity.py \
tests/test_multigraph_diagnostics.py tests/test_build.py tests/test_validate.py
→ 130 passed.
This is a collapse of an earlier 12-commit stack on wave3-pr3-internal-build into a
single commit so that every commit in origin history passes Copilot review individually.
The pre-collapse stack is preserved as the tag
archive/2026-05-22-wave3-pr3-internal-build.
Rebased onto upstream/v8 (740382a). Conflict in graphify/extract.py resolved preserving both upstream (TypeScript abstract class, C# base-list, Java interface inheritance, defusedxml) and local multigraph behavior. Full-repo ruff/pyright/security pass: 0 errors, 0 warnings, all .AUDIT gates clean. Added --no-viz support to graphify update. Raised AUDIT_COPILOT_MAX_DIFF_BYTES default from 120KB to 2MB. Updated AGENTS.md (no-waiver rule, conflict rule, removed stale memory block) and added CLAUDE.md with durable project policy. 1507 passed, ruff clean, pyright clean. gost
Make entity dedup/remap safe under MultiDiGraph keyed parallel edges: - deduplicate_entities() tracks self-loop drops and exact-duplicate collapses per relation/source via new diagnostics dict parameter - DEC-013: preserve real pre-existing self-loops; drop only remap-induced artifacts - remove_all_parallel_edges() helper avoids the NetworkX two-tuple removal trap (remove_edges_from removes only one edge per 2-tuple) - Diagnostics wired through build() only for multigraph=True builds so simple-graph output stays byte-identical - 18 new tests in test_dedup_remap.py; 105 targeted pass, 1507 full gost
Route clustering, cohesion, god-node, and report logic through projections
so parallel edges do not inflate metrics or crash algorithms:
- analyze.py: edge betweenness uses project_for_community() on multigraphs
to avoid 3-tuple key crash; god_nodes/surprise scoring uses
distinct_neighbor_degree() instead of G.degree() to prevent inflation
- cluster.py: Louvain/Leiden receives projected simple graph instead of
raw MultiDiGraph; cohesion_score() projects before counting edges so
result stays in [0.0, 1.0]
- report.py: edge counts distinguish total records from unique pairs on
multigraphs ("8 edges (3 unique pairs)")
12 new tests: betweenness crash, degree inflation, cohesion bound,
community detection, edge count semantics. 1515 full suite pass.
gost
…elationships User-facing read surfaces no longer silently show only the first parallel edge on a MultiDiGraph. Adds a stable, capped relationship envelope routed through explicit projections (preserving the analytics/report projection boundary from PR 4B): - projections.py: relationship_envelope() (structured bundle for MCP) and format_relationship_envelope() (capped text "a, b, c (+K more, N total)") with DEFAULT_RELATIONSHIP_CAP=3. directed_only flag isolates a single direction for arrow rendering. - serve.py: _subgraph_to_text, _neighbors_text, _shortest_path_text bundle all parallel relations per hop/neighbor (directed_only=True) instead of edge_data() first-edge-only. Historical single-relation format preserved byte-for-byte across all three; envelope bundle only for multi-relation. - __main__.py: path and explain handlers render directional bundles via directed_only=True so reverse-edge relations never bleed onto an arrow. edge_data() in build.py kept unchanged for non-display callers. Simple-graph output stays byte-stable (pinned by format-regression tests). 32 new tests incl. directional-isolation, capped-summary, and exact-format-pin regressions. Full suite 1624 passed. gost
test_ollama and test_incremental assumed a clean environment but did not clear all backend env vars detect_backend() checks (DEEPSEEK_API_KEY, GEMINI/OPENAI/AWS), so they failed in any shell exporting one. No-waiver fix makes them hermetic: - test_ollama.py: autouse fixture strips every backend key (derived from BACKENDS + _backend_env_keys) plus AWS/OLLAMA before each test. - test_incremental.py: _run passes a sanitized env to the extract subprocess so the "no LLM API key" path triggers regardless of ambient keys. Verified green with DEEPSEEK_API_KEY set. Full suite 1624 passed. gost
The upstream/v8 rebase resolved test_languages.py conflicts by taking upstream's behavioral assertions (safishamsi#1077 markdown code-block drop), which reintroduced the ambiguous `l` loop variable (E741) and a combined `import tempfile, os` (E401) that our earlier no-waiver cleanup had fixed. Re-apply lint-clean style on top of upstream's behavior: l -> label, split imports. Ruff E4/E7/E9/F clean; test_languages 218 passed. gost
…l edges External outputs no longer silently collapse MultiDiGraph parallel edges. Per the gate, each surface either preserves every parallel edge or documents and tests an intentional summarization: PRESERVE-ALL (structural): - to_json / to_graphml: native multi-edge support (round-trip tests). Fixed to_graphml crash: strip non-scalar graph-level attrs before write_graphml. - to_cypher: each parallel edge gets a distinct edge_key (positional key, int or str, unique per (u,v) pair) so Neo4j MERGE never dedups parallels — even when relation/source_file/source_location are identical. - to_canvas: unique keyed edge IDs (e_u_v_idx) replace colliding endpoint-only IDs. Visual cap is additive: real edges fill the 200-cap first by weight, then overflow-summary edges append only for surviving pairs (never evict a real edge). INTENTIONAL-SUMMARIZE (display, documented): - to_obsidian / wiki: bundle all parallel relations per neighbor instead of edge_data() first-edge-only. - to_html / to_svg: cap parallel edges drawn per pair + summary label. report.py verified already-preserving (no change; regression test added). Simple-graph output byte-stable (pinned). 19 new tests incl. integer-key collision and >200-edge canvas-cap regressions. Full suite 1658 passed; ruff + pyright clean. gost
Codex verification found that PR 6 canvas ids could still collide when node ids contained underscores, for example a_b->c and a->b_c. Add a deterministic digest fallback while preserving readable ids when unique. Also isolate report-generation tests from optional clustering imports, apply ruff formatting across the rebased stack, and refresh the lockfile package version to match pyproject.
…parallel edges Stateful per-repo update is now MultiDiGraph-safe with no silent fallback to simple-graph behavior (the PR 7 go/no-go gate): - build.py build_merge: removed the multigraph NotImplementedError rejection; inherits the saved multigraph flag; prune is now key + source_file aware (removes only the parallel edge records whose source_file is evicted, keeping same-pair parallels from other files). Simple path byte-stable. - export.py to_json: persists graphify_profile (graph_type) in graph.json so the profile round-trips through save/load (only added "graph" metadata). - watch.py: _rebuild_code inherits the saved multigraph profile on both the clustered and --no-cluster paths (build_from_json(multigraph=...) + profile re-stamp) so a rewrite keeps multigraph=true and reloads as a MultiDiGraph (fixes a deferred collapse: edges survived but the dropped flag collapsed them on next load). Canonical comparison strips graph-level metadata and is key-aware (parallels not treated as duplicates). Changed/deleted-file eviction is source_file + key aware (evicts stale cross-file edges between surviving nodes). - cache.py: confirmed raw extraction is profile-independent; added CACHE_SCHEMA_VERSION invalidation guard (version mismatch / legacy unversioned entries -> safe rebuild) without per-profile key fragmentation. - __main__.py update: now PRESERVES a multidigraph (delegates to the profile-inheriting _rebuild_code) instead of refusing/collapsing. 26 new tests across realistic temp-repo scenarios (unchanged-file parallels persist; changed-file evicts only its parallels; deleted-file removes all its records; profile preserved through rewrite + reload; simple mode unchanged). Full suite 1683 passed; ruff + pyright clean. gost
…allel edges Cross-repo global composition and merge are now MultiDiGraph-safe with explicit recovery + backup policy. Gate met: mixed simple/multi inputs never crash through NetworkX class mismatch and never silently collapse a multigraph without an explicit simple target. global_graph.py: - global_add composes key-aware after normalizing existing + incoming to an inferred target class — no silent collapse; re-adding the same repo is idempotent (no edge drift). - Reusable helpers: normalize_graphs_for_global (infers target; warns+projects only on explicit simple target over multi input), detect_pre_profile, refuse_pre_profile_upgrade, backup_global_graph. - Stores graphify_profile; pre-profile files refuse silent multidigraph "upgrade" (unreconstructable parallels) with a recovery message; dated backup before any overwrite. __main__.py merge-driver / merge-graphs: - Consume the same helpers: normalize-then-keyed-compose (no nx.compose class mismatch crash), backup before write, pre-profile refusal. merge-graphs gains --multigraph/--simple flags; default infers (no silent collapse). - merge-driver pre-profile refusal scoped to the OVERWRITTEN current file only (other is read-only — never blocks a valid merge). Verification (PR 7 lesson): every merge/recovery path tested under REPEATED application (global_add + both merge commands 3x assert stable records/keys/ counts/profile). Independent coordinator probe confirmed global_add 3x keeps parallels with no drift. 23 new tests incl. idempotence-under-repeat, mixed-input no-crash, pre-profile refusal scope (both directions), backup + NO_BACKUP, simple-only regression. Full suite 1707 passed; ruff + pyright clean. gost
Exposes the MultiDiGraph feature publicly now that every stateful path maintains it safely (PR 1-8). End-to-end public workflow is green. graphify extract: - New --multigraph / --simple flags (mutually exclusive). --multigraph builds a keyed MultiDiGraph preserving parallel edges; capability is gated by require_multigraph_capabilities() surfaced as a clean CLI error (no traceback). - STICKY profile: with NO flag, extract/update inherit the existing graph.json profile (via watch._existing_is_multigraph), so a graph built --multigraph stays multigraph across later commands without restating the flag. - --simple is the explicit downgrade; over an existing multigraph it WARNS before collapsing parallel edges (never silent). - Help text lists the flags + the sticky default. Docs (behavior is now real): README + skill.md + skill-windows.md document --multigraph / --simple and the sticky behavior. Tests: full CLI lifecycle — simple default, --multigraph (real subprocess), sticky stays-multigraph across repeated default extract+update (idempotence- under-repeat), explicit --simple lossy-downgrade warning, capability-failure message, and a multigraph extract->query parallel-relationship roundtrip. Full suite 1714 passed; ruff + pyright clean. No private testing-tool refs. gost
Normalize legacy edges-keyed graph JSON during watch/update comparisons and add a zero-node overwrite floor across export, watch, and no-cluster extract write paths. Preserve upstream provider registry behavior while clearing post-rebase lint/type/security gates.
395b392 to
526f848
Compare
c136e5d to
322c074
Compare
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
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.
Summary
Stacked on PR #968. Implements PR 3 of the MultiDiGraph rollout: the internal write path for building keyed
nx.MultiDiGraphinstances from extraction dictionaries.graphify/build.py:build_from_json()andbuild()now acceptmultigraph: bool = False. Whenmultigraph=True, Graphify requires the runtime multigraph capability probe, builds annx.MultiDiGraph, assigns deterministic string edge keys, preserves same-endpoint parallel edges, and records duplicate/collision diagnostics. Default simple-graph behavior is unchanged.graphify/build.py/graphify/validate.py: hashable non-string node IDs are preserved, while malformed nodes/edges and unhashable node IDs or endpoints produce explicit validation/build warnings and are skipped safely.tests/test_build.py,tests/test_validate.py, andtests/test_multigraph_diagnostics.py: add regression coverage for multigraph parallel-edge preservation, duplicate diagnostics, JSON round-trip behavior, merge behavior, empty string schema keys, deterministic collision repair, explicit-key conflict handling, non-string hashable endpoints, unhashable node IDs, unhashable endpoint handling, malformed node/edge entries, and diagnostics no-crash behavior.Test plan
uv run pytest tests/test_build.py tests/test_validate.py tests/test_multigraph_diagnostics.py tests/test_graph_loader.py tests/test_edge_identity.py tests/test_multigraph_compat.py -q— 122 passedAUDIT_BASE_REF=origin/wave3-pr3-internal-build AUDIT_MAX_LINES=5000 .AUDIT/pre-commit-all.sh --strict-fast— clean, 1287 passed in test-quick.AUDIT/test-ci.sh— clean, 1287 passed + graphify help/install smoke