fix(graph): execution-flow picker on the module overview; hierarchical layout guardrails (#805)

swati510 · web-flow · commit fb969b3daf99 · 2026-07-12T16:10:07.000+05:30
* fix(graph): fetch execution flows on the module overview so the picker works there

The shell already supports picking a flow from the module overview —
enterFullViewFromModule switches to the file-level graph and highlights
the trace. But the wrapper only fetched flows for file-level scopes, so
on the module overview the panel had no data and the toolbar button
silently did nothing. Widen the fetch gate to include the module view;
add a shell test covering the picker-then-jump path.

* fix(graph): refuse hierarchical layout above the ELK cap at the click

Past 500 nodes ELK never runs, but the toggle still activated: the force
layout stopped and the button sat active doing nothing. Guard the mode
switch itself — keep force running, surface the existing skip notice
synchronously — instead of relying on the canvas hook to fire it after
the mode has already changed.

* fix(graph): keep hierarchical layers readable when most modules are isolated

Modules with no dependency edges each became their own ELK component,
packed side by side into one huge band that drowned out the layering.
Lay out only the connected subgraph and shelve isolated modules in a
grid below it.

* fix(graph): drop leftover shell prop; focus flow trace once the full graph lands; quiet sigma in jsdom
diff --git a/packages/ui/__tests__/graph/graph-flow.test.tsx b/packages/ui/__tests__/graph/graph-flow.test.tsx
@@ -1,7 +1,29 @@
-import { describe, it, expect, vi } from "vitest";
-import { render, screen, fireEvent } from "@testing-library/react";
+import { describe, it, expect, vi, afterEach } from "vitest";
+import { render, screen, fireEvent, act } from "@testing-library/react";
+import { forwardRef, useImperativeHandle } from "react";
 import { GraphFlow } from "../../src/graph/graph-flow.js";
 
+// Stub the Sigma canvas: it dynamically imports "sigma", which needs WebGL2
+// and rejects under jsdom. These tests assert toolbar / notice / picker
+// behavior, none of which lives inside the canvas.
+const { focusNodeSpy } = vi.hoisted(() => ({ focusNodeSpy: vi.fn() }));
+vi.mock("../../src/graph/sigma/sigma-canvas.js", () => ({
+  SigmaCanvas: forwardRef(function MockSigmaCanvas(_props, ref) {
+    useImperativeHandle(ref, () => ({
+      focusNode: focusNodeSpy,
+      fitView: () => {},
+      zoomIn: () => {},
+      zoomOut: () => {},
+    }));
+    return <div data-testid="sigma-canvas" />;
+  }),
+}));
+
+afterEach(() => {
+  vi.useRealTimers();
+  focusNodeSpy.mockClear();
+});
+
 // Minimal prop set — no graphs supplied, so the canvas renders its empty state
 // while the toolbar (and its color-mode control) still mounts.
 const baseProps = {
@@ -102,4 +124,121 @@ describe("GraphFlow shell", () => {
     fireEvent.click(screen.getByRole("radio", { name: "Dead" }));
     expect(screen.getByText("No dead files in this repo")).toBeTruthy();
   });
+
+  it("opens the flow picker on the module overview and jumps to the full graph on selection", () => {
+    const onViewModeChange = vi.fn();
+    render(
+      <GraphFlow
+        {...baseProps}
+        initialViewMode="module"
+        onViewModeChange={onViewModeChange}
+        executionFlows={{
+          total_entry_points: 1,
+          flows: [
+            {
+              entry_point: "app.py::main",
+              entry_point_name: "main",
+              entry_point_score: 1,
+              trace: ["app.py::main", "core.py::run"],
+              depth: 1,
+              crosses_community: false,
+              communities_visited: [0],
+            },
+          ],
+        }}
+      />,
+    );
+
+    fireEvent.click(screen.getByRole("button", { name: "Execution flows" }));
+    expect(screen.getByText("Execution Flows")).toBeTruthy();
+
+    // Selecting a flow from the module overview switches to the file-level
+    // graph so the trace can actually be highlighted.
+    fireEvent.click(screen.getByText("main"));
+    expect(onViewModeChange).toHaveBeenCalledWith("full");
+  });
+
+  it("focuses the flow trace head once the graph gains it, exactly once", () => {
+    vi.useFakeTimers();
+    const flows = {
+      total_entry_points: 1,
+      flows: [
+        {
+          entry_point: "app.py::main",
+          entry_point_name: "main",
+          entry_point_score: 1,
+          trace: ["app.py::main", "core.py::run"],
+          depth: 1,
+          crosses_community: false,
+          communities_visited: [0],
+        },
+      ],
+    };
+    const { rerender } = render(
+      <GraphFlow {...baseProps} initialViewMode="full" executionFlows={flows} />,
+    );
+
+    fireEvent.click(screen.getByRole("button", { name: "Execution flows" }));
+    fireEvent.click(screen.getByText("main"));
+
+    // The focus timer fires while the full graph is still loading — the
+    // trace head isn't in the (empty) graph yet, so nothing is focused.
+    act(() => {
+      vi.advanceTimersByTime(800);
+    });
+    expect(focusNodeSpy).not.toHaveBeenCalled();
+
+    // The full graph lands: the deferred focus fires once for the trace head.
+    const nodes = flows.flows[0]!.trace.map((id) => ({
+      node_id: id,
+      label: id,
+      language: "python",
+    }));
+    rerender(
+      <GraphFlow
+        {...baseProps}
+        initialViewMode="full"
+        executionFlows={flows}
+        fullGraph={{ nodes, links: [] }}
+      />,
+    );
+    expect(focusNodeSpy).toHaveBeenCalledWith("app.py::main");
+    expect(focusNodeSpy).toHaveBeenCalledTimes(1);
+
+    // Later graph changes must not re-steer the camera for the same flow.
+    rerender(
+      <GraphFlow
+        {...baseProps}
+        initialViewMode="full"
+        executionFlows={flows}
+        fullGraph={{ nodes: [...nodes], links: [] }}
+      />,
+    );
+    expect(focusNodeSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("refuses hierarchical layout above the ELK cap and explains why", () => {
+    const nodes = Array.from({ length: 501 }, (_, i) => ({
+      node_id: `f${i}.ts`,
+      label: `f${i}.ts`,
+      language: "typescript",
+    }));
+    render(
+      <GraphFlow
+        {...baseProps}
+        initialViewMode="full"
+        fullGraph={{ nodes, links: [] }}
+      />,
+    );
+
+    fireEvent.click(screen.getByRole("button", { name: "Hierarchical" }));
+
+    // The mode must not activate — force stays on — and the notice says why.
+    expect(
+      screen.getByRole("button", { name: "Hierarchical" }).getAttribute("aria-pressed"),
+    ).toBe("false");
+    expect(screen.getByRole("status").textContent).toContain(
+      "Hierarchical layout is limited to 500 nodes",
+    );
+  });
 });
diff --git a/packages/ui/src/graph/elk-layout.ts b/packages/ui/src/graph/elk-layout.ts
@@ -374,11 +374,26 @@ export async function computeElkModulePositions(
 ): Promise<Map<string, { x: number; y: number }>> {
   if (moduleNodes.length === 0) return new Map();
 
-  const moduleIds = new Set(moduleNodes.map((m) => m.module_id));
+  // Modules without any dependency edge would each become their own ELK
+  // component, packed side by side into one huge band that drowns out the
+  // actual layering. Run ELK on the connected subgraph only; the isolated
+  // modules get a compact grid below it (see after extractPositions).
+  const connectedIds = new Set<string>();
+  for (const e of moduleEdges) {
+    connectedIds.add(e.source);
+    connectedIds.add(e.target);
+  }
+  const isolated = moduleNodes.filter((m) => !connectedIds.has(m.module_id));
+  const layoutNodes =
+    isolated.length === moduleNodes.length
+      ? moduleNodes
+      : moduleNodes.filter((m) => connectedIds.has(m.module_id));
+
+  const moduleIds = new Set(layoutNodes.map((m) => m.module_id));
 
   // Build all ancestor directories for compound node nesting
   const dirSet = new Set<string>();
-  for (const m of moduleNodes) {
+  for (const m of layoutNodes) {
     const ancestors = ancestorDirs(dirOf(m.module_id));
     for (const a of ancestors) {
       if (!moduleIds.has(a)) dirSet.add(a);
@@ -413,7 +428,7 @@ export async function computeElkModulePositions(
   }
 
   // Place module nodes into their parent directory (or root)
-  for (const m of moduleNodes) {
+  for (const m of layoutNodes) {
     const elkNode: ElkNode = {
       id: m.module_id,
       width: MODULE_NODE_WIDTH,
@@ -474,6 +489,39 @@ export async function computeElkModulePositions(
   };
   extractPositions(layout);
 
+  // Shelve isolated modules in a grid below the layered area so they stay
+  // visible without stretching the layers into a single band.
+  if (isolated.length > 0 && isolated.length < moduleNodes.length) {
+    let minX = Infinity;
+    let maxX = -Infinity;
+    let maxY = -Infinity;
+    for (const p of positions.values()) {
+      minX = Math.min(minX, p.x);
+      maxX = Math.max(maxX, p.x);
+      maxY = Math.max(maxY, p.y);
+    }
+    if (!Number.isFinite(minX)) {
+      minX = 0;
+      maxX = 0;
+      maxY = 0;
+    }
+    // Wider than the layered spacing: grid rows have no edges to separate
+    // nodes visually, so labels need the room instead.
+    const cellW = MODULE_NODE_WIDTH + 90;
+    const cellH = MODULE_NODE_HEIGHT + 45;
+    const cols = Math.max(
+      Math.ceil(Math.sqrt(isolated.length)),
+      Math.min(isolated.length, Math.floor((maxX - minX) / cellW) || 1),
+    );
+    const startY = maxY + MODULE_NODE_HEIGHT + 90;
+    isolated.forEach((m, i) => {
+      positions.set(m.module_id, {
+        x: minX + (i % cols) * cellW,
+        y: startY + Math.floor(i / cols) * cellH,
+      });
+    });
+  }
+
   return positions;
 }
 
diff --git a/packages/ui/src/graph/graph-flow.tsx b/packages/ui/src/graph/graph-flow.tsx
@@ -70,6 +70,7 @@ import {
   mergeCommunitySlice,
 } from "./sigma/constellation-adapter";
 import { computeRadialLayout } from "./sigma/radial-layout";
+import { ELK_MAX_NODES, elkSkipReason } from "./sigma/use-elk-sigma-layout";
 import type { SigmaNodeAttributes, SigmaEdgeAttributes } from "./sigma/types";
 import type GraphologyGraph from "graphology";
 import { useEgoFilter } from "./sigma/use-ego-filter";
@@ -842,6 +843,15 @@ export function GraphFlow(props: GraphFlowProps) {
     }
   }, [viewMode, onViewModeChange]);
 
+  // Flow index whose trace head has already been focused, so the deferred
+  // re-focus below fires at most once per selection and never re-steers the
+  // camera on later graph changes while the same flow stays active.
+  const flowFocusedRef = useRef<number | null>(null);
+  // Live graph handle for the focus timer (the effect below deliberately
+  // keeps sigmaGraph out of its deps).
+  const sigmaGraphRef = useRef(sigmaGraph);
+  sigmaGraphRef.current = sigmaGraph;
+
   // Execution flow highlighting
   useEffect(() => {
     if (activeFlowIdx === null || !executionFlows) {
@@ -860,13 +870,37 @@ export function GraphFlow(props: GraphFlowProps) {
 
     clearTimeout(focusTimerRef.current);
     focusTimerRef.current = setTimeout(() => {
+      focusTimerRef.current = undefined;
       const firstNode = flow.trace[0];
-      if (firstNode) sigmaRef.current?.focusNode(firstNode);
+      if (!firstNode) return;
+      if (sigmaGraphRef.current?.hasNode(firstNode)) {
+        flowFocusedRef.current = activeFlowIdx;
+        sigmaRef.current?.focusNode(firstNode);
+      }
+      // Node not loaded yet (module → full jump still fetching): the
+      // deferred-focus effect below picks it up once the graph gains it.
     }, 800);
     return () => clearTimeout(focusTimerRef.current);
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [activeFlowIdx, executionFlows]);
 
+  // Deferred flow focus: selecting a flow from the module overview kicks off
+  // the full-graph fetch, which can land after the 800ms timer above already
+  // fired against a graph without the trace head. Focus once when the graph
+  // gains the node; while the timer is still pending it stays the fast path.
+  useEffect(() => {
+    if (activeFlowIdx === null) {
+      flowFocusedRef.current = null;
+      return;
+    }
+    if (flowFocusedRef.current === activeFlowIdx) return;
+    if (focusTimerRef.current !== undefined) return;
+    const firstNode = executionFlows?.flows[activeFlowIdx]?.trace[0];
+    if (!firstNode || !sigmaGraph?.hasNode(firstNode)) return;
+    flowFocusedRef.current = activeFlowIdx;
+    sigmaRef.current?.focusNode(firstNode);
+  }, [activeFlowIdx, executionFlows, sigmaGraph]);
+
   // Context value (hover fields use static empty sets — highlighting handled by Sigma reducers)
   const ctxValue = useMemo<GraphContextValue>(
     () => ({
@@ -1045,9 +1079,17 @@ export function GraphFlow(props: GraphFlowProps) {
   }, [onViewModeChange, collapseAllHubs]);
 
   const handleLayoutModeChange = useCallback((mode: LayoutMode) => {
+    // Refuse right at the click when ELK can't run: switching the mode anyway
+    // would stop the force layout and leave an active-looking toggle doing
+    // nothing (the canvas-side notice covers graphs that grow past the cap
+    // after the mode is already active).
+    if (mode === "hierarchical" && sigmaGraph && sigmaGraph.order > ELK_MAX_NODES) {
+      setLayoutNotice(elkSkipReason(sigmaGraph.order));
+      return;
+    }
     setLayoutMode(mode);
     setLayoutNotice(null);
-  }, []);
+  }, [sigmaGraph]);
 
   const handleGraphThemeChange = useCallback(
     (theme: GraphTheme) => {
diff --git a/packages/ui/src/graph/sigma/use-elk-sigma-layout.ts b/packages/ui/src/graph/sigma/use-elk-sigma-layout.ts
@@ -21,6 +21,10 @@ import {
 // interactive; raising this ceiling means moving ELK into a web worker first.
 export const ELK_MAX_NODES = 500;
 
+export function elkSkipReason(order: number): string {
+  return `Hierarchical layout is limited to ${ELK_MAX_NODES} nodes — this view has ${order.toLocaleString()}. Switch to the Modules scope or narrow the view to use it.`;
+}
+
 export interface UseElkSigmaLayoutOptions {
   graph: Graph<SigmaNodeAttributes, SigmaEdgeAttributes> | null;
   sigma: Sigma | null;
@@ -102,9 +106,7 @@ export function useElkSigmaLayout(
   useEffect(() => {
     if (enabled && graph && graph.order > 0) {
       if (graph.order > ELK_MAX_NODES) {
-        options.onSkipped?.(
-          `Hierarchical layout is limited to ${ELK_MAX_NODES} nodes — this view has ${graph.order.toLocaleString()}. Switch to the Modules scope or narrow the view to use it.`,
-        );
+        options.onSkipped?.(elkSkipReason(graph.order));
         return;
       }
       compute();
diff --git a/packages/web/src/components/graph/graph-flow.tsx b/packages/web/src/components/graph/graph-flow.tsx
@@ -107,11 +107,13 @@ export function GraphFlow({
   const { repo } = useRepo(repoId);
   const resolvedRepoName = repoName ?? repo?.name;
   const { communities } = useCommunities(repoId);
-  // Execution flows only highlight a file-level trace, so defer the fetch until
-  // a full/file graph is actually needed (keeps the modules mount to
-  // /communities + /modules only).
+  // Flow traces highlight file-level nodes, but the picker must also work from
+  // the module overview: selecting a flow there makes the shell jump to the
+  // full graph (enterFullViewFromModule) and highlight the trace. Dead/hot
+  // scopes render file-level graphs too, so flows work there as well — only
+  // the constellation skips the fetch.
   const { flows: executionFlowsData } = useExecutionFlows(
-    needsFullGraph ? repoId : null,
+    viewMode !== "architecture" || needsFullGraph ? repoId : null,
     {
       top_n: 10,
       max_depth: 6,
