mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-01-30 06:12:03 +00:00
feat(graph-view): implement task deletion and dependency management enhancements
- Added onDeleteTask functionality to allow task deletion from both board and graph views. - Integrated delete options for dependencies in the graph view, enhancing user interaction. - Updated ancestor context section to clarify the role of parent tasks in task descriptions. - Improved layout handling in graph view to preserve node positions during updates. This update enhances task management capabilities and improves user experience in the graph view.
This commit is contained in:
@@ -1057,6 +1057,7 @@ export function BoardView() {
|
||||
setSpawnParentFeature(feature);
|
||||
setShowAddDialog(true);
|
||||
}}
|
||||
onDeleteTask={(feature) => handleDeleteFeature(feature.id)}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -169,9 +169,8 @@ export function AddFeatureDialog({
|
||||
if (parentFeature) {
|
||||
const ancestorList = getAncestors(parentFeature, allFeatures);
|
||||
setAncestors(ancestorList);
|
||||
// Select all ancestors by default (including parent)
|
||||
const allIds = new Set([parentFeature.id, ...ancestorList.map((a) => a.id)]);
|
||||
setSelectedAncestorIds(allIds);
|
||||
// Only select parent by default - ancestors are optional context
|
||||
setSelectedAncestorIds(new Set([parentFeature.id]));
|
||||
} else {
|
||||
setAncestors([]);
|
||||
setSelectedAncestorIds(new Set());
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useState } from 'react';
|
||||
import { Checkbox } from '@/components/ui/checkbox';
|
||||
import { Label } from '@/components/ui/label';
|
||||
import { Button } from '@/components/ui/button';
|
||||
import { ChevronDown, ChevronRight, Users } from 'lucide-react';
|
||||
import { ChevronDown, ChevronRight, Users, CheckCircle2 } from 'lucide-react';
|
||||
import { cn } from '@/lib/utils';
|
||||
import type { AncestorContext } from '@automaker/dependency-resolver';
|
||||
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible';
|
||||
@@ -102,7 +102,8 @@ export function AncestorContextSection({
|
||||
</div>
|
||||
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Select ancestors to include their context in the new task's prompt.
|
||||
The parent task context will be included to help the AI understand the background.
|
||||
Additional ancestors can optionally be included for more context.
|
||||
</p>
|
||||
|
||||
<div className="space-y-1 max-h-[200px] overflow-y-auto border rounded-lg p-2 bg-muted/20">
|
||||
@@ -122,7 +123,13 @@ export function AncestorContextSection({
|
||||
<div
|
||||
className={cn(
|
||||
'flex items-start gap-2 p-2 rounded-md transition-colors',
|
||||
isSelected ? 'bg-primary/10' : 'hover:bg-muted/50'
|
||||
item.isParent
|
||||
? isSelected
|
||||
? 'bg-[var(--status-success-bg)] border border-[var(--status-success)]/30'
|
||||
: 'bg-muted/30 border border-border hover:bg-muted/50'
|
||||
: isSelected
|
||||
? 'bg-primary/10'
|
||||
: 'hover:bg-muted/50'
|
||||
)}
|
||||
style={{ marginLeft: item.isParent ? 0 : `${item.depth * 12}px` }}
|
||||
>
|
||||
@@ -156,10 +163,13 @@ export function AncestorContextSection({
|
||||
className="text-sm font-medium cursor-pointer truncate flex-1"
|
||||
>
|
||||
{displayTitle}
|
||||
{item.isParent && (
|
||||
<span className="ml-2 text-xs text-primary font-normal">(Parent)</span>
|
||||
)}
|
||||
</label>
|
||||
{item.isParent && (
|
||||
<span className="ml-2 inline-flex items-center gap-1 text-xs text-[var(--status-success)] font-medium">
|
||||
<CheckCircle2 className="w-3 h-3" />
|
||||
Completed Parent
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<CollapsibleContent>
|
||||
|
||||
@@ -1,14 +1,16 @@
|
||||
import { memo } from 'react';
|
||||
import { memo, useState } from 'react';
|
||||
import { BaseEdge, getBezierPath, EdgeLabelRenderer } from '@xyflow/react';
|
||||
import type { EdgeProps } from '@xyflow/react';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { Feature } from '@/store/app-store';
|
||||
import { Trash2 } from 'lucide-react';
|
||||
|
||||
export interface DependencyEdgeData {
|
||||
sourceStatus: Feature['status'];
|
||||
targetStatus: Feature['status'];
|
||||
isHighlighted?: boolean;
|
||||
isDimmed?: boolean;
|
||||
onDeleteDependency?: (sourceId: string, targetId: string) => void;
|
||||
}
|
||||
|
||||
const getEdgeColor = (sourceStatus?: Feature['status'], targetStatus?: Feature['status']) => {
|
||||
@@ -31,6 +33,8 @@ const getEdgeColor = (sourceStatus?: Feature['status'], targetStatus?: Feature['
|
||||
export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) {
|
||||
const {
|
||||
id,
|
||||
source,
|
||||
target,
|
||||
sourceX,
|
||||
sourceY,
|
||||
targetX,
|
||||
@@ -42,6 +46,7 @@ export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) {
|
||||
animated,
|
||||
} = props;
|
||||
|
||||
const [isHovered, setIsHovered] = useState(false);
|
||||
const edgeData = data as DependencyEdgeData | undefined;
|
||||
|
||||
const [edgePath, labelX, labelY] = getBezierPath({
|
||||
@@ -67,14 +72,30 @@ export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) {
|
||||
edgeData?.sourceStatus === 'completed' || edgeData?.sourceStatus === 'verified';
|
||||
const isInProgress = edgeData?.targetStatus === 'in_progress';
|
||||
|
||||
const handleDelete = (e: React.MouseEvent) => {
|
||||
e.stopPropagation();
|
||||
edgeData?.onDeleteDependency?.(source, target);
|
||||
};
|
||||
|
||||
return (
|
||||
<>
|
||||
{/* Invisible wider path for hover detection */}
|
||||
<path
|
||||
d={edgePath}
|
||||
fill="none"
|
||||
stroke="transparent"
|
||||
strokeWidth={20}
|
||||
onMouseEnter={() => setIsHovered(true)}
|
||||
onMouseLeave={() => setIsHovered(false)}
|
||||
style={{ cursor: 'pointer' }}
|
||||
/>
|
||||
|
||||
{/* Background edge for better visibility */}
|
||||
<BaseEdge
|
||||
id={`${id}-bg`}
|
||||
path={edgePath}
|
||||
style={{
|
||||
strokeWidth: isHighlighted ? 6 : 4,
|
||||
strokeWidth: isHighlighted || isHovered ? 6 : 4,
|
||||
stroke: 'var(--background)',
|
||||
opacity: isDimmed ? 0.3 : 1,
|
||||
}}
|
||||
@@ -92,20 +113,51 @@ export const DependencyEdge = memo(function DependencyEdge(props: EdgeProps) {
|
||||
isDimmed && 'graph-edge-dimmed'
|
||||
)}
|
||||
style={{
|
||||
strokeWidth: isHighlighted ? 4 : selected ? 3 : isDimmed ? 1 : 2,
|
||||
stroke: edgeColor,
|
||||
strokeWidth: isHighlighted ? 4 : isHovered || selected ? 3 : isDimmed ? 1 : 2,
|
||||
stroke: isHovered || selected ? 'var(--status-error)' : edgeColor,
|
||||
strokeDasharray: isCompleted ? 'none' : '5 5',
|
||||
filter: isHighlighted
|
||||
? 'drop-shadow(0 0 6px var(--brand-500))'
|
||||
: selected
|
||||
? 'drop-shadow(0 0 3px var(--brand-500))'
|
||||
: isHovered || selected
|
||||
? 'drop-shadow(0 0 4px var(--status-error))'
|
||||
: 'none',
|
||||
opacity: isDimmed ? 0.2 : 1,
|
||||
}}
|
||||
/>
|
||||
|
||||
{/* Delete button on hover or select */}
|
||||
{(isHovered || selected) && edgeData?.onDeleteDependency && (
|
||||
<EdgeLabelRenderer>
|
||||
<div
|
||||
style={{
|
||||
position: 'absolute',
|
||||
transform: `translate(-50%, -50%) translate(${labelX}px,${labelY}px)`,
|
||||
pointerEvents: 'auto',
|
||||
zIndex: 1000,
|
||||
}}
|
||||
onMouseEnter={() => setIsHovered(true)}
|
||||
onMouseLeave={() => setIsHovered(false)}
|
||||
>
|
||||
<button
|
||||
onClick={handleDelete}
|
||||
className={cn(
|
||||
'flex items-center justify-center',
|
||||
'w-6 h-6 rounded-full',
|
||||
'bg-[var(--status-error)] hover:bg-[var(--status-error)]/80',
|
||||
'text-white shadow-lg',
|
||||
'transition-all duration-150',
|
||||
'hover:scale-110'
|
||||
)}
|
||||
title="Delete dependency"
|
||||
>
|
||||
<Trash2 className="w-3 h-3" />
|
||||
</button>
|
||||
</div>
|
||||
</EdgeLabelRenderer>
|
||||
)}
|
||||
|
||||
{/* Animated particles for in-progress edges */}
|
||||
{animated && (
|
||||
{animated && !isHovered && (
|
||||
<EdgeLabelRenderer>
|
||||
<div
|
||||
style={{
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
Terminal,
|
||||
RotateCcw,
|
||||
GitFork,
|
||||
Trash2,
|
||||
} from 'lucide-react';
|
||||
import { TaskNodeData } from '../hooks/use-graph-nodes';
|
||||
import { Button } from '@/components/ui/button';
|
||||
@@ -91,8 +92,10 @@ export const TaskNode = memo(function TaskNode({ data, selected }: TaskNodeProps
|
||||
<>
|
||||
{/* Target handle (left side - receives dependencies) */}
|
||||
<Handle
|
||||
id="target"
|
||||
type="target"
|
||||
position={Position.Left}
|
||||
isConnectable={true}
|
||||
className={cn(
|
||||
'w-3 h-3 !bg-border border-2 border-background',
|
||||
'transition-colors duration-200',
|
||||
@@ -277,6 +280,18 @@ export const TaskNode = memo(function TaskNode({ data, selected }: TaskNodeProps
|
||||
<GitFork className="w-3 h-3 mr-2" />
|
||||
Spawn Sub-Task
|
||||
</DropdownMenuItem>
|
||||
{!data.isRunning && (
|
||||
<DropdownMenuItem
|
||||
className="text-xs text-[var(--status-error)] cursor-pointer"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
data.onDeleteTask?.();
|
||||
}}
|
||||
>
|
||||
<Trash2 className="w-3 h-3 mr-2" />
|
||||
Delete Task
|
||||
</DropdownMenuItem>
|
||||
)}
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
</div>
|
||||
@@ -336,8 +351,10 @@ export const TaskNode = memo(function TaskNode({ data, selected }: TaskNodeProps
|
||||
|
||||
{/* Source handle (right side - provides to dependents) */}
|
||||
<Handle
|
||||
id="source"
|
||||
type="source"
|
||||
position={Position.Right}
|
||||
isConnectable={true}
|
||||
className={cn(
|
||||
'w-3 h-3 !bg-border border-2 border-background',
|
||||
'transition-colors duration-200',
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useState, useEffect } from 'react';
|
||||
import { useCallback, useState, useEffect, useRef } from 'react';
|
||||
import {
|
||||
ReactFlow,
|
||||
Background,
|
||||
@@ -110,10 +110,52 @@ function GraphCanvasInner({
|
||||
const [nodes, setNodes, onNodesChange] = useNodesState(layoutedNodes);
|
||||
const [edges, setEdges, onEdgesChange] = useEdgesState(layoutedEdges);
|
||||
|
||||
// Update nodes/edges when features change
|
||||
// Track if initial layout has been applied
|
||||
const hasInitialLayout = useRef(false);
|
||||
// Track the previous node IDs to detect new nodes
|
||||
const prevNodeIds = useRef<Set<string>>(new Set());
|
||||
|
||||
// Update nodes/edges when features change, but preserve user positions
|
||||
useEffect(() => {
|
||||
setNodes(layoutedNodes);
|
||||
setEdges(layoutedEdges);
|
||||
const currentNodeIds = new Set(layoutedNodes.map((n) => n.id));
|
||||
const isInitialRender = !hasInitialLayout.current;
|
||||
|
||||
// Check if there are new nodes that need layout
|
||||
const hasNewNodes = layoutedNodes.some((n) => !prevNodeIds.current.has(n.id));
|
||||
|
||||
if (isInitialRender) {
|
||||
// Apply full layout for initial render
|
||||
setNodes(layoutedNodes);
|
||||
setEdges(layoutedEdges);
|
||||
hasInitialLayout.current = true;
|
||||
} else if (hasNewNodes) {
|
||||
// New nodes added - need to re-layout but try to preserve existing positions
|
||||
setNodes((currentNodes) => {
|
||||
const positionMap = new Map(currentNodes.map((n) => [n.id, n.position]));
|
||||
return layoutedNodes.map((node) => ({
|
||||
...node,
|
||||
position: positionMap.get(node.id) || node.position,
|
||||
}));
|
||||
});
|
||||
setEdges(layoutedEdges);
|
||||
} else {
|
||||
// No new nodes - just update data without changing positions
|
||||
setNodes((currentNodes) => {
|
||||
const positionMap = new Map(currentNodes.map((n) => [n.id, n.position]));
|
||||
// Filter to only include nodes that still exist
|
||||
const existingNodeIds = new Set(layoutedNodes.map((n) => n.id));
|
||||
|
||||
return layoutedNodes.map((node) => ({
|
||||
...node,
|
||||
position: positionMap.get(node.id) || node.position,
|
||||
}));
|
||||
});
|
||||
// Update edges without triggering re-render of nodes
|
||||
setEdges(layoutedEdges);
|
||||
}
|
||||
|
||||
// Update prev node IDs for next comparison
|
||||
prevNodeIds.current = currentNodeIds;
|
||||
}, [layoutedNodes, layoutedEdges, setNodes, setEdges]);
|
||||
|
||||
// Handle layout direction change
|
||||
@@ -154,6 +196,16 @@ function GraphCanvasInner({
|
||||
[onCreateDependency]
|
||||
);
|
||||
|
||||
// Allow any connection between different nodes
|
||||
const isValidConnection = useCallback(
|
||||
(connection: Connection | { source: string; target: string }) => {
|
||||
// Don't allow self-connections
|
||||
if (connection.source === connection.target) return false;
|
||||
return true;
|
||||
},
|
||||
[]
|
||||
);
|
||||
|
||||
// MiniMap node color based on status
|
||||
const minimapNodeColor = useCallback((node: Node<TaskNodeData>) => {
|
||||
const data = node.data as TaskNodeData | undefined;
|
||||
@@ -182,6 +234,7 @@ function GraphCanvasInner({
|
||||
onEdgesChange={onEdgesChange}
|
||||
onNodeDoubleClick={handleNodeDoubleClick}
|
||||
onConnect={handleConnect}
|
||||
isValidConnection={isValidConnection}
|
||||
nodeTypes={nodeTypes}
|
||||
edgeTypes={edgeTypes}
|
||||
fitView
|
||||
|
||||
@@ -21,6 +21,7 @@ interface GraphViewProps {
|
||||
onResumeTask?: (feature: Feature) => void;
|
||||
onUpdateFeature?: (featureId: string, updates: Partial<Feature>) => void;
|
||||
onSpawnTask?: (feature: Feature) => void;
|
||||
onDeleteTask?: (feature: Feature) => void;
|
||||
}
|
||||
|
||||
export function GraphView({
|
||||
@@ -38,6 +39,7 @@ export function GraphView({
|
||||
onResumeTask,
|
||||
onUpdateFeature,
|
||||
onSpawnTask,
|
||||
onDeleteTask,
|
||||
}: GraphViewProps) {
|
||||
const { currentProject } = useAppStore();
|
||||
|
||||
@@ -83,6 +85,34 @@ export function GraphView({
|
||||
// Handle creating a dependency via edge connection
|
||||
const handleCreateDependency = useCallback(
|
||||
async (sourceId: string, targetId: string): Promise<boolean> => {
|
||||
const sourceFeature = features.find((f) => f.id === sourceId);
|
||||
const targetFeature = features.find((f) => f.id === targetId);
|
||||
|
||||
// Debug logging
|
||||
console.log('[Dependency Check] ===========================');
|
||||
console.log('[Dependency Check] Source (prerequisite):', {
|
||||
id: sourceId,
|
||||
title: sourceFeature?.title || sourceFeature?.description?.slice(0, 50),
|
||||
dependencies: sourceFeature?.dependencies,
|
||||
});
|
||||
console.log('[Dependency Check] Target (will depend on source):', {
|
||||
id: targetId,
|
||||
title: targetFeature?.title || targetFeature?.description?.slice(0, 50),
|
||||
dependencies: targetFeature?.dependencies,
|
||||
});
|
||||
console.log(
|
||||
'[Dependency Check] Action:',
|
||||
`${targetFeature?.title || targetId} will depend on ${sourceFeature?.title || sourceId}`
|
||||
);
|
||||
console.log(
|
||||
'[Dependency Check] All features:',
|
||||
features.map((f) => ({
|
||||
id: f.id,
|
||||
title: f.title || f.description?.slice(0, 30),
|
||||
deps: f.dependencies,
|
||||
}))
|
||||
);
|
||||
|
||||
// Prevent self-dependency
|
||||
if (sourceId === targetId) {
|
||||
toast.error('A task cannot depend on itself');
|
||||
@@ -96,7 +126,18 @@ export function GraphView({
|
||||
}
|
||||
|
||||
// Check for circular dependency
|
||||
if (wouldCreateCircularDependency(features, sourceId, targetId)) {
|
||||
// This checks: if we make targetId depend on sourceId, would it create a cycle?
|
||||
// A cycle would occur if sourceId already depends on targetId (transitively)
|
||||
console.log('[Cycle Check] Checking if adding dependency would create cycle...');
|
||||
console.log(
|
||||
'[Cycle Check] Would create cycle if:',
|
||||
sourceFeature?.title || sourceId,
|
||||
'already depends on',
|
||||
targetFeature?.title || targetId
|
||||
);
|
||||
const wouldCycle = wouldCreateCircularDependency(features, sourceId, targetId);
|
||||
console.log('[Cycle Check] Result:', wouldCycle ? 'WOULD CREATE CYCLE' : 'Safe to add');
|
||||
if (wouldCycle) {
|
||||
toast.error('Cannot create circular dependency', {
|
||||
description: 'This would create a dependency cycle',
|
||||
});
|
||||
@@ -104,13 +145,12 @@ export function GraphView({
|
||||
}
|
||||
|
||||
// Get target feature and update its dependencies
|
||||
const targetFeature = features.find((f) => f.id === targetId);
|
||||
if (!targetFeature) {
|
||||
toast.error('Target task not found');
|
||||
return false;
|
||||
}
|
||||
|
||||
const currentDeps = targetFeature.dependencies || [];
|
||||
const currentDeps = (targetFeature.dependencies as string[] | undefined) || [];
|
||||
|
||||
// Add the dependency
|
||||
onUpdateFeature?.(targetId, {
|
||||
@@ -162,8 +202,38 @@ export function GraphView({
|
||||
onSpawnTask?.(feature);
|
||||
}
|
||||
},
|
||||
onDeleteTask: (featureId: string) => {
|
||||
const feature = features.find((f) => f.id === featureId);
|
||||
if (feature) {
|
||||
onDeleteTask?.(feature);
|
||||
}
|
||||
},
|
||||
onDeleteDependency: (sourceId: string, targetId: string) => {
|
||||
// Find the target feature and remove the source from its dependencies
|
||||
const targetFeature = features.find((f) => f.id === targetId);
|
||||
if (!targetFeature) return;
|
||||
|
||||
const currentDeps = (targetFeature.dependencies as string[] | undefined) || [];
|
||||
const newDeps = currentDeps.filter((depId) => depId !== sourceId);
|
||||
|
||||
onUpdateFeature?.(targetId, {
|
||||
dependencies: newDeps,
|
||||
});
|
||||
|
||||
toast.success('Dependency removed');
|
||||
},
|
||||
}),
|
||||
[features, onViewOutput, onEditFeature, onStartTask, onStopTask, onResumeTask, onSpawnTask]
|
||||
[
|
||||
features,
|
||||
onViewOutput,
|
||||
onEditFeature,
|
||||
onStartTask,
|
||||
onStopTask,
|
||||
onResumeTask,
|
||||
onSpawnTask,
|
||||
onDeleteTask,
|
||||
onUpdateFeature,
|
||||
]
|
||||
);
|
||||
|
||||
return (
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useMemo } from 'react';
|
||||
import { useCallback, useMemo, useRef } from 'react';
|
||||
import dagre from 'dagre';
|
||||
import { Node, Edge, useReactFlow } from '@xyflow/react';
|
||||
import { TaskNode, DependencyEdge } from './use-graph-nodes';
|
||||
@@ -18,6 +18,10 @@ interface UseGraphLayoutProps {
|
||||
export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) {
|
||||
const { fitView, setNodes } = useReactFlow();
|
||||
|
||||
// Cache the last computed positions to avoid recalculating layout
|
||||
const positionCache = useRef<Map<string, { x: number; y: number }>>(new Map());
|
||||
const lastStructureKey = useRef<string>('');
|
||||
|
||||
const getLayoutedElements = useCallback(
|
||||
(
|
||||
inputNodes: TaskNode[],
|
||||
@@ -48,12 +52,15 @@ export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) {
|
||||
|
||||
const layoutedNodes = inputNodes.map((node) => {
|
||||
const nodeWithPosition = dagreGraph.node(node.id);
|
||||
const position = {
|
||||
x: nodeWithPosition.x - NODE_WIDTH / 2,
|
||||
y: nodeWithPosition.y - NODE_HEIGHT / 2,
|
||||
};
|
||||
// Update cache
|
||||
positionCache.current.set(node.id, position);
|
||||
return {
|
||||
...node,
|
||||
position: {
|
||||
x: nodeWithPosition.x - NODE_WIDTH / 2,
|
||||
y: nodeWithPosition.y - NODE_HEIGHT / 2,
|
||||
},
|
||||
position,
|
||||
targetPosition: isHorizontal ? 'left' : 'top',
|
||||
sourcePosition: isHorizontal ? 'right' : 'bottom',
|
||||
} as TaskNode;
|
||||
@@ -64,13 +71,45 @@ export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) {
|
||||
[]
|
||||
);
|
||||
|
||||
// Initial layout
|
||||
// Create a stable structure key based only on node IDs (not edge changes)
|
||||
// Edges changing shouldn't trigger re-layout
|
||||
const structureKey = useMemo(() => {
|
||||
const nodeIds = nodes
|
||||
.map((n) => n.id)
|
||||
.sort()
|
||||
.join(',');
|
||||
return nodeIds;
|
||||
}, [nodes]);
|
||||
|
||||
// Initial layout - only recalculate when node structure changes (new nodes added/removed)
|
||||
const layoutedElements = useMemo(() => {
|
||||
if (nodes.length === 0) {
|
||||
positionCache.current.clear();
|
||||
lastStructureKey.current = '';
|
||||
return { nodes: [], edges: [] };
|
||||
}
|
||||
return getLayoutedElements(nodes, edges, 'LR');
|
||||
}, [nodes, edges, getLayoutedElements]);
|
||||
|
||||
// Check if structure changed (new nodes added or removed)
|
||||
const structureChanged = structureKey !== lastStructureKey.current;
|
||||
|
||||
if (structureChanged) {
|
||||
// Structure changed - run full layout
|
||||
lastStructureKey.current = structureKey;
|
||||
return getLayoutedElements(nodes, edges, 'LR');
|
||||
} else {
|
||||
// Structure unchanged - preserve cached positions, just update node data
|
||||
const layoutedNodes = nodes.map((node) => {
|
||||
const cachedPosition = positionCache.current.get(node.id);
|
||||
return {
|
||||
...node,
|
||||
position: cachedPosition || { x: 0, y: 0 },
|
||||
targetPosition: 'left',
|
||||
sourcePosition: 'right',
|
||||
} as TaskNode;
|
||||
});
|
||||
return { nodes: layoutedNodes, edges };
|
||||
}
|
||||
}, [nodes, edges, structureKey, getLayoutedElements]);
|
||||
|
||||
// Manual re-layout function
|
||||
const runLayout = useCallback(
|
||||
|
||||
@@ -25,6 +25,7 @@ export interface TaskNodeData extends Feature {
|
||||
onStopTask?: () => void;
|
||||
onResumeTask?: () => void;
|
||||
onSpawnTask?: () => void;
|
||||
onDeleteTask?: () => void;
|
||||
}
|
||||
|
||||
export type TaskNode = Node<TaskNodeData, 'task'>;
|
||||
@@ -33,6 +34,7 @@ export type DependencyEdge = Edge<{
|
||||
targetStatus: Feature['status'];
|
||||
isHighlighted?: boolean;
|
||||
isDimmed?: boolean;
|
||||
onDeleteDependency?: (sourceId: string, targetId: string) => void;
|
||||
}>;
|
||||
|
||||
export interface NodeActionCallbacks {
|
||||
@@ -42,6 +44,8 @@ export interface NodeActionCallbacks {
|
||||
onStopTask?: (featureId: string) => void;
|
||||
onResumeTask?: (featureId: string) => void;
|
||||
onSpawnTask?: (featureId: string) => void;
|
||||
onDeleteTask?: (featureId: string) => void;
|
||||
onDeleteDependency?: (sourceId: string, targetId: string) => void;
|
||||
}
|
||||
|
||||
interface UseGraphNodesProps {
|
||||
@@ -117,6 +121,9 @@ export function useGraphNodes({
|
||||
onSpawnTask: actionCallbacks?.onSpawnTask
|
||||
? () => actionCallbacks.onSpawnTask!(feature.id)
|
||||
: undefined,
|
||||
onDeleteTask: actionCallbacks?.onDeleteTask
|
||||
? () => actionCallbacks.onDeleteTask!(feature.id)
|
||||
: undefined,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -146,6 +153,7 @@ export function useGraphNodes({
|
||||
targetStatus: feature.status,
|
||||
isHighlighted: edgeIsHighlighted,
|
||||
isDimmed: edgeIsDimmed,
|
||||
onDeleteDependency: actionCallbacks?.onDeleteDependency,
|
||||
},
|
||||
};
|
||||
edgeList.push(edge);
|
||||
|
||||
@@ -212,7 +212,8 @@ export function getBlockingDependencies(feature: Feature, allFeatures: Feature[]
|
||||
|
||||
/**
|
||||
* Checks if adding a dependency from sourceId to targetId would create a circular dependency.
|
||||
* Uses DFS to detect if targetId can reach sourceId through existing dependencies.
|
||||
* When we say "targetId depends on sourceId", we add sourceId to targetId.dependencies.
|
||||
* A cycle would occur if sourceId already depends on targetId (directly or transitively).
|
||||
*
|
||||
* @param features - All features in the system
|
||||
* @param sourceId - The feature that would become a dependency (the prerequisite)
|
||||
@@ -227,22 +228,24 @@ export function wouldCreateCircularDependency(
|
||||
const featureMap = new Map(features.map((f) => [f.id, f]));
|
||||
const visited = new Set<string>();
|
||||
|
||||
function canReach(currentId: string, target: string): boolean {
|
||||
if (currentId === target) return true;
|
||||
if (visited.has(currentId)) return false;
|
||||
// Check if 'from' can reach 'to' by following dependencies
|
||||
function canReach(fromId: string, toId: string): boolean {
|
||||
if (fromId === toId) return true;
|
||||
if (visited.has(fromId)) return false;
|
||||
|
||||
visited.add(currentId);
|
||||
const feature = featureMap.get(currentId);
|
||||
visited.add(fromId);
|
||||
const feature = featureMap.get(fromId);
|
||||
if (!feature?.dependencies) return false;
|
||||
|
||||
for (const depId of feature.dependencies) {
|
||||
if (canReach(depId, target)) return true;
|
||||
if (canReach(depId, toId)) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if source can reach target through existing dependencies
|
||||
// If so, adding target -> source would create a cycle
|
||||
// We want to add: targetId depends on sourceId (sourceId -> targetId in dependency graph)
|
||||
// This would create a cycle if sourceId already depends on targetId (transitively)
|
||||
// i.e., if we can reach targetId starting from sourceId by following dependencies
|
||||
return canReach(sourceId, targetId);
|
||||
}
|
||||
|
||||
@@ -321,8 +324,10 @@ export function getAncestors(
|
||||
|
||||
/**
|
||||
* Formats ancestor context for inclusion in a task description.
|
||||
* The parent task (depth=-1) is formatted with special emphasis indicating
|
||||
* it was already completed and is provided for context only.
|
||||
*
|
||||
* @param ancestors - Array of ancestor contexts (including parent)
|
||||
* @param ancestors - Array of ancestor contexts (including parent with depth=-1)
|
||||
* @param selectedIds - Set of selected ancestor IDs to include
|
||||
* @returns Formatted markdown string with ancestor context
|
||||
*/
|
||||
@@ -333,24 +338,59 @@ export function formatAncestorContextForPrompt(
|
||||
const selectedAncestors = ancestors.filter((a) => selectedIds.has(a.id));
|
||||
if (selectedAncestors.length === 0) return '';
|
||||
|
||||
const sections = selectedAncestors.map((ancestor) => {
|
||||
const parts: string[] = [];
|
||||
const title = ancestor.title || `Task (${ancestor.id.slice(0, 8)})`;
|
||||
// Separate parent (depth=-1) from other ancestors
|
||||
const parent = selectedAncestors.find((a) => a.depth === -1);
|
||||
const otherAncestors = selectedAncestors.filter((a) => a.depth !== -1);
|
||||
|
||||
parts.push(`### ${title}`);
|
||||
const sections: string[] = [];
|
||||
|
||||
if (ancestor.description) {
|
||||
parts.push(`**Description:** ${ancestor.description}`);
|
||||
// Format parent with special emphasis
|
||||
if (parent) {
|
||||
const parentTitle = parent.title || `Task (${parent.id.slice(0, 8)})`;
|
||||
const parentParts: string[] = [];
|
||||
|
||||
parentParts.push(`## Parent Task Context (Already Completed)`);
|
||||
parentParts.push(
|
||||
`> **Note:** The following parent task has already been completed. This context is provided to help you understand the background and requirements for this sub-task. Do not re-implement the parent task - focus only on the new sub-task described below.`
|
||||
);
|
||||
parentParts.push(`### ${parentTitle}`);
|
||||
|
||||
if (parent.description) {
|
||||
parentParts.push(`**Description:** ${parent.description}`);
|
||||
}
|
||||
if (ancestor.spec) {
|
||||
parts.push(`**Specification:**\n${ancestor.spec}`);
|
||||
if (parent.spec) {
|
||||
parentParts.push(`**Specification:**\n${parent.spec}`);
|
||||
}
|
||||
if (ancestor.summary) {
|
||||
parts.push(`**Summary:** ${ancestor.summary}`);
|
||||
if (parent.summary) {
|
||||
parentParts.push(`**Summary:** ${parent.summary}`);
|
||||
}
|
||||
|
||||
return parts.join('\n\n');
|
||||
});
|
||||
sections.push(parentParts.join('\n\n'));
|
||||
}
|
||||
|
||||
return `## Ancestor Context\n\n${sections.join('\n\n---\n\n')}`;
|
||||
// Format other ancestors if any
|
||||
if (otherAncestors.length > 0) {
|
||||
const ancestorSections = otherAncestors.map((ancestor) => {
|
||||
const parts: string[] = [];
|
||||
const title = ancestor.title || `Task (${ancestor.id.slice(0, 8)})`;
|
||||
|
||||
parts.push(`### ${title}`);
|
||||
|
||||
if (ancestor.description) {
|
||||
parts.push(`**Description:** ${ancestor.description}`);
|
||||
}
|
||||
if (ancestor.spec) {
|
||||
parts.push(`**Specification:**\n${ancestor.spec}`);
|
||||
}
|
||||
if (ancestor.summary) {
|
||||
parts.push(`**Summary:** ${ancestor.summary}`);
|
||||
}
|
||||
|
||||
return parts.join('\n\n');
|
||||
});
|
||||
|
||||
sections.push(`## Additional Ancestor Context\n\n${ancestorSections.join('\n\n---\n\n')}`);
|
||||
}
|
||||
|
||||
return sections.join('\n\n---\n\n');
|
||||
}
|
||||
|
||||
@@ -3,6 +3,8 @@ import {
|
||||
resolveDependencies,
|
||||
areDependenciesSatisfied,
|
||||
getBlockingDependencies,
|
||||
wouldCreateCircularDependency,
|
||||
dependencyExists,
|
||||
} from '../src/resolver';
|
||||
import type { Feature } from '@automaker/types';
|
||||
|
||||
@@ -348,4 +350,204 @@ describe('resolver.ts', () => {
|
||||
expect(blocking).not.toContain('Dep2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('wouldCreateCircularDependency', () => {
|
||||
it('should return false for features with no existing dependencies', () => {
|
||||
const features = [createFeature('A'), createFeature('B')];
|
||||
|
||||
// Making B depend on A should not create a cycle
|
||||
expect(wouldCreateCircularDependency(features, 'A', 'B')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for valid linear dependency chain', () => {
|
||||
// A <- B <- C (C depends on B, B depends on A)
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['B'] }),
|
||||
];
|
||||
|
||||
// Making D depend on C should not create a cycle
|
||||
const featuresWithD = [...features, createFeature('D')];
|
||||
expect(wouldCreateCircularDependency(featuresWithD, 'C', 'D')).toBe(false);
|
||||
});
|
||||
|
||||
it('should detect direct circular dependency (A -> B -> A)', () => {
|
||||
// B depends on A
|
||||
const features = [createFeature('A'), createFeature('B', { dependencies: ['A'] })];
|
||||
|
||||
// Making A depend on B would create: A -> B -> A (cycle!)
|
||||
// sourceId = B (prerequisite), targetId = A (will depend on B)
|
||||
// This creates a cycle because B already depends on A
|
||||
expect(wouldCreateCircularDependency(features, 'B', 'A')).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect transitive circular dependency (A -> B -> C -> A)', () => {
|
||||
// C depends on B, B depends on A
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['B'] }),
|
||||
];
|
||||
|
||||
// Making A depend on C would create: A -> C -> B -> A (cycle!)
|
||||
// sourceId = C (prerequisite), targetId = A (will depend on C)
|
||||
expect(wouldCreateCircularDependency(features, 'C', 'A')).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect cycle in complex graph', () => {
|
||||
// Graph: A <- B, A <- C, B <- C (C depends on both A and B, B depends on A)
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['A', 'B'] }),
|
||||
];
|
||||
|
||||
// Making A depend on C would create a cycle
|
||||
expect(wouldCreateCircularDependency(features, 'C', 'A')).toBe(true);
|
||||
|
||||
// Making B depend on C would also create a cycle
|
||||
expect(wouldCreateCircularDependency(features, 'C', 'B')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for parallel branches', () => {
|
||||
// A <- B, A <- C (B and C both depend on A, but not on each other)
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['A'] }),
|
||||
];
|
||||
|
||||
// Making B depend on C should be fine (no cycle)
|
||||
expect(wouldCreateCircularDependency(features, 'C', 'B')).toBe(false);
|
||||
|
||||
// Making C depend on B should also be fine
|
||||
expect(wouldCreateCircularDependency(features, 'B', 'C')).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle self-dependency check', () => {
|
||||
const features = [createFeature('A')];
|
||||
|
||||
// A depending on itself would be a trivial cycle
|
||||
expect(wouldCreateCircularDependency(features, 'A', 'A')).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle feature not in list', () => {
|
||||
const features = [createFeature('A')];
|
||||
|
||||
// Non-existent source - should return false (no path exists)
|
||||
expect(wouldCreateCircularDependency(features, 'NonExistent', 'A')).toBe(false);
|
||||
|
||||
// Non-existent target - should return false
|
||||
expect(wouldCreateCircularDependency(features, 'A', 'NonExistent')).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle empty features list', () => {
|
||||
const features: Feature[] = [];
|
||||
|
||||
expect(wouldCreateCircularDependency(features, 'A', 'B')).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle longer transitive chains', () => {
|
||||
// A <- B <- C <- D <- E
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['B'] }),
|
||||
createFeature('D', { dependencies: ['C'] }),
|
||||
createFeature('E', { dependencies: ['D'] }),
|
||||
];
|
||||
|
||||
// Making A depend on E would create a 5-node cycle
|
||||
expect(wouldCreateCircularDependency(features, 'E', 'A')).toBe(true);
|
||||
|
||||
// Making B depend on E would create a 4-node cycle
|
||||
expect(wouldCreateCircularDependency(features, 'E', 'B')).toBe(true);
|
||||
|
||||
// Making E depend on A is fine (already exists transitively, but adding explicit is ok)
|
||||
// Wait, E already depends on A transitively. Let's add F instead
|
||||
const featuresWithF = [...features, createFeature('F')];
|
||||
expect(wouldCreateCircularDependency(featuresWithF, 'E', 'F')).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle diamond dependency pattern', () => {
|
||||
// A
|
||||
// / \
|
||||
// B C
|
||||
// \ /
|
||||
// D
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['A'] }),
|
||||
createFeature('D', { dependencies: ['B', 'C'] }),
|
||||
];
|
||||
|
||||
// Making A depend on D would create a cycle through both paths
|
||||
expect(wouldCreateCircularDependency(features, 'D', 'A')).toBe(true);
|
||||
|
||||
// Making B depend on D would create a cycle
|
||||
expect(wouldCreateCircularDependency(features, 'D', 'B')).toBe(true);
|
||||
|
||||
// Adding E that depends on D should be fine
|
||||
const featuresWithE = [...features, createFeature('E')];
|
||||
expect(wouldCreateCircularDependency(featuresWithE, 'D', 'E')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('dependencyExists', () => {
|
||||
it('should return false when target has no dependencies', () => {
|
||||
const features = [createFeature('A'), createFeature('B')];
|
||||
|
||||
expect(dependencyExists(features, 'A', 'B')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true when direct dependency exists', () => {
|
||||
const features = [createFeature('A'), createFeature('B', { dependencies: ['A'] })];
|
||||
|
||||
expect(dependencyExists(features, 'A', 'B')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for reverse direction', () => {
|
||||
const features = [createFeature('A'), createFeature('B', { dependencies: ['A'] })];
|
||||
|
||||
// B depends on A, but A does not depend on B
|
||||
expect(dependencyExists(features, 'B', 'A')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for transitive dependencies', () => {
|
||||
// This function only checks direct dependencies, not transitive
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B', { dependencies: ['A'] }),
|
||||
createFeature('C', { dependencies: ['B'] }),
|
||||
];
|
||||
|
||||
// C depends on B which depends on A, but C doesn't directly depend on A
|
||||
expect(dependencyExists(features, 'A', 'C')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true for one of multiple dependencies', () => {
|
||||
const features = [
|
||||
createFeature('A'),
|
||||
createFeature('B'),
|
||||
createFeature('C', { dependencies: ['A', 'B'] }),
|
||||
];
|
||||
|
||||
expect(dependencyExists(features, 'A', 'C')).toBe(true);
|
||||
expect(dependencyExists(features, 'B', 'C')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when target feature does not exist', () => {
|
||||
const features = [createFeature('A')];
|
||||
|
||||
expect(dependencyExists(features, 'A', 'NonExistent')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false for empty dependencies array', () => {
|
||||
const features = [createFeature('A'), createFeature('B', { dependencies: [] })];
|
||||
|
||||
expect(dependencyExists(features, 'A', 'B')).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user