mirror of
https://github.com/safishamsi/graphify.git
synced 2026-07-18 21:37:34 +00:00
docs: revise v5 spec after senior engineering review -- GraphBundle, correct rustworkx APIs, git fetch strategy
This commit is contained in:
@@ -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 <dest>`
|
||||
- Subsequent runs: `git -C <dest> pull --ff-only`
|
||||
- Subsequent runs (dest already exists):
|
||||
```
|
||||
git -C <dest> fetch --depth 1 origin
|
||||
git -C <dest> 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)
|
||||
|
||||
Reference in New Issue
Block a user