Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ import {
createGraphPreferences,
createSessionState,
normalizeGraphPreferences,
computeAutoLayout
computeAutoLayout,
laneHierarchyFromPrimary
} from "./graphLayout";
import { GraphLaneNode } from "./graphNodes/LaneNode";
import { GraphProposalNode } from "./graphNodes/ProposalNode";
Expand Down Expand Up @@ -336,6 +337,12 @@ function GraphInner() {
const [textPromptError, setTextPromptError] = React.useState<string | null>(null);
const [singleActionsOpen, setSingleActionsOpen] = React.useState(false);
const [batchActionsOpen, setBatchActionsOpen] = React.useState(false);
/** In Overview, risk edges are hidden by default so the tree stays readable. */
const [showOverviewRiskEdges, setShowOverviewRiskEdges] = React.useState(false);

React.useEffect(() => {
if (viewMode !== "all") setShowOverviewRiskEdges(false);
}, [viewMode]);

React.useEffect(() => {
void refreshEnvironmentMappings();
Expand Down Expand Up @@ -463,7 +470,11 @@ function GraphInner() {
}, [collapsedLaneIds, lanes]);

const laneById = React.useMemo(() => new Map(lanes.map((lane) => [lane.id, lane] as const)), [lanes]);
const primaryLaneId = React.useMemo(() => lanes.find((lane) => lane.laneType === "primary")?.id ?? null, [lanes]);
const primaryHierarchyMeta = React.useMemo(() => laneHierarchyFromPrimary(lanes), [lanes]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟠 High] [🔵 Bug]

This memo now assumes laneHierarchyFromPrimary(lanes) always returns a real primary lane, but GraphInner later has an explicit if (lanes.length === 0) empty-state branch and the app store initializes lanes as []. In the empty-workspace / initial-load case, laneHierarchyFromPrimary falls back to lanes[0]!, so primaryHierarchyMeta.primary.id throws before the component can render either the loading UI or the "No lanes yet" state. Guard the hierarchy helper for empty input or make primary nullable and handle that here before dereferencing .id.

// apps/desktop/src/renderer/components/graph/WorkspaceGraphPage.tsx
const primaryHierarchyMeta = React.useMemo(() => laneHierarchyFromPrimary(lanes), [lanes]);
const primaryLaneId = React.useMemo(
  () => primaryHierarchyMeta.primary.id,
  [primaryHierarchyMeta.primary.id]
);

const primaryLaneId = React.useMemo(
() => primaryHierarchyMeta.primary.id,
[primaryHierarchyMeta.primary.id]
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Guard hierarchy metadata while lanes are empty.

This hook runs before the lanes.length === 0 empty-state return. With an empty workspace or transient refresh state, laneHierarchyFromPrimary([]) falls back to lanes[0]!, and Line 475 dereferences undefined.id, crashing the graph instead of showing “No lanes yet.”

Proposed guard
-  const primaryHierarchyMeta = React.useMemo(() => laneHierarchyFromPrimary(lanes), [lanes]);
-  const primaryLaneId = React.useMemo(
-    () => primaryHierarchyMeta.primary.id,
-    [primaryHierarchyMeta.primary.id]
-  );
+  const primaryHierarchyMeta = React.useMemo(
+    () => (lanes.length > 0 ? laneHierarchyFromPrimary(lanes) : null),
+    [lanes]
+  );
+  const primaryLaneId = primaryHierarchyMeta?.primary.id ?? null;

Then update downstream reads to handle the null case, e.g. primaryHierarchyMeta?.depthByLaneId.get(lane.id) ?? 0 and primaryHierarchyMeta?.primary ?? null.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/components/graph/WorkspaceGraphPage.tsx` around
lines 473 - 477, The hook computes hierarchy metadata unguarded and crashes when
lanes is empty because laneHierarchyFromPrimary([]) can return a structure based
on lanes[0] and primaryHierarchyMeta.primary.id gets dereferenced; change the
useMemo to only call laneHierarchyFromPrimary when lanes.length > 0 (e.g.
React.useMemo(() => lanes.length ? laneHierarchyFromPrimary(lanes) : null,
[lanes])) and update any downstream reads of primaryHierarchyMeta (such as
primaryHierarchyMeta.primary.id and primaryHierarchyMeta.depthByLaneId.get(...))
to handle the null case (e.g. use optional chaining and fallbacks like
primaryHierarchyMeta?.primary ?? null and
primaryHierarchyMeta?.depthByLaneId.get(lane.id) ?? 0) so the component shows
the empty-state safely.

const laneIdByBranchRef = React.useMemo(() => {
const map = new Map<string, string>();
for (const lane of lanes) {
Expand Down Expand Up @@ -1166,6 +1177,8 @@ function GraphInner() {
autoRebaseStatus: autoRebaseByLaneId[lane.id] ?? null,
activeSessions: activeSessionsByLaneId[lane.id] ?? 0,
collapsedChildCount,
hierarchyDepth: primaryHierarchyMeta.depthByLaneId.get(lane.id) ?? 0,
parentLaneName: primaryHierarchyMeta.parentNameByLaneId.get(lane.id) ?? null,
dimmed: false,
activityBucket: activityBucketByLaneId[lane.id] ?? "medium",
viewMode,
Expand Down Expand Up @@ -1246,6 +1259,8 @@ function GraphInner() {
autoRebaseStatus: null,
activeSessions: 0,
collapsedChildCount: 0,
hierarchyDepth: 0,
parentLaneName: null,
dimmed: false,
activityBucket: "medium",
viewMode,
Expand Down Expand Up @@ -1274,7 +1289,7 @@ function GraphInner() {
}

const nextEdges: Array<Edge<GraphEdgeData>> = [];
const primaryLane = lanes.find((lane) => lane.laneType === "primary") ?? null;
const primaryLane = primaryHierarchyMeta.primary;
const riskPairsWithVisibleEdge = new Set<string>();
const laneHasProposalConflict = (proposal: IntegrationProposal, sourceLaneId: string): boolean => {
const steps = proposalSteps(proposal);
Expand Down Expand Up @@ -1306,6 +1321,8 @@ function GraphInner() {
if (viewMode === "all" || viewMode === "stack") {
for (const lane of lanes) {
if (!primaryLane || lane.id === primaryLane.id) continue;
// In Overview, stack edges already show the tree; skip redundant "primary → lane" spokes.
if (viewMode === "all" && lane.parentLaneId && laneById.has(lane.parentLaneId)) continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

The new Overview spoke suppression only checks whether the lane has an in-workspace parent, not whether that parent is actually visible. ```ts
// apps/desktop/src/renderer/components/graph/WorkspaceGraphPage.tsx
if (viewMode === "all" && lane.parentLaneId && laneById.has(lane.parentLaneId)) continue;
if (!visibleNodeIds.has(primaryLane.id) || !visibleNodeIds.has(lane.id)) continue;

Because filters are applied per lane, a child can remain visible while its parent is hidden by tag/status/archive/root filters; in that case the stack edge is skipped for invisibility and this guard also removes the fallback `primary → child` edge, leaving the child isolated in Overview. Only suppress the primary spoke when the parent node is visible enough for the stack edge to be rendered.

```suggestion
        if (viewMode === "all" && lane.parentLaneId && visibleNodeIds.has(lane.parentLaneId)) continue;

if (!visibleNodeIds.has(primaryLane.id) || !visibleNodeIds.has(lane.id)) continue;
const pair = edgePairKey(primaryLane.id, lane.id);
const pr = prOverlayByPair.get(pair);
Expand Down Expand Up @@ -1341,7 +1358,7 @@ function GraphInner() {
}
}

if (viewMode === "all" || viewMode === "risk") {
if (viewMode === "risk" || (viewMode === "all" && showOverviewRiskEdges)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

This new render gate no longer matches the earlier riskPairsWithVisibleEdge population logic, which still runs for every viewMode === "all" pair. As a result, default Overview (showOverviewRiskEdges = false) still marks risk pairs as having a visible risk edge, so the topology/stack edge builders strip their pr payloads, but the risk edge carrying that PR overlay is never rendered. Users lose the PR badge/click target for any pair that has both a stack/topology relationship and overlap metadata. Mirror the same showOverviewRiskEdges condition when filling riskPairsWithVisibleEdge so PR overlays stay on the visible edge when the overlap web is hidden.

// apps/desktop/src/renderer/components/graph/WorkspaceGraphPage.tsx
if (viewMode === "risk" || (viewMode === "all" && showOverviewRiskEdges)) {
  for (const [key, risk] of riskByPair.entries()) {
    if (risk.riskLevel === "none" && risk.overlapCount === 0) continue;

for (const [key, risk] of riskByPair.entries()) {
if (risk.riskLevel === "none" && risk.overlapCount === 0) continue;
const [laneAId, laneBId] = key.split("::");
Expand Down Expand Up @@ -1433,7 +1450,9 @@ function GraphInner() {
loadedGraphPreferences,
prOverlayByPair,
prOverlayByLaneId,
primaryHierarchyMeta,
riskByPair,
showOverviewRiskEdges,
statusByLane,
syncByLaneId,
viewMode,
Expand Down Expand Up @@ -2922,6 +2941,18 @@ function GraphInner() {
Reset View
</Button>

{viewMode === "all" ? (
<Button
size="sm"
variant={showOverviewRiskEdges ? "primary" : "outline"}
className="h-7 px-2 text-[11px]"
title="Show or hide predicted file-overlap links between lanes. Stack links stay visible."
onClick={() => setShowOverviewRiskEdges((prev) => !prev)}
>
{showOverviewRiskEdges ? "Hide overlap web" : "Show overlap web"}
</Button>
) : null}

<Button
size="sm"
variant={showRiskMatrix ? "primary" : "outline"}
Expand All @@ -2940,6 +2971,8 @@ function GraphInner() {
edges={edges}
nodeTypes={nodeTypes}
edgeTypes={edgeTypes}
onlyRenderVisibleElements
nodesConnectable={false}
onNodesChange={onNodesChange}
onEdgesChange={onEdgesChange}
onNodeDragStart={onNodeDragStart}
Expand Down
18 changes: 11 additions & 7 deletions apps/desktop/src/renderer/components/graph/graphHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,23 @@ export const VIEW_MODES: GraphViewMode[] = ["stack", "risk", "activity", "all"];
export const VIEW_MODE_META: Record<GraphViewMode, { label: string; helper: string }> = {
all: {
label: "Overview",
helper: "See the full workspace map with dependencies, environments, and active pull requests."
helper:
"Primary lane on top, children row by row. Solid lines are stack order; use Show overlap web for conflict-risk links."
},
stack: {
label: "Dependencies",
helper: "Follow parent-child lane relationships and drag lanes to change how work is stacked."
helper:
"Same top-down tree as Overview: primary on top, children row by row. Drag lanes to change how work is stacked."
},
risk: {
label: "Conflict Risk",
helper: "Highlight overlapping work and jump into the pair matrix when you need file-level conflict detail."
helper:
"Same tree layout as Overview; risk edges highlight overlapping work. Open the pair matrix for file-level detail."
},
activity: {
label: "Activity",
helper: "Surface the lanes with the most recent work, sessions, and movement."
helper:
"Same tree rows as other modes; siblings within a row sort by recent activity, then stack depth and name."
}
};

Expand Down Expand Up @@ -169,9 +173,9 @@ export function nodeDimensions(
if (bucket === "high") return { width: 200, height: 100 };
return { width: 160, height: 80 };
}
if (isIntegration) return { width: 220, height: integrationSourceCount > 2 ? 122 : 110 };
if (lane.laneType === "primary") return { width: 200, height: 100 };
return { width: 160, height: 80 };
if (isIntegration) return { width: 228, height: integrationSourceCount > 2 ? 130 : 118 };
if (lane.laneType === "primary") return { width: 228, height: 132 };
return { width: 208, height: 124 };
}

export function branchNameFromRef(ref: string): string {
Expand Down
90 changes: 89 additions & 1 deletion apps/desktop/src/renderer/components/graph/graphLayout.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
import { describe, expect, it } from "vitest";
import { normalizeGraphPreferences } from "./graphLayout";
import type { LaneSummary } from "../../../shared/types";
import { computeAutoLayout, laneHierarchyFromPrimary, normalizeGraphPreferences } from "./graphLayout";

function lane(partial: Partial<LaneSummary> & Pick<LaneSummary, "id" | "name" | "laneType" | "parentLaneId">): LaneSummary {
return {
description: null,
attachedRootPath: null,
baseRef: "refs/heads/main",
branchRef: `refs/heads/${partial.name}`,
worktreePath: "",
childCount: 0,
stackDepth: 0,
parentStatus: null,
isEditProtected: false,
status: { dirty: false, ahead: 0, behind: 0, remoteBehind: -1, rebaseInProgress: false },
color: null,
icon: null,
tags: [],
createdAt: "2026-01-01T00:00:00.000Z",
archivedAt: null,
...partial
};
}

describe("normalizeGraphPreferences", () => {
it("keeps the new preferences shape unchanged", () => {
Expand Down Expand Up @@ -40,3 +62,69 @@ describe("normalizeGraphPreferences", () => {
});
});
});

describe("laneHierarchyFromPrimary", () => {
it("assigns depth 0 to primary and increments along parent chain", () => {
const lanes = [
lane({ id: "p", name: "main", laneType: "primary", parentLaneId: null }),
lane({ id: "a", name: "feat-a", laneType: "worktree", parentLaneId: "p" }),
lane({ id: "b", name: "feat-b", laneType: "worktree", parentLaneId: "a" })
];
const { depthByLaneId, parentNameByLaneId } = laneHierarchyFromPrimary(lanes);
expect(depthByLaneId.get("p")).toBe(0);
expect(depthByLaneId.get("a")).toBe(1);
expect(depthByLaneId.get("b")).toBe(2);
expect(parentNameByLaneId.get("a")).toBe("main");
expect(parentNameByLaneId.get("b")).toBe("feat-a");
});

it("marks lanes not under primary with a large sentinel depth", () => {
const lanes = [
lane({ id: "p", name: "main", laneType: "primary", parentLaneId: null }),
lane({ id: "o", name: "orphan", laneType: "worktree", parentLaneId: null })
];
const { depthByLaneId } = laneHierarchyFromPrimary(lanes);
expect(depthByLaneId.get("o")).toBe(10_000);
});
});

describe("computeAutoLayout overview", () => {
it("places primary above children in row order", () => {
const lanes = [
lane({ id: "p", name: "main", laneType: "primary", parentLaneId: null, stackDepth: 0 }),
lane({ id: "x", name: "z-last", laneType: "worktree", parentLaneId: "p", stackDepth: 1 }),
lane({ id: "y", name: "a-first", laneType: "worktree", parentLaneId: "p", stackDepth: 1 })
];
const pos = computeAutoLayout(lanes, "all", {}, {});
expect(pos.p!.y).toBeLessThan(pos.x!.y);
expect(pos.p!.y).toBeLessThan(pos.y!.y);
expect(pos.x!.y).toBe(pos.y!.y);
expect(pos.y!.x).toBeLessThan(pos.x!.x);
});
});

describe("computeAutoLayout aligned modes", () => {
it("matches stack and risk positions to overview for the same lanes", () => {
const lanes = [
lane({ id: "p", name: "main", laneType: "primary", parentLaneId: null, stackDepth: 0 }),
lane({ id: "c", name: "child", laneType: "worktree", parentLaneId: "p", stackDepth: 1 })
];
const allPos = computeAutoLayout(lanes, "all", {}, {});
const stackPos = computeAutoLayout(lanes, "stack", {}, {});
const riskPos = computeAutoLayout(lanes, "risk", {}, {});
expect(stackPos).toEqual(allPos);
expect(riskPos).toEqual(allPos);
});

it("activity mode sorts siblings by activity score within the same depth", () => {
const lanes = [
lane({ id: "p", name: "main", laneType: "primary", parentLaneId: null, stackDepth: 0 }),
lane({ id: "low", name: "low", laneType: "worktree", parentLaneId: "p", stackDepth: 1 }),
lane({ id: "high", name: "high", laneType: "worktree", parentLaneId: "p", stackDepth: 1 })
];
const scores = { low: 1, high: 99 };
const pos = computeAutoLayout(lanes, "activity", scores, {});
expect(pos.low!.y).toBe(pos.high!.y);
expect(pos.high!.x).toBeLessThan(pos.low!.x);
});
});
Loading
Loading