diff --git a/docs/superpowers/specs/2026-04-16-v5-rustworkx-github-design.md b/docs/superpowers/specs/2026-04-16-v5-rustworkx-github-design.md index 647eb82a..360eb67e 100644 --- a/docs/superpowers/specs/2026-04-16-v5-rustworkx-github-design.md +++ b/docs/superpowers/specs/2026-04-16-v5-rustworkx-github-design.md @@ -2,7 +2,7 @@ **Date:** 2026-04-16 **Branch:** v5 -**Status:** Approved +**Status:** Approved (revised after senior engineering review) --- @@ -11,7 +11,7 @@ v5 introduces two major changes on a new branch: 1. **GitHub repo ingestion** -- users can pass a GitHub URL directly instead of a local path. graphify clones the repo and runs the full pipeline on it. -2. **rustworkx graph backend** -- NetworkX replaced with rustworkx throughout, with a NetworkX fallback if rustworkx is not installed. Adds `--dag` flag for acyclic directed graphs and parallel shortest-path in `graphify path`. +2. **rustworkx graph backend** -- rustworkx replaces NetworkX as the in-memory graph type throughout, with a NetworkX fallback if rustworkx is not installed. Adds `--dag` flag for acyclic directed graphs and parallel betweenness/shortest-path. Both changes are independent. The user-facing API and `graph.json` format are unchanged. @@ -33,7 +33,12 @@ Recognised URL formats: **`clone_or_update(org: str, repo: str, base_dir: Path) -> Path`** - Clone destination: `~/.graphify/repos/org/repo/` - First run: `git clone --depth 1 https://github.com/org/repo ` -- Subsequent runs: `git -C pull --ff-only` +- Subsequent runs (dest already exists): + ``` + git -C fetch --depth 1 origin + git -C reset --hard origin/HEAD + ``` + This unconditionally updates to the remote tip without requiring fast-forward eligibility and keeps history shallow. `git pull --ff-only` is explicitly avoided -- it fails on shallow clones when the upstream has rebased or advanced more than one commit. - Returns the local path on success ### Integration point @@ -45,10 +50,10 @@ Recognised URL formats: | Condition | Behaviour | |-----------|-----------| | Repo not found / private | Clear error message, exit 1 | -| git not installed | Error message pointing to git install, exit 1 | +| git not installed | `"git is required for GitHub repo ingestion. Install git and retry."`, exit 1 | | Network timeout | Retry once, then fail with message | -| Partial clone (disk full) | Detect incomplete state, clean up, report error | -| Already cloned, pull fails | Warn, use existing local copy | +| Partial clone (disk full, `.git` exists but incomplete) | Delete dest dir, report error, exit 1 | +| Already cloned, fetch/reset fails | Warn, continue with existing local copy | --- @@ -57,66 +62,164 @@ Recognised URL formats: ### Dependency - `rustworkx` added as optional dependency: `pip install graphifyy[fast]` -- If not installed: fall back to NetworkX with a one-time warning +- If not installed: fall back to NetworkX with a one-time warning printed to stderr: + `"[graphify] rustworkx not installed -- using NetworkX. Install graphifyy[fast] for 2-10x speedup."` - `pyproject.toml`: `fast = ["rustworkx"]`, added to `all` +- Note: NetworkX remains a hard dependency (required for Louvain community detection fallback -- rustworkx has no built-in community detection) ### Graph type mapping -| v4 (NetworkX) | v5 (rustworkx) | -|---------------|----------------| -| `nx.Graph` | `rustworkx.PyGraph` | -| `nx.DiGraph` | `rustworkx.PyDiGraph` | -| `nx.DiGraph` + `--dag` | `rustworkx.PyDAG` | +| v4 (NetworkX) | v5 rustworkx backend | v5 NetworkX fallback | +|---------------|----------------------|----------------------| +| `nx.Graph` | `rustworkx.PyGraph` | `nx.Graph` | +| `nx.DiGraph` | `rustworkx.PyDiGraph` | `nx.DiGraph` | +| `nx.DiGraph` + `--dag` | `rustworkx.PyDAG(check_cycle=True)` | `nx.DiGraph` (no cycle enforcement) | + +### GraphBundle -- the central abstraction + +`PyGraph`/`PyDiGraph`/`PyDAG` are Rust extension types (pyo3 `#[pyclass]`) with no `__dict__` slot. Attribute assignment (`G._id_to_idx = ...`) raises `AttributeError`. The correct design is a thin dataclass returned by `build_from_json()` and passed through the entire pipeline: + +```python +# graphify/utils.py (new file) +from __future__ import annotations +from dataclasses import dataclass, field +from typing import Union +import networkx as nx + +try: + import rustworkx as rx + _RX_GRAPH_TYPES = (rx.PyGraph, rx.PyDiGraph, rx.PyDAG) + HAS_RUSTWORKX = True +except ImportError: + _RX_GRAPH_TYPES = () + HAS_RUSTWORKX = False + +AnyGraph = Union["rx.PyGraph", "rx.PyDiGraph", "rx.PyDAG", nx.Graph, nx.DiGraph] + +@dataclass +class GraphBundle: + graph: AnyGraph + id_to_idx: dict[str, int] = field(default_factory=dict) # empty for NetworkX backend + idx_to_id: dict[int, str] = field(default_factory=dict) # empty for NetworkX backend + +def is_rustworkx(bundle: GraphBundle) -> bool: + return isinstance(bundle.graph, _RX_GRAPH_TYPES) +``` + +Every function that currently accepts `nx.Graph` is updated to accept `GraphBundle`. The internal graph and lookup dicts are accessed via `bundle.graph`, `bundle.id_to_idx`, `bundle.idx_to_id`. + +`is_rustworkx()` lives in `graphify/utils.py`. It is imported by every module that needs to branch on backend. No copies. ### ID mapping -rustworkx uses integer node indices internally. `build.py` maintains two dicts alongside every graph: -- `_id_to_idx: dict[str, int]` -- string node ID → rustworkx index -- `_idx_to_id: dict[int, str]` -- rustworkx index → string node ID +rustworkx uses integer node indices internally. `GraphBundle` carries two dicts: +- `id_to_idx: dict[str, int]` -- string node ID → rustworkx index +- `idx_to_id: dict[int, str]` -- rustworkx index → string node ID -These are attached as `G._id_to_idx` and `G._idx_to_id` on the graph object so downstream modules can look up either direction without re-scanning. +These are populated in `build_from_json()` as nodes are added and carried through the pipeline in the `GraphBundle`. The NetworkX fallback leaves both dicts empty (not needed). + +### API translation reference + +The following access patterns appear ~35 times across `analyze.py`, `cluster.py`, `export.py`, `serve.py`, `wiki.py`. Each must be dual-pathed via `is_rustworkx()`: + +| NetworkX | rustworkx equivalent | +|----------|---------------------| +| `G.nodes[nid]` | `G[id_to_idx[nid]]` | +| `G.nodes(data=True)` | `zip(G.node_indices(), G.nodes())` → use `idx_to_id[idx]` for ID | +| `G.edges(nid, data=True)` | `[(idx_to_id[u], idx_to_id[v], G.get_edge_data(u,v)) for u,v in G.incident_edges(id_to_idx[nid])]` | +| `G.degree(nid)` | `G.degree(id_to_idx[nid])` | +| `G.neighbors(nid)` → string IDs | `[idx_to_id[i] for i in G.neighbors(id_to_idx[nid])]` | +| `G.edges[u, v]` | `G.get_edge_data(id_to_idx[u], id_to_idx[v])` | +| `G.number_of_nodes()` | `G.num_nodes()` | +| `G.number_of_edges()` | `G.num_edges()` | ### Module changes -**`build.py`** -- `build_from_json()` returns a `PyGraph`/`PyDiGraph`/`PyDAG` (or `nx.Graph`/`nx.DiGraph` if rustworkx absent) -- ID normalization from v0.4.18 preserved -- Edge-add under `--dag`: cycle check via `rustworkx.is_directed_acyclic_graph()`; drop edge + warn on violation +**`graphify/utils.py`** (new) +- `GraphBundle` dataclass +- `is_rustworkx(bundle)` helper +- `AnyGraph` type alias -**`cluster.py`** -- Leiden (graspologic) unchanged -- takes adjacency matrix, not graph object -- Louvain fallback: replace `nx.community.louvain_communities()` with `rustworkx.community.louvain_communities()` -- Node list extraction uses `_idx_to_id` map +**`graphify/build.py`** +- `build_from_json()` returns `GraphBundle` (not a bare graph) +- Nodes added via `G.add_node(payload_dict)` → captures returned index → populates `id_to_idx`/`idx_to_id` +- Edges: `src_idx = id_to_idx.get(src)`, `tgt_idx = id_to_idx.get(tgt)` -- missing indices skip the edge (same semantics as v4 node_set check) +- ID normalization from v0.4.18 preserved (normalize before lookup) +- `--dag` edge-add: wrap in `try/except rustworkx.DAGWouldBeCyclic` -- drop edge, print warning to stderr. Do NOT use `rustworkx.is_directed_acyclic_graph()` for pre-checking (it cannot pre-check a prospective edge) +- NetworkX fallback: `GraphBundle(graph=nx.Graph(), id_to_idx={}, idx_to_id={})` -**`analyze.py`** -- `betweenness_centrality`: replace `nx.betweenness_centrality()` with `rustworkx.betweenness_centrality()` (parallel) -- `edge_betweenness_centrality`: replace with `rustworkx.edge_betweenness_centrality()` -- `shortest_path`: replace `nx.shortest_path()` with `rustworkx.dijkstra_shortest_paths()` (parallel) -- All functions accept either graph type via duck-typed helper `_is_rustworkx(G)` +**`graphify/cluster.py`** +- `_partition(bundle)` replaces `_partition(G)` +- Leiden (graspologic): graspologic's `leiden()` accepts a NetworkX graph. When rustworkx backend is active, convert to NetworkX for leiden only: + ```python + if is_rustworkx(bundle): + G_nx = nx.Graph() + for u, v in bundle.graph.edge_list(): + G_nx.add_edge(bundle.idx_to_id[u], bundle.idx_to_id[v]) + communities = leiden(G_nx) + else: + communities = leiden(bundle.graph) + ``` +- Louvain fallback: stays `nx.community.louvain_communities()` -- rustworkx has no built-in community detection. When rustworkx backend is active, same edge-list conversion as above. +- Node list extraction from leiden/louvain results uses `idx_to_id` where needed -**`export.py`** -- Replace `networkx.readwrite.json_graph.node_link_data()` with custom serializer that walks `G.node_indices()` and `G.edge_list()` -- SVG export (`nx.draw_networkx_*`): replaced with manual matplotlib scatter + line drawing using node positions from `rustworkx.spring_layout()` +**`graphify/analyze.py`** +- All public functions updated to accept `GraphBundle` +- `betweenness_centrality`: `rustworkx.betweenness_centrality(bundle.graph)` returns `dict[int, float]` -- remap to string IDs via `idx_to_id` +- `edge_betweenness_centrality`: `rustworkx.edge_betweenness_centrality(bundle.graph)` returns `dict[(int,int), float]` -- remap edge tuples to string ID pairs +- `shortest_path`: `rustworkx.dijkstra_shortest_paths(bundle.graph, src_idx)` returns `dict[int, list[int]]` -- decode path using `idx_to_id` at every position +- `suggest_questions()`: calls `nx.betweenness_centrality(G, k=k)` with approximation parameter `k`. rustworkx's `betweenness_centrality()` has no `k` parameter (always exact, parallel). When rustworkx backend active, drop `k` and call `rustworkx.betweenness_centrality(bundle.graph)`. This is always exact but faster due to parallelism; behavior change is documented. +- `_is_rustworkx()` removed -- use `is_rustworkx()` from `utils.py` -**`serve.py`** -- Replace `json_graph.node_link_data()` with same custom serializer as export.py -- MCP tool handlers updated to use `_id_to_idx` for node lookup +**`graphify/export.py`** +- Replace `json_graph.node_link_data()` with `_bundle_to_json(bundle)` -- custom serializer that produces the same schema as `node_link_data()` (see JSON schema below) +- SVG: `rustworkx.spring_layout(bundle.graph)` returns `dict[int, list[float]]` (integer-keyed). Map to string IDs via `idx_to_id` before passing to matplotlib. Node drawing iterates `zip(bundle.graph.node_indices(), bundle.graph.nodes())`. -**`wiki.py`** -- `nx.Graph` type hints replaced with union type -- Neighbour iteration uses `G.neighbors(idx)` + `_idx_to_id` lookup +**`graphify/serve.py`** +- `_load_graph()` uses same custom deserializer as export.py (loads `graph.json` → `GraphBundle`) +- MCP tool handlers updated: node lookups via `bundle.id_to_idx[node_id]`, neighbour traversal via API translation table above + +**`graphify/wiki.py`** +- Accepts `GraphBundle`, uses `is_rustworkx()` + API translation table for all graph traversal + +### JSON serializer schema + +The custom serializer `_bundle_to_json(bundle)` must produce output byte-compatible with `networkx.readwrite.json_graph.node_link_data()` so v4 `graph.json` files load without modification in v5. The schema: + +```json +{ + "directed": true, + "multigraph": false, + "graph": {}, + "nodes": [ + {"id": "session_validatetoken", "label": "ValidateToken", "file_type": "code", ...} + ], + "links": [ + {"source": "session_validatetoken", "target": "other_node", + "relation": "calls", "confidence": "EXTRACTED", "weight": 1.0, ...} + ] +} +``` + +Key points: +- Top-level key is `"links"` not `"edges"` (this is what `node_link_data()` produces; `build.py` already handles both via the `"links"` → `"edges"` remap on load) +- Node dicts include all attributes from `bundle.graph.nodes()` plus `"id"` key +- Edge dicts include all attributes from `bundle.graph.get_edge_data()` plus `"source"` and `"target"` string IDs ### `--dag` flag - New CLI flag: `graphify /path --dag` -- Uses `PyDAG` instead of `PyDiGraph` -- Cycle violations at edge-add time: drop edge, print warning to stderr -- Report includes topological sort order of god nodes -- skill.md updated to document `--dag` +- `build_from_json()` receives `dag=True`, uses `rustworkx.PyDAG(check_cycle=True)` +- Cycle violations: `except rustworkx.DAGWouldBeCyclic` → drop edge, print `"[graphify] warning: skipping edge {src} → {tgt} (would create cycle)"` to stderr +- Report includes topological sort order of god nodes via `rustworkx.topological_sort(bundle.graph)` decoded with `idx_to_id` +- NetworkX fallback when rustworkx absent: `--dag` flag accepted but cycle enforcement is silently skipped (no PyDAG available); warning printed once +- `"dag": true` written to `graph.json` metadata so serve.py can surface it in `get_graph_info` MCP tool. DAG enforcement is build-time only -- reloaded graphs are not re-enforced. +- `skill.md` updated to document `--dag` -### `graphify path` parallel shortest-path +### `graphify path` shortest-path speedup -- `analyze.py`: `shortest_path()` uses `rustworkx.dijkstra_shortest_paths()` with `parallel_threshold=500` (falls back to single-thread for small graphs) +- `analyze.py`: `shortest_path()` uses `rustworkx.dijkstra_shortest_paths(bundle.graph, src_idx)` -- no `parallel_threshold` parameter (rustworkx Dijkstra is always Rust-backed; per-query overhead reduction vs NetworkX is already ~10x) +- Path result decoded via `idx_to_id` at every element - No CLI change -- transparent speedup --- @@ -125,7 +228,7 @@ These are attached as `G._id_to_idx` and `G._idx_to_id` on the graph object so d ### graph.json -Format unchanged. v5 reads v4 `graph.json` files without modification. The integer index mapping is rebuilt from the JSON node list on load. +Format unchanged -- the custom serializer produces identical output to `node_link_data()`. v5 reads v4 `graph.json` files without modification. The integer index mapping is rebuilt from the JSON node list on load. ### pip install @@ -135,6 +238,8 @@ Format unchanged. v5 reads v4 `graph.json` files without modification. The integ | `pip install graphifyy[fast]` | rustworkx | yes | | `pip install graphifyy[all]` | rustworkx | yes | +NetworkX remains a hard dependency in all cases (required for community detection). + ### Python version Unchanged: Python 3.10+ @@ -143,12 +248,13 @@ Unchanged: Python 3.10+ ## Testing -- All 433 existing tests must pass with both backends (NetworkX fallback + rustworkx) +- All 433 existing tests must pass on the NetworkX fallback path (rustworkx not installed) +- Dual-backend coverage: `conftest.py` adds a `graph_backend` pytest fixture parametrized over `["networkx", "rustworkx"]`. Tests that create graphs import the fixture and get a `GraphBundle` built with the appropriate backend. This gives dual-backend coverage without duplicating test files. - New tests: - - `tests/test_github.py`: URL parsing, clone/update logic (mocked subprocess), error cases - - `tests/test_build_rustworkx.py`: graph round-trip, ID mapping correctness, DAG cycle rejection - - `tests/test_analyze_rustworkx.py`: betweenness output matches NetworkX within 1e-6 tolerance - - `tests/test_cluster_rustworkx.py`: community structure matches within reasonable variance + - `tests/test_github.py`: URL parsing (all four formats), clone logic (mocked `subprocess.run`), update logic (mocked fetch+reset), each error case + - `tests/test_build_rustworkx.py`: `GraphBundle` round-trip, `id_to_idx`/`idx_to_id` correctness, DAG cycle rejection (`DAGWouldBeCyclic` caught), JSON serializer output matches `node_link_data()` byte-for-byte on a fixture graph + - `tests/test_analyze_rustworkx.py`: betweenness output matches NetworkX within 1e-6 tolerance; `suggest_questions()` betweenness behavior change documented in test comment + - `tests/test_cluster_rustworkx.py`: leiden edge-list conversion produces same community structure as direct NetworkX call on same graph --- @@ -156,16 +262,18 @@ Unchanged: Python 3.10+ | File | Change | |------|--------| -| `graphify/github.py` | New | -| `graphify/build.py` | rustworkx backend, ID mapping | -| `graphify/cluster.py` | rustworkx Louvain fallback | -| `graphify/analyze.py` | parallel betweenness + shortest path | -| `graphify/export.py` | custom JSON serializer, matplotlib layout | -| `graphify/serve.py` | custom JSON serializer | -| `graphify/wiki.py` | graph type abstraction | -| `graphify/__main__.py` | `resolve_target()` call, `--dag` flag | -| `graphify/skill.md` | document `--dag`, GitHub URL input | -| `pyproject.toml` | `fast = ["rustworkx"]`, add to `all` | +| `graphify/github.py` | New -- GitHub URL resolution + clone/update | +| `graphify/utils.py` | New -- `GraphBundle`, `is_rustworkx()`, `AnyGraph` | +| `graphify/build.py` | Returns `GraphBundle`; rustworkx + NetworkX dual backend | +| `graphify/cluster.py` | `GraphBundle` input; leiden edge-list conversion | +| `graphify/analyze.py` | `GraphBundle` input; rustworkx parallel betweenness + path | +| `graphify/export.py` | `GraphBundle` input; custom JSON serializer; matplotlib layout fix | +| `graphify/serve.py` | `GraphBundle` input; custom deserializer; MCP handler updates | +| `graphify/wiki.py` | `GraphBundle` input; dual-path graph traversal | +| `graphify/__main__.py` | `resolve_target()` call; `--dag` flag | +| `graphify/skill.md` | Document `--dag`; GitHub URL input | +| `pyproject.toml` | `fast = ["rustworkx"]`; add to `all` | +| `tests/conftest.py` | `graph_backend` fixture parametrized over both backends | | `tests/test_github.py` | New | | `tests/test_build_rustworkx.py` | New | | `tests/test_analyze_rustworkx.py` | New | @@ -176,6 +284,7 @@ Unchanged: Python 3.10+ ## Out of scope for v5 - Private repo support (requires GitHub token -- future work) -- Incremental re-extraction after `git pull` (tracked via `--update`, already works once cloned) +- Incremental re-extraction after `git pull` (`--update` already handles this once cloned) - GraphQL / GitHub API (issues, PRs, file-level fetch) -- future work - rustworkx GPU acceleration -- future work +- DAG cycle enforcement on graph reload (enforcement is build-time only)