feat: task dependency graph view#222
Conversation
Add a new interactive graph view alongside the kanban board for visualizing task dependencies. The graph view uses React Flow with dagre auto-layout to display tasks as nodes connected by dependency edges. Key features: - Toggle between kanban and graph view via new control buttons - Custom TaskNode component matching existing card styling/themes - Animated edges that flow when tasks are in progress - Status-aware node colors (backlog, in-progress, waiting, verified) - Blocked tasks show lock icon with dependency count tooltip - MiniMap for navigation in large graphs - Zoom, pan, fit-view, and lock controls - Horizontal/vertical layout options via dagre - Click node to view details, double-click to edit - Respects all 32 themes via CSS variables - Reduced motion support for animations New dependencies: @xyflow/react, dagre
Override React Flow's default node styling (white background) with transparent to allow the TaskNode component's bg-card class to show through with the correct theme colors.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a React Flow–based Graph view to the board: new GraphView/GraphCanvas, custom TaskNode/DependencyEdge components, Dagre layout hooks, view-mode toggle in board controls with persisted state, styles for the graph, and test/navigation wait adjustments from networkidle to load. Changes
Sequence DiagramsequenceDiagram
participant User
participant BoardView
participant GraphView
participant useGraphNodes
participant useGraphLayout
participant GraphCanvas
participant ReactFlow
User->>BoardView: toggle view -> 'graph'
BoardView->>GraphView: render(features, callbacks)
GraphView->>GraphView: filter features by worktree
GraphView->>useGraphNodes: build nodes & edges
useGraphNodes-->>GraphView: return nodes, edges
GraphView->>GraphCanvas: pass nodes, edges, handlers
GraphCanvas->>useGraphLayout: request layout (LR/TB)
useGraphLayout-->>GraphCanvas: layouted nodes & edges
GraphCanvas->>ReactFlow: render graph with TaskNode/DependencyEdge
ReactFlow-->>GraphCanvas: user interactions (click/dblclick)
GraphCanvas->>GraphView: emit node events
GraphView->>User: call onViewOutput / onEditFeature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
Comment |
Summary of ChangesHello @JBotwina, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the task management interface by introducing a dynamic and interactive dependency graph view. This new visualization allows users to understand complex task relationships and their current states at a glance, improving project oversight and planning. It provides a powerful alternative to the Kanban board for scenarios where task interdependencies are crucial. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable new feature: a task dependency graph view. The implementation is robust, leveraging @xyflow/react for the graph visualization and dagre for automatic layout, which is a great choice. The new components are well-structured and the addition of theming and a prefers-reduced-motion media query shows great attention to detail and user experience. My review comments focus on minor improvements to enhance maintainability and code quality, such as removing unused code, avoiding hardcoded values, and opportunities for code deduplication.
| Maximize2, | ||
| Lock, | ||
| Unlock, | ||
| GitBranch, |
| <DropdownMenuItem className="text-xs"> | ||
| <Eye className="w-3 h-3 mr-2" /> | ||
| View Details | ||
| </DropdownMenuItem> | ||
| {data.status === 'backlog' && !data.isBlocked && ( | ||
| <DropdownMenuItem className="text-xs"> | ||
| <Play className="w-3 h-3 mr-2" /> | ||
| Start Task | ||
| </DropdownMenuItem> | ||
| )} | ||
| {data.isRunning && ( | ||
| <DropdownMenuItem className="text-xs text-[var(--status-error)]"> | ||
| <Pause className="w-3 h-3 mr-2" /> | ||
| Stop Task | ||
| </DropdownMenuItem> | ||
| )} | ||
| <DropdownMenuSeparator /> | ||
| <DropdownMenuItem className="text-xs"> | ||
| <GitBranch className="w-3 h-3 mr-2" /> | ||
| View Branch | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
These dropdown menu items are currently placeholders without any associated actions. This can be confusing for users and adds dead code to the component. If implementing these actions is out of scope for this pull request, I recommend removing them for now. You could leave a TODO comment to track this for future implementation.
{/* TODO: Implement node actions like View Details, Start/Stop Task, and View Branch. */}
| case 'waiting_approval': | ||
| return 'var(--status-waiting)'; | ||
| default: | ||
| if (data?.isBlocked) return 'rgb(249, 115, 22)'; // orange-500 |
There was a problem hiding this comment.
The color for blocked nodes in the minimap is hardcoded as rgb(249, 115, 22). This is inconsistent with the theming approach used elsewhere, which relies on CSS variables. Hardcoding colors can lead to maintenance issues and inconsistencies if the theme is updated.
I suggest defining a CSS variable, for example --status-blocked, and using it here. This would also allow you to replace the text-orange-500 and bg-orange-500/20 classes in task-node.tsx and graph-legend.tsx for better theme consistency.
| if (data?.isBlocked) return 'rgb(249, 115, 22)'; // orange-500 | |
| if (data?.isBlocked) return 'var(--status-blocked)'; // orange-500 |
| const filteredFeatures = useMemo(() => { | ||
| const effectiveBranch = currentWorktreeBranch; | ||
|
|
||
| return features.filter((f) => { | ||
| // Skip completed features (they're in archive) | ||
| if (f.status === 'completed') return false; | ||
|
|
||
| const featureBranch = f.branchName; | ||
|
|
||
| if (!featureBranch) { | ||
| // No branch assigned - show only on primary worktree | ||
| return currentWorktreePath === null; | ||
| } else if (effectiveBranch === null) { | ||
| // Viewing main but branch not initialized | ||
| return projectPath | ||
| ? useAppStore.getState().isPrimaryWorktreeBranch(projectPath, featureBranch) | ||
| : false; | ||
| } else { | ||
| // Match by branch name | ||
| return featureBranch === effectiveBranch; | ||
| } | ||
| }); | ||
| }, [features, currentWorktreePath, currentWorktreeBranch, projectPath]); |
There was a problem hiding this comment.
The feature filtering logic in this useMemo hook appears to be a duplication of the logic used in the Kanban board view. To improve maintainability and ensure consistency between the two views, this logic should be extracted into a single, reusable utility function. This will prevent potential future bugs where one view's filtering logic is updated but the other is missed.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
apps/ui/src/components/views/graph-view/components/graph-legend.tsx (1)
37-42: Consider using a CSS variable for the "Blocked" status color.Other statuses use theme CSS variables (
--status-*), but "Blocked" uses hardcodedtext-orange-500. For theme consistency, consider adding a--status-blockedCSS variable.apps/ui/src/components/views/graph-view/components/graph-controls.tsx (1)
10-10: Remove unused import.
GitBranchis imported but not used in this component.🔎 Proposed fix
import { ZoomIn, ZoomOut, Maximize2, Lock, Unlock, - GitBranch, ArrowRight, ArrowDown, } from 'lucide-react';apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts (2)
49-60: Type handling for position properties could be improved.The
targetPositionandsourcePositionare assigned as string literals, but React Flow'sNodetype expectsPositionenum values. While this works at runtime due to theas TaskNodeassertion, consider using thePositionenum for type safety.🔎 Proposed fix
+import { Node, Edge, useReactFlow, Position } from '@xyflow/react'; ... const layoutedNodes = inputNodes.map((node) => { const nodeWithPosition = dagreGraph.node(node.id); return { ...node, position: { x: nodeWithPosition.x - NODE_WIDTH / 2, y: nodeWithPosition.y - NODE_HEIGHT / 2, }, - targetPosition: isHorizontal ? 'left' : 'top', - sourcePosition: isHorizontal ? 'right' : 'bottom', + targetPosition: isHorizontal ? Position.Left : Position.Top, + sourcePosition: isHorizontal ? Position.Right : Position.Bottom, } as TaskNode; });
76-86: The setTimeout for fitView is a common workaround but fragile.The 50ms delay works but is timing-dependent. Consider using
requestAnimationFrameor React Flow's callback options if available for more reliable behavior.🔎 Alternative approach
const runLayout = useCallback( (direction: 'LR' | 'TB' = 'LR') => { const { nodes: layoutedNodes } = getLayoutedElements(nodes, edges, direction); setNodes(layoutedNodes); - // Fit view after layout with a small delay to allow DOM updates - setTimeout(() => { - fitView({ padding: 0.2, duration: 300 }); - }, 50); + // Use requestAnimationFrame for more reliable timing + requestAnimationFrame(() => { + requestAnimationFrame(() => { + fitView({ padding: 0.2, duration: 300 }); + }); + }); }, [nodes, edges, getLayoutedElements, setNodes, fitView] );apps/ui/src/components/views/board-view/board-controls.tsx (1)
40-51: Addtype="button"and accessibility attributes to toggle buttons.The native
<button>elements are missingtype="button", which could cause unintended form submissions if these controls are ever placed inside a form. Additionally, for toggle buttons, addingaria-pressedimproves screen reader accessibility.Suggested improvement
<button + type="button" onClick={() => onBoardViewModeChange('kanban')} className={cn( 'p-2 rounded-l-lg transition-colors', boardViewMode === 'kanban' ? 'bg-brand-500/20 text-brand-500' : 'text-muted-foreground hover:text-foreground hover:bg-accent' )} + aria-pressed={boardViewMode === 'kanban'} data-testid="view-mode-kanban" >Apply the same pattern to the graph toggle button (lines 59-70).
Also applies to: 59-70
apps/ui/src/components/views/graph-view/graph-canvas.tsx (2)
64-72: Potential unnecessary state updates on initial render.The
useEffectwill trigger immediately after mount sincelayoutedNodesandlayoutedEdgesare the same values used to initializeuseNodesStateanduseEdgesState. This causes an extra render cycle on mount. Consider using a ref to track if this is the first render, or restructure to avoid the redundant initial update.Option: Skip initial update
+import { useCallback, useState, useEffect, useRef } from 'react'; ... + const isInitialMount = useRef(true); + // Update nodes/edges when features change useEffect(() => { + if (isInitialMount.current) { + isInitialMount.current = false; + return; + } setNodes(layoutedNodes); setEdges(layoutedEdges); }, [layoutedNodes, layoutedEdges, setNodes, setEdges]);
99-116: Use a CSS variable for the blocked state color to maintain theme consistency.Line 112 uses a hardcoded RGB value
'rgb(249, 115, 22)'while all other colors in this function use CSS variables. This could cause visual inconsistency across themes.Suggested fix
default: - if (data?.isBlocked) return 'rgb(249, 115, 22)'; // orange-500 + if (data?.isBlocked) return 'var(--status-blocked, rgb(249, 115, 22))'; if (data?.error) return 'var(--status-error)'; return 'var(--muted-foreground)';Alternatively, define
--status-blockedin your CSS theme files if it doesn't already exist.apps/ui/src/components/views/graph-view/components/dependency-edge.tsx (2)
7-10: Consider importing the edge data type from the hooks module to avoid duplication.The
DependencyEdgeDatainterface duplicates the type structure already defined inuse-graph-nodes.ts(DependencyEdgetype). This creates a maintenance burden where changes need to be made in two places.Suggested approach
-import { Feature } from '@/store/app-store'; +import type { DependencyEdge as DependencyEdgeType } from '../hooks'; +import type { Feature } from '@/store/app-store'; -export interface DependencyEdgeData { - sourceStatus: Feature['status']; - targetStatus: Feature['status']; -} +// Re-export for consumers that need the data type +export type DependencyEdgeData = DependencyEdgeType extends { data: infer D } ? D : never;Or extract the shared data type to a common types file and import it in both locations.
21-27: Redundant return case can be simplified.The
backlogcase (lines 22-23) returns the same value as the default case (line 26). While this may be intentional for documentation purposes, it could be simplified.Optional simplification
- // If target is blocked (in backlog with incomplete deps) - if (targetStatus === 'backlog') { - return 'var(--border)'; - } - // Default + // Default (including backlog/blocked state) return 'var(--border)';apps/ui/src/components/views/graph-view/components/task-node.tsx (1)
27-29: Consider using the exportedTaskNodetype from the hooks module.The
TaskNodePropstype is defined locally extendingNodeProps. Sinceuse-graph-nodes.tsalready exportsTaskNode = Node<TaskNodeData, 'task'>, you could leverage that type for better consistency:import type { TaskNode } from '../hooks/use-graph-nodes'; import type { NodeProps } from '@xyflow/react'; type TaskNodeProps = NodeProps<TaskNode>;This ensures the node type string
'task'is consistently typed across the codebase.apps/ui/src/components/views/graph-view/graph-view.tsx (1)
30-53: Consider simplifying the effectiveBranch variable.The
effectiveBranchvariable on line 32 is simply assigned fromcurrentWorktreeBranchwithout any transformation. You could usecurrentWorktreeBranchdirectly to reduce an unnecessary indirection.🔎 Suggested simplification
const filteredFeatures = useMemo(() => { - const effectiveBranch = currentWorktreeBranch; - return features.filter((f) => { // Skip completed features (they're in archive) if (f.status === 'completed') return false; const featureBranch = f.branchName; if (!featureBranch) { // No branch assigned - show only on primary worktree return currentWorktreePath === null; - } else if (effectiveBranch === null) { + } else if (currentWorktreeBranch === null) { // Viewing main but branch not initialized return projectPath ? useAppStore.getState().isPrimaryWorktreeBranch(projectPath, featureBranch) : false; } else { // Match by branch name - return featureBranch === effectiveBranch; + return featureBranch === currentWorktreeBranch; } }); }, [features, currentWorktreePath, currentWorktreeBranch, projectPath]);apps/ui/package.json (1)
70-70: Consider evaluating modern alternatives to dagre for graph layout.The dagre library (v0.8.5) is stable and widely adopted for graph auto-layout, but has been unmaintained since ~2017. While no security vulnerabilities are present, consider evaluating
@dagrejs/dagre(community fork) orelkjsas alternatives in future iterations if maintenance or additional features become necessary.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
apps/ui/package.jsonapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-controls.tsxapps/ui/src/components/views/graph-view/components/dependency-edge.tsxapps/ui/src/components/views/graph-view/components/graph-controls.tsxapps/ui/src/components/views/graph-view/components/graph-legend.tsxapps/ui/src/components/views/graph-view/components/index.tsapps/ui/src/components/views/graph-view/components/task-node.tsxapps/ui/src/components/views/graph-view/graph-canvas.tsxapps/ui/src/components/views/graph-view/graph-view.tsxapps/ui/src/components/views/graph-view/hooks/index.tsapps/ui/src/components/views/graph-view/hooks/use-graph-layout.tsapps/ui/src/components/views/graph-view/hooks/use-graph-nodes.tsapps/ui/src/components/views/graph-view/index.tsapps/ui/src/store/app-store.tsapps/ui/src/styles/global.css
🧰 Additional context used
🧬 Code graph analysis (7)
apps/ui/src/components/views/board-view/board-controls.tsx (2)
apps/ui/src/store/app-store.ts (1)
BoardViewMode(50-50)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/ui/src/components/views/graph-view/components/dependency-edge.tsx (6)
apps/ui/src/store/app-store.ts (1)
Feature(256-272)apps/ui/src/components/views/graph-view/components/index.ts (1)
DependencyEdge(2-2)apps/ui/src/components/views/graph-view/hooks/index.ts (1)
DependencyEdge(1-1)apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts (1)
DependencyEdge(13-13)apps/ui/src/components/views/graph-view/index.ts (2)
DependencyEdge(3-3)DependencyEdge(4-4)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/ui/src/components/views/graph-view/components/task-node.tsx (2)
apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts (2)
TaskNodeData(6-10)TaskNode(12-12)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts (3)
apps/ui/src/store/app-store.ts (1)
Feature(256-272)apps/ui/src/components/views/graph-view/components/task-node.tsx (1)
TaskNode(75-248)apps/ui/src/components/views/graph-view/components/dependency-edge.tsx (1)
DependencyEdge(29-115)
apps/ui/src/components/views/graph-view/graph-view.tsx (2)
apps/ui/src/store/app-store.ts (1)
Feature(256-272)apps/ui/src/components/views/graph-view/graph-canvas.tsx (1)
GraphCanvas(168-174)
apps/ui/src/components/views/board-view.tsx (3)
apps/ui/src/components/views/board-view/kanban-board.tsx (1)
KanbanBoard(53-228)apps/ui/src/components/views/graph-view/graph-view.tsx (1)
GraphView(16-89)apps/ui/src/components/views/graph-view/index.ts (1)
GraphView(1-1)
apps/ui/src/components/views/graph-view/components/graph-legend.tsx (3)
apps/ui/src/components/views/graph-view/components/index.ts (1)
GraphLegend(4-4)apps/ui/src/components/views/graph-view/index.ts (1)
GraphLegend(3-3)apps/ui/src/lib/utils.ts (1)
cn(5-7)
🪛 GitHub Actions: Format Check
apps/ui/src/components/views/graph-view/index.ts
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
apps/ui/src/components/views/graph-view/hooks/index.ts
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
apps/ui/src/components/views/graph-view/components/dependency-edge.tsx
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
apps/ui/src/components/views/graph-view/components/graph-controls.tsx
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
apps/ui/src/components/views/graph-view/components/task-node.tsx
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
apps/ui/src/components/views/graph-view/components/graph-legend.tsx
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
🔇 Additional comments (20)
apps/ui/src/styles/global.css (2)
892-1060: Well-structured theme-aware graph styling with good accessibility support.The CSS additions are cleanly organized with:
- Proper CSS variable usage for theme consistency
- Reduced motion media query for accessibility
- Retro theme overrides maintaining design consistency
- Clear separation of concerns (minimap, edges, nodes, handles, panels)
1032-1035: Clarify the attribution usage context.Hiding the React Flow attribution depends on the project's use: for commercial use, a React Flow Pro subscription is expected; for non-commercial applications, it may be hidden without subscribing. Confirm whether this project qualifies as commercial or non-commercial, then either add the attribution back (if commercial and unsubscribed) or document the subscription status.
apps/ui/src/components/views/graph-view/hooks/index.ts (1)
1-2: LGTM - Clean barrel export for graph hooks.The re-exports consolidate the graph hook API surface appropriately. Note the Prettier warning from CI - run
prettier --writeto fix formatting.apps/ui/src/components/views/graph-view/components/index.ts (1)
1-4: LGTM - Standard barrel export pattern.Clean re-exports for the graph view components.
apps/ui/src/components/views/graph-view/components/graph-controls.tsx (1)
23-144: Well-structured controls component with good UX patterns.The component properly:
- Uses
useReactFlowhook for zoom/pan operations with animation duration- Provides clear visual feedback for active layout direction and lock state
- Groups related controls with dividers
- Includes tooltips for discoverability
apps/ui/src/components/views/board-view.tsx (1)
999-1042: Clean view mode integration with appropriate prop passing.The conditional rendering correctly:
- Passes full
hookFeaturesto GraphView (which handles its own filtering internally)- Maintains all necessary callbacks for KanbanBoard
- GraphView receives the essential props for node interactions
apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts (1)
18-93: Solid dagre layout implementation.The hook properly:
- Memoizes the layout computation to avoid unnecessary recalculations
- Provides both initial layout and manual re-layout capability
- Uses appropriate dagre graph settings for readable DAG visualization
apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts (2)
24-79: Well-implemented feature-to-graph transformation.The hook correctly:
- Creates a feature map for efficient dependency lookups
- Guards against missing dependencies (line 56 check)
- Enriches node data with
isBlocked,isRunning, andblockingDependencies- Animates edges when either source or target is running
- Uses appropriate memoization
6-13: Clean type definitions extending the Feature interface.The
TaskNodeDatainterface properly extendsFeaturewith graph-specific properties, and the type aliases forTaskNodeandDependencyEdgeprovide good type safety for React Flow integration.apps/ui/src/store/app-store.ts (1)
50-51: LGTM - BoardViewMode state implementation is well-integrated.The new
BoardViewModetype, state property, action, and persistence are correctly implemented following the existing patterns in the store. The default value of'kanban'ensures backward compatibility.Also applies to: 440-441, 704-704, 908-908, 1459-1459, 2667-2667
apps/ui/src/components/views/graph-view/components/dependency-edge.tsx (1)
29-115: Well-structured edge component with good performance optimizations.The component is properly memoized, handles multiple visual states cleanly, and the layered edge approach (background + main) provides good visibility. The animation support with reduced motion considerations (via CSS) is a nice touch for accessibility.
apps/ui/src/components/views/graph-view/graph-canvas.tsx (1)
135-135: Clarify React Flow attribution expectations.While you're allowed to hide the attribution in any application, whether commercial or not, React Flow Pro is suggested for commercial use to support the library's ongoing maintenance. If the project is making money from React Flow, consider supporting the library before hiding the attribution.
Likely an incorrect or invalid review comment.
apps/ui/src/components/views/graph-view/components/task-node.tsx (4)
1-26: Run Prettier to fix formatting.The pipeline indicates a code style issue found by Prettier on this file. Run
prettier --writeto fix the formatting.
96-104: Well-structured card container with good theme integration.The card container properly uses theme CSS variables for styling, handles multiple states (selected, running, error), and applies appropriate visual feedback. The use of
cn()for conditional class composition is clean.
234-245: LGTM - Source handle with completion state styling.The source handle correctly applies visual feedback for completed/verified tasks using the success color, providing clear visual indication that a task's dependencies are satisfied.
211-221: Bothanimate-progress-indeterminateandanimate-pulse-subtleare already properly defined inapps/ui/src/styles/global.css. No action needed.The
animate-progress-indeterminateclass has a corresponding@keyframes progress-indeterminatedefinition (1.5s ease-in-out infinite animation), andanimate-pulse-subtlehas a corresponding@keyframes pulse-subtledefinition (2s ease-in-out infinite animation). Both animation classes are accessible and correctly configured.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/graph-view/graph-view.tsx (3)
1-14: LGTM - Clean interface definition.The imports and interface are well-defined with appropriate prop types. The interface clearly documents what the component expects.
55-75: LGTM - Event handlers are correctly memoized.The
useCallbackhooks have correct dependency arrays, and the feature lookup logic is straightforward. For typical feature counts, the linear search is acceptable.
77-88: LGTM - Clean delegation to GraphCanvas.The component properly delegates rendering to
GraphCanvaswith filtered features and handlers. The container div withoverflow-hidden relativeis appropriate for containing the React Flow canvas.apps/ui/package.json (1)
66-66: No issues identified. @xyflow/react@^12.10.0 is compatible with React 19.
| "@playwright/test": "^1.57.0", | ||
| "@tailwindcss/vite": "^4.1.18", | ||
| "@tanstack/router-plugin": "^1.141.7", | ||
| "@types/dagre": "^0.7.53", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat apps/ui/package.json | grep -A 5 -B 5 "dagre"Repository: AutoMaker-Org/automaker
Length of output: 766
Version mismatch: @types/dagre 0.7.53 does not match dagre 0.8.5.
The type definitions are for dagre 0.7, but the project uses dagre 0.8.5. There are no published @types/dagre versions for the 0.8.x series. Consider either downgrading dagre to a 0.7.x version or finding an alternative type definition source.
🤖 Prompt for AI Agents
In apps/ui/package.json around line 100, the devDependency "@types/dagre":
"0.7.53" does not match the runtime dependency dagre 0.8.5; fix by making the
versions compatible: either downgrade dagre to a 0.7.x release to match the
installed @types, or remove the @types/dagre entry and add appropriate type
support for 0.8.x (for example add a local declaration file like declare module
"dagre" or install/point to an alternative type package) and then run npm/yarn
install to update lockfile; update package.json accordingly.
| @@ -0,0 +1,115 @@ | |||
| import { memo } from 'react'; | |||
There was a problem hiding this comment.
Fix Prettier formatting issues flagged by CI.
The pipeline indicates code style issues found by Prettier. Run prettier --write on this file to fix formatting.
🧰 Tools
🪛 GitHub Actions: Format Check
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
🤖 Prompt for AI Agents
In apps/ui/src/components/views/graph-view/components/dependency-edge.tsx around
line 1, Prettier formatting violations were reported; run your project's
Prettier formatter (e.g., run prettier --write
apps/ui/src/components/views/graph-view/components/dependency-edge.tsx) or apply
the repo's formatting rules to this file so import spacing, semicolons, and line
breaks match the configured style, then stage the updated file and push the
changes.
| { | ||
| icon: Pause, | ||
| label: 'Waiting', | ||
| colorClass: 'text-[var(--status-waiting)]', | ||
| bgClass: 'bg-[var(--status-warning)]/20', | ||
| }, |
There was a problem hiding this comment.
Inconsistent CSS variable usage for "Waiting" status.
The colorClass uses --status-waiting but bgClass uses --status-warning. This appears unintentional.
🔎 Proposed fix
{
icon: Pause,
label: 'Waiting',
colorClass: 'text-[var(--status-waiting)]',
- bgClass: 'bg-[var(--status-warning)]/20',
+ bgClass: 'bg-[var(--status-waiting)]/20',
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| icon: Pause, | |
| label: 'Waiting', | |
| colorClass: 'text-[var(--status-waiting)]', | |
| bgClass: 'bg-[var(--status-warning)]/20', | |
| }, | |
| { | |
| icon: Pause, | |
| label: 'Waiting', | |
| colorClass: 'text-[var(--status-waiting)]', | |
| bgClass: 'bg-[var(--status-waiting)]/20', | |
| }, |
🤖 Prompt for AI Agents
In apps/ui/src/components/views/graph-view/components/graph-legend.tsx around
lines 25 to 30, the "Waiting" legend entry uses colorClass: '--status-waiting'
but bgClass: '--status-warning', causing inconsistent CSS variable usage; update
bgClass to use the matching '--status-waiting' variable (preserving the existing
opacity / format) so both colorClass and bgClass reference the same status
variable.
| {/* Actions dropdown */} | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-background/50" | ||
| > | ||
| <MoreHorizontal className="w-4 h-4" /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end" className="w-40"> | ||
| <DropdownMenuItem className="text-xs"> | ||
| <Eye className="w-3 h-3 mr-2" /> | ||
| View Details | ||
| </DropdownMenuItem> | ||
| {data.status === 'backlog' && !data.isBlocked && ( | ||
| <DropdownMenuItem className="text-xs"> | ||
| <Play className="w-3 h-3 mr-2" /> | ||
| Start Task | ||
| </DropdownMenuItem> | ||
| )} | ||
| {data.isRunning && ( | ||
| <DropdownMenuItem className="text-xs text-[var(--status-error)]"> | ||
| <Pause className="w-3 h-3 mr-2" /> | ||
| Stop Task | ||
| </DropdownMenuItem> | ||
| )} | ||
| <DropdownMenuSeparator /> | ||
| <DropdownMenuItem className="text-xs"> | ||
| <GitBranch className="w-3 h-3 mr-2" /> | ||
| View Branch | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> |
There was a problem hiding this comment.
Dropdown menu items lack onClick handlers.
The dropdown menu items are rendered but have no onClick handlers attached, making them non-functional. Users will see interactive-looking options that do nothing when clicked.
If these actions are intended to be wired up later, consider either:
- Adding TODO comments to track the missing handlers
- Disabling the items until implemented
- Wiring up the handlers now using callbacks passed through
data
Also, the "View Branch" option (lines 190-193) is always visible even when data.branchName is undefined, which may confuse users.
🔎 Suggested fix for View Branch visibility
<DropdownMenuSeparator />
+ {data.branchName && (
<DropdownMenuItem className="text-xs">
<GitBranch className="w-3 h-3 mr-2" />
View Branch
</DropdownMenuItem>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {/* Actions dropdown */} | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| className="h-6 w-6 p-0 hover:bg-background/50" | |
| > | |
| <MoreHorizontal className="w-4 h-4" /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent align="end" className="w-40"> | |
| <DropdownMenuItem className="text-xs"> | |
| <Eye className="w-3 h-3 mr-2" /> | |
| View Details | |
| </DropdownMenuItem> | |
| {data.status === 'backlog' && !data.isBlocked && ( | |
| <DropdownMenuItem className="text-xs"> | |
| <Play className="w-3 h-3 mr-2" /> | |
| Start Task | |
| </DropdownMenuItem> | |
| )} | |
| {data.isRunning && ( | |
| <DropdownMenuItem className="text-xs text-[var(--status-error)]"> | |
| <Pause className="w-3 h-3 mr-2" /> | |
| Stop Task | |
| </DropdownMenuItem> | |
| )} | |
| <DropdownMenuSeparator /> | |
| <DropdownMenuItem className="text-xs"> | |
| <GitBranch className="w-3 h-3 mr-2" /> | |
| View Branch | |
| </DropdownMenuItem> | |
| </DropdownMenuContent> | |
| </DropdownMenu> | |
| {/* Actions dropdown */} | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| className="h-6 w-6 p-0 hover:bg-background/50" | |
| > | |
| <MoreHorizontal className="w-4 h-4" /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent align="end" className="w-40"> | |
| <DropdownMenuItem className="text-xs"> | |
| <Eye className="w-3 h-3 mr-2" /> | |
| View Details | |
| </DropdownMenuItem> | |
| {data.status === 'backlog' && !data.isBlocked && ( | |
| <DropdownMenuItem className="text-xs"> | |
| <Play className="w-3 h-3 mr-2" /> | |
| Start Task | |
| </DropdownMenuItem> | |
| )} | |
| {data.isRunning && ( | |
| <DropdownMenuItem className="text-xs text-[var(--status-error)]"> | |
| <Pause className="w-3 h-3 mr-2" /> | |
| Stop Task | |
| </DropdownMenuItem> | |
| )} | |
| <DropdownMenuSeparator /> | |
| {data.branchName && ( | |
| <DropdownMenuItem className="text-xs"> | |
| <GitBranch className="w-3 h-3 mr-2" /> | |
| View Branch | |
| </DropdownMenuItem> | |
| )} | |
| </DropdownMenuContent> | |
| </DropdownMenu> |
🤖 Prompt for AI Agents
In apps/ui/src/components/views/graph-view/components/task-node.tsx around lines
161 to 195, the DropdownMenuItem entries are missing onClick handlers (making
options non-functional) and the "View Branch" item is shown even when
data.branchName is undefined; add onClick handlers for each actionable item by
calling callbacks provided via props/data (e.g., onViewDetails, onStartTask,
onStopTask, onViewBranch) or, if those callbacks aren’t available yet, disable
the items and add concise TODO comments pointing to the missing handlers;
additionally, wrap the "View Branch" item in a conditional so it only renders
when data.branchName is truthy (or render a disabled item with a tooltip when
absent).
| export { GraphView } from './graph-view'; | ||
| export { GraphCanvas } from './graph-canvas'; | ||
| export { TaskNode, DependencyEdge, GraphControls, GraphLegend } from './components'; | ||
| export { useGraphNodes, useGraphLayout, type TaskNode as TaskNodeType, type DependencyEdge as DependencyEdgeType } from './hooks'; |
There was a problem hiding this comment.
Fix Prettier formatting issues flagged by CI.
The pipeline indicates code style issues found by Prettier. Run prettier --write on this file to fix formatting.
🧰 Tools
🪛 GitHub Actions: Format Check
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix this file.
🤖 Prompt for AI Agents
In apps/ui/src/components/views/graph-view/index.ts around lines 1 to 4,
Prettier formatting issues were flagged by CI; run the project's Prettier
formatter on this file (e.g., prettier --write
apps/ui/src/components/views/graph-view/index.ts) or apply the repository's
formatting rules via your editor/IDE to normalize spacing and trailing newlines
so the export lines conform to the project's style, then re-run lint/CI and
commit the formatted file.
…/task-dependency-graph-iPz1k
- Changed instances of `waitForLoadState('networkidle')` to `waitForLoadState('load')` across multiple test files and utility functions to improve test reliability in applications with persistent connections.
- Added documentation to the e2e testing guide explaining the rationale behind using 'load' state instead of 'networkidle' to prevent timeouts and flaky tests.
Summary
Test plan
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.