feat(backend): add deterministic graph auto-layout for legacy model graphs#307
feat(backend): add deterministic graph auto-layout for legacy model graphs#307kartikeyg0104 wants to merge 2 commits intoc2siorg:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a deterministic, DAG-aware fallback auto-layout for legacy model graphs so ReactFlow can render graphs with meaningful structure when node positions are missing.
Changes:
- Replaced the previous vertical-stack fallback in
_apply_auto_layout()with a layered topological layout based on edges (with deterministic ordering). - Preserves existing node positions and only assigns positions to nodes without them.
- Adds handling for cycles/disconnected leftovers by assigning them to successive layers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Assign deterministic DAG-aware positions to nodes lacking coordinates. | ||
|
|
||
| This is a simple vertical-stack layout used as a fallback so that the | ||
| ReactFlow canvas can render the graph without crashing. For a more | ||
| sophisticated layout (e.g. dagre), see: | ||
| https://reactflow.dev/docs/examples/layout/dagre/ | ||
|
|
||
| TODO: Replace with a proper auto-layout algorithm (e.g. dagre) in a | ||
| follow-up issue. | ||
| Existing node positions are preserved. New positions are generated using a | ||
| lightweight layered layout derived from edge directions so ReactFlow can | ||
| render meaningful structure even for legacy graphs without saved positions. |
There was a problem hiding this comment.
Docstring says nodes “lacking coordinates”, but the implementation only assigns positions when the "position" key is absent (it does not fill missing/partial x/y). Consider rewording to “lacking a position” (or expand logic to handle partial coordinates) so behavior and docs match.
| for edge in graph.get("edges", []): | ||
| source = str(edge.get("source")) if edge.get("source") is not None else None | ||
| target = str(edge.get("target")) if edge.get("target") is not None else None | ||
| if source in by_id and target in by_id and target not in adjacency[source]: | ||
| adjacency[source].add(target) | ||
| indegree[target] += 1 | ||
|
|
||
| queue = deque(sorted(node_id for node_id, degree in indegree.items() if degree == 0)) | ||
| layer: dict[str, int] = {node_id: 0 for node_id in queue} | ||
| topo: list[str] = [] | ||
|
|
||
| while queue: | ||
| node_id = queue.popleft() | ||
| topo.append(node_id) | ||
| for nxt in sorted(adjacency[node_id]): | ||
| layer[nxt] = max(layer.get(nxt, 0), layer.get(node_id, 0) + 1) | ||
| indegree[nxt] -= 1 | ||
| if indegree[nxt] == 0: | ||
| queue.append(nxt) | ||
|
|
||
| # Cycles or disconnected leftovers: place them into successive layers. | ||
| leftovers = [node_id for node_id in sorted(by_id) if node_id not in topo] | ||
| max_layer = max(layer.values(), default=0) | ||
| for idx, node_id in enumerate(leftovers, start=1): | ||
| layer[node_id] = max_layer + idx | ||
|
|
||
| rows_by_layer: dict[int, list[str]] = defaultdict(list) | ||
| for node_id in sorted(by_id): | ||
| rows_by_layer[layer.get(node_id, 0)].append(node_id) | ||
|
|
||
| for x_layer, node_ids in rows_by_layer.items(): | ||
| for y_row, node_id in enumerate(node_ids): | ||
| node = by_id[node_id] | ||
| if "position" not in node: | ||
| node["position"] = {"x": float(250 * x_layer + 100), "y": float(140 * y_row + 100)} |
There was a problem hiding this comment.
The new layered layout logic isn’t covered by assertions that edge direction affects X-layering or that cycles/disconnected graphs are handled deterministically (current tests only assert positions exist). Add a unit test that builds a small multi-layer DAG (and optionally a simple cycle) and asserts the relative x positions/layer ordering are as expected.
PR ReviewRebaseMerge commits detected — please use rebase instead of merge: SquashYour PR has 2 commits. Please squash into a single commit. How to fixgit fetch origin
git rebase -i origin/main # mark all but first commit as "squash"
git push --force-with-leaseThis comment updates automatically on each push. |
Summary
_apply_auto_layout()with deterministic layered DAG-aware positioning:Why
Validation
ruff check app/services/deep_learning.pypytest -q tests/test_graph_json.py tests/test_deep_learning_service.pyRisk