feat(agentflow): enhance validation and feedback mechanisms (#5915)

* feat(agentflow): enhance validation and feedback mechanisms

- Added constraint validation functions to ensure flow integrity, including checks for single start nodes, nested iterations, and human input restrictions within iterations.
- Implemented a new ValidationFeedback component to display validation results and issues directly in the UI.
- Updated the Agentflow component to integrate validation feedback and display snackbar notifications for constraint violations.
- Enhanced flow validation logic to detect hanging edges and validate node inputs, improving overall flow integrity checks.
- Introduced new test cases for validation functions to ensure robustness and reliability.

* address gemini review feedback
This commit is contained in:
Jocelyn Lin
2026-03-06 10:37:40 -08:00
committed by GitHub
parent 09f2a15310
commit 11a393600a
20 changed files with 1102 additions and 213 deletions
+82 -107
View File
@@ -1,137 +1,112 @@
# @flowiseai/agentflow Test Plan
# @flowiseai/agentflow — Testing Guide
## Running Tests
```bash
# Fast run (no coverage)
pnpm test
# With coverage enforcement
pnpm test:coverage
# Watch mode during development
pnpm test:watch
pnpm test # Fast run (no coverage)
pnpm test:coverage # With coverage enforcement
pnpm test:watch # Watch mode during development
```
## Test Coverage by Tier
## Test Strategy
Add tests when actively working on these files. Each tier reflects impact and testability.
Tests are prioritized by impact. When modifying a file, add or update tests in the same PR.
### Tier 1 — Core Logic
### Tier 1 — Core Logic (must test)
These modules carry the highest risk. Test in the same PR when modifying.
Pure business logic in `core/`, `infrastructure/`, and critical hooks. These carry the highest risk — a bug here affects every user. Always test in the same PR when modifying.
<!-- prettier-ignore -->
| File | Key exports to test | Status |
| --- | --- | --- |
| `src/core/validation/` | `validateFlow`, `validateNode` — empty flows, missing/multiple starts, disconnected nodes, cycles, required inputs; `isValidConnectionAgentflowV2` — self-connections, cycle detection | ✅ Done |
| `src/core/utils/` | `getUniqueNodeId`, `getUniqueNodeLabel`, `initNode`, `generateExportFlowData` | ✅ Done |
| `src/core/node-catalog/` | `filterNodesByComponents`, `isAgentflowNode`, `groupNodesByCategory` | ✅ Done |
| `src/core/node-config/` | `getAgentflowIcon`, `getNodeColor` | ✅ Done |
| `src/core/theme/tokens.ts` | All design tokens — node colors, light/dark variants, spacing scale, semantic colors, ReactFlow colors, shadows, border radius, gradients | ✅ Done |
| `src/core/theme/cssVariables.ts` | `generateCSSVariables()` — valid CSS strings, all variables, correct light/dark values, proper formatting, consistency with tokens | ✅ Done |
| `src/core/theme/createAgentflowTheme.ts` | `createAgentflowTheme()` — MUI theme creation, palette mode, colors from tokens, custom card palette, spacing, border radius, consistency | ✅ Done |
| `src/infrastructure/api/client.ts` | `createApiClient` — headers, auth token, 401 interceptor | ✅ Done |
| `src/infrastructure/api/chatflows.ts` | All CRUD + `generateAgentflow` + `getChatModels`, FlowData serialization | ✅ Done |
| `src/infrastructure/api/nodes.ts` | `getAllNodes`, `getNodeByName`, `getNodeIconUrl` | ✅ Done |
| `src/infrastructure/store/AgentflowContext.tsx` | `agentflowReducer` (all actions), `normalizeNodes`, `deleteNode()`, `duplicateNode()`, `openEditDialog()`, `closeEditDialog()`, `setNodes()`, `setEdges()`, `updateNodeData()`, `deleteEdge()`, state synchronization with local setters. E2E: composite workflow (add→connect→edit→save), load→modify→save roundtrip, multi-edge from single node, rapid connect/disconnect cycles, edge deletion | ✅ Done |
| `src/infrastructure/store/ApiContext.tsx` | `ApiProvider` — client creation, memoization, `useApiContext` error boundary | ✅ Done |
| `src/useAgentflow.ts` | `getFlow()`, `toJSON()`, `validate()`, `addNode()`, `clear()`, `fitView()`, `getReactFlowInstance()`, instance stability | ✅ Done |
| `src/features/canvas/hooks/useFlowHandlers.ts` | `handleConnect`, `handleNodesChange`, `handleEdgesChange`, `handleAddNode` — synchronous `onFlowChange` callbacks, dirty tracking, viewport resolution, change filtering | ✅ Done |
**What belongs here:** validation rules, node utilities, API clients, state management (reducers, context actions), flow data hooks (`useFlowHandlers`).
### Tier 2 — Feature Hooks & Dialogs
### Tier 2 — Feature Hooks & Dialogs (test when changing)
Test when adding features or fixing bugs in these areas.
Feature-level hooks and dialog components that orchestrate UI behavior. Test when adding features or fixing bugs.
<!-- prettier-ignore -->
| File | Key exports to test | Status |
| --- | --- | --- |
| `src/features/node-palette/search.ts` | `fuzzyScore`, `searchNodes`, `debounce` | ✅ Done |
| `src/features/canvas/hooks/useFlowNodes.ts` | `useFlowNodes()` — category filtering, component whitelist, error states | ✅ Done |
| `src/features/canvas/hooks/useDragAndDrop.ts` | `useDragAndDrop()` — JSON parse error handling, node init on drop | ✅ Done |
| `src/features/canvas/hooks/useNodeColors.ts` | `useNodeColors()` — color calculations for selected/hover/dark mode | ✅ Done |
| `src/infrastructure/store/ConfigContext.tsx` | `ConfigProvider` — theme detection (light/dark/system), media query listener | ✅ Done |
| `src/features/generator/GenerateFlowDialog.tsx` | Dialog state machine — API call flow, error handling, progress state | ✅ Done |
| `src/features/node-editor/EditNodeDialog.tsx` | Label editing — keyboard handling (Enter/Escape), node data updates | ✅ Done |
| `src/features/canvas/hooks/useOpenNodeEditor.ts` | `openNodeEditor()` — node/schema lookup, inputValues initialization, early returns, fallback to `node.data.inputs` when API schema unavailable, API schema priority over `data.inputs` | ✅ Done |
**What belongs here:** search logic, drag-and-drop, node color calculations, dialog state machines, theme detection.
### Tier 3 — UI Components
### Tier 3 — UI Components (test if logic exists)
Mostly JSX with minimal logic. Only add tests if business logic is introduced.
Presentational components that are mostly JSX. Only add tests if the component contains meaningful business logic (e.g., an exported helper function). Pure styling components (`styled.ts`, `MainCard.tsx`, etc.) do not need tests.
<!-- prettier-ignore -->
| File | Key exports to test | Status |
| --- | --- | --- |
| `src/features/canvas/components/NodeOutputHandles.tsx` | `getMinimumNodeHeight()` — linear scaling, MIN_NODE_HEIGHT floor | ✅ Done |
| `src/features/canvas/components/ConnectionLine.tsx` | Edge label visibility per node type, label content (condition index, humanInput proceed/reject), edge color from AGENTFLOW_ICONS | ✅ Done |
| `src/Agentflow.tsx` | Integration test — dark mode, ThemeProvider, CSS variables, header rendering, keyboard shortcuts (Cmd+S / Ctrl+S save), generate flow dialog, imperative ref | ✅ Done |
## Writing Tests
Files that are pure styling or data constants (`styled.ts`, `nodeIcons.ts`, `MainCard.tsx`, etc.) do not need dedicated tests.
### File Extension Convention
## Test Utilities
### Factory Functions (`src/__test_utils__/factories.ts`)
Use factory functions to create test fixtures with sensible defaults:
```typescript
import { makeFlowNode, makeFlowEdge, makeNodeData } from '@test-utils/factories'
// Create test nodes
const node = makeFlowNode('node-1', {
type: 'agentflowNode',
data: { name: 'llmAgentflow', label: 'LLM' }
})
// Create test edges
const edge = makeFlowEdge('node-1', 'node-2')
// Create node data (for palette/search tests)
const nodeData = makeNodeData({ name: 'llmAgentflow', label: 'LLM' })
```
### Custom Jest Environment
**File**: `src/__test_utils__/jest-environment-jsdom.js`
Prevents the `canvas` native module from being loaded during jsdom initialization. The canvas package requires native compilation which fails in many environments, but it's only an optional dependency of jsdom and not needed for React component tests.
This custom environment intercepts `require('canvas')` at the module level and returns a mock before jsdom tries to load the native binary.
### Module Mocks
**ReactFlow Mock** (`src/__mocks__/reactflow.tsx`): Provides mock implementations of ReactFlow components and hooks.
Key features:
- Uses `forwardRef` for MUI `styled()` compatibility (prevents emotion errors)
- Uses `useState` internally to maintain stable references (prevents infinite re-render loops)
- Exports all commonly used ReactFlow components (`Controls`, `MiniMap`, `Background`, etc.)
- Mocks hooks (`useNodesState`, `useEdgesState`, `useReactFlow`)
**Axios Mock** (`src/__mocks__/axios.ts`): Prevents network errors by mocking all HTTP requests. Returns empty arrays/objects for all API calls to silence network warnings in tests.
**CSS Mock** (`src/__mocks__/styleMock.js`): Empty object export for CSS imports.
## File Extension Convention
The jest config uses file extensions to select the test environment:
The Jest config uses file extensions to select the test environment:
| Extension | Environment | When to use |
| ----------- | ----------------------- | -------------------------------------------------------------------------- |
| `.test.ts` | **node** (no DOM) | Pure logic — utilities, reducers, data transformations |
| `.test.tsx` | **jsdom** (browser DOM) | Anything that renders React — `renderHook` with providers, component tests |
**Source files** follow a different rule: use `.tsx` only when the file contains JSX syntax. A React hook like `useAgentflow.ts` has no JSX, so it stays `.ts` even though its test file is `.test.tsx` (because the test uses `renderHook` with a JSX wrapper).
Source files use `.tsx` only when they contain JSX syntax. A hook like `useAgentflow.ts` has no JSX, so it stays `.ts` even though its test is `.test.tsx` (because the test uses `renderHook` with a JSX wrapper).
### Factory Functions
Use factory functions from `@test-utils/factories` to create test fixtures with sensible defaults:
```typescript
import { makeFlowNode, makeFlowEdge, makeNodeData } from '@test-utils/factories'
const node = makeFlowNode('node-1', {
type: 'agentflowNode',
data: { name: 'llmAgentflow', label: 'LLM' }
})
const edge = makeFlowEdge('node-1', 'node-2')
const nodeData = makeNodeData({ name: 'llmAgentflow', label: 'LLM' })
```
### Mocking Patterns
**Mocking a module with `jest.mock`:**
```typescript
import { isValidConnectionAgentflowV2 } from '@/core'
jest.mock('@/core', () => ({
isValidConnectionAgentflowV2: jest.fn(() => true),
getUniqueNodeId: jest.fn(() => 'new-node-1')
}))
// Override per-test:
it('should reject invalid connection', () => {
;(isValidConnectionAgentflowV2 as jest.Mock).mockReturnValueOnce(false)
// ...
})
```
**Mocking context hooks:**
```typescript
const mockSetDirty = jest.fn()
jest.mock('@/infrastructure/store', () => ({
useAgentflowContext: () => ({
state: { reactFlowInstance: null },
setDirty: mockSetDirty
})
}))
```
### Module Mocks
**ReactFlow** (`src/__mocks__/reactflow.tsx`): Mock implementations of ReactFlow components and hooks. Uses `forwardRef` for MUI `styled()` compatibility and `useState` internally for stable references.
**Axios** (`src/__mocks__/axios.ts`): Prevents network errors by mocking all HTTP methods. Returns empty arrays/objects by default.
**CSS/SVG** (`src/__mocks__/styleMock.js`): Empty object export for CSS and SVG imports.
### Custom Jest Environment
`src/__test_utils__/jest-environment-jsdom.js` intercepts `require('canvas')` and returns a mock before jsdom tries to load the native binary. This prevents build failures in environments without native canvas compilation.
## Configuration
- **Jest config**: `jest.config.js` — two projects: `unit` (node env, `.test.ts`) and `components` (custom jsdom env, `.test.tsx`)
- **Test environment**: Component tests use custom jsdom environment (`src/__test_utils__/jest-environment-jsdom.js`) to handle canvas loading
- **Import aliases**: `@test-utils` maps to `src/__test_utils__` for convenient imports
- **Coverage thresholds**: uniform 80% floor (`branches`, `functions`, `lines`, `statements`) — see `coverageThreshold` in `jest.config.js` for the full list
- **Coverage exclusions**:
- `src/__test_utils__/**` — test utilities
- `src/__mocks__/**` — module mocks
- **Import aliases**: `@test-utils` maps to `src/__test_utils__`, `@/` maps to `src/`
- **Coverage thresholds**: 80% floor for `branches`, `functions`, `lines`, `statements` — see `coverageThreshold` in `jest.config.js` for per-path entries
- **Coverage exclusions**: `src/__test_utils__/**`, `src/__mocks__/**`
- **CI**: `pnpm test:coverage` runs in GitHub Actions between lint and build
- **Reports**: `coverage/lcov-report/index.html` for detailed HTML report
+1
View File
@@ -13,6 +13,7 @@ const baseConfig = {
testPathIgnorePatterns: ['/node_modules/', '/dist/'],
moduleNameMapper: {
'^@test-utils/(.*)$': '<rootDir>/src/__test_utils__/$1',
'\\.svg$': '<rootDir>/src/__mocks__/styleMock.js',
'^@/(.*)$': '<rootDir>/src/$1',
'\\.(css|less|scss|sass)$': '<rootDir>/src/__mocks__/styleMock.js'
}
+57 -7
View File
@@ -1,10 +1,12 @@
import { forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react'
import ReactFlow, { Background, Controls, MiniMap, ReactFlowProvider, useEdgesState, useNodesState } from 'reactflow'
import { Alert, Snackbar } from '@mui/material'
import { IconSparkles } from '@tabler/icons-react'
import { tokens } from './core/theme'
import type { AgentFlowInstance, AgentflowProps, FlowData, FlowDataCallback, FlowEdge, FlowNode } from './core/types'
import { applyValidationErrorsToNodes, validateFlow } from './core/validation'
import {
AgentflowHeader,
ConnectionLine,
@@ -15,6 +17,7 @@ import {
useFlowHandlers,
useFlowNodes
} from './features/canvas'
import { ValidationFeedback } from './features/canvas/components'
import { GenerateFlowDialog } from './features/generator'
import { EditNodeDialog } from './features/node-editor'
import { AddNodesDrawer, StyledFab } from './features/node-palette'
@@ -80,6 +83,17 @@ function AgentflowCanvas({
const [edges, setLocalEdges, onEdgesChange] = useEdgesState(initialFlow?.edges || [])
const [showGenerateDialog, setShowGenerateDialog] = useState(false)
// Constraint violation snackbar state
const [snackbar, setSnackbar] = useState<{ open: boolean; message: string }>({ open: false, message: '' })
const handleConstraintViolation = useCallback((message: string) => {
setSnackbar({ open: true, message })
}, [])
const handleSnackbarClose = useCallback(() => {
setSnackbar({ open: false, message: '' })
}, [])
// Load available nodes
const { availableNodes } = useFlowNodes()
@@ -106,14 +120,16 @@ function AgentflowCanvas({
onNodesChange,
onEdgesChange,
onFlowChange,
availableNodes
availableNodes,
onConstraintViolation: handleConstraintViolation
})
// Drag and drop handlers
const { handleDragOver, handleDrop } = useDragAndDrop({
nodes: nodes as FlowNode[],
setLocalNodes: setLocalNodes as React.Dispatch<React.SetStateAction<FlowNode[]>>,
reactFlowWrapper: reactFlowWrapper as React.RefObject<HTMLDivElement>
reactFlowWrapper: reactFlowWrapper as React.RefObject<HTMLDivElement>,
onConstraintViolation: handleConstraintViolation
})
// Handle generated flow from dialog
@@ -134,13 +150,25 @@ function AgentflowCanvas({
[setLocalNodes, setLocalEdges, setDirty, onFlowGenerated]
)
// Handle save
// Handle save — run validation first and highlight problem nodes
const handleSave = useCallback(() => {
if (onSave) {
onSave(agentflow.getFlow())
setDirty(false)
if (!onSave) return
const flowNodes = nodes as FlowNode[]
const flowEdges = edges as FlowEdge[]
const result = validateFlow(flowNodes, flowEdges, availableNodes)
// Update node border highlighting: set errors on failing nodes, clear errors on now-valid nodes
setLocalNodes((prev) => applyValidationErrorsToNodes(prev as FlowNode[], result.errors) as FlowNode[])
if (!result.valid) {
handleConstraintViolation('Flow has validation errors. Please fix them before saving.')
return
}
}, [onSave, agentflow, setDirty])
onSave(agentflow.getFlow())
setDirty(false)
}, [onSave, agentflow, setDirty, nodes, edges, availableNodes, setLocalNodes, handleConstraintViolation])
// Keyboard shortcut: Cmd+S / Ctrl+S to save
useEffect(() => {
@@ -210,6 +238,16 @@ function AgentflowCanvas({
</StyledFab>
)}
{/* Validation Feedback - positioned at top right */}
{!readOnly && (
<ValidationFeedback
nodes={nodes as FlowNode[]}
edges={edges as FlowEdge[]}
availableNodes={availableNodes}
setNodes={setLocalNodes as React.Dispatch<React.SetStateAction<FlowNode[]>>}
/>
)}
<ReactFlow
nodes={nodes}
edges={edges}
@@ -244,6 +282,18 @@ function AgentflowCanvas({
{/* Edit Node Dialog */}
<EditNodeDialog show={state.editingNodeId !== null} dialogProps={state.editDialogProps || {}} onCancel={closeEditDialog} />
{/* Constraint Violation Snackbar */}
<Snackbar
open={snackbar.open}
autoHideDuration={5000}
onClose={handleSnackbarClose}
anchorOrigin={{ vertical: 'bottom', horizontal: 'left' }}
>
<Alert onClose={handleSnackbarClose} severity='error' variant='filled' sx={{ width: '100%' }}>
{snackbar.message}
</Alert>
</Snackbar>
</div>
)
}
File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 10 KiB

@@ -83,6 +83,7 @@ export interface NodeData {
error?: string
warning?: string
hint?: string
validationErrors?: string[]
[key: string]: unknown
}
@@ -131,6 +132,7 @@ export interface InputParam {
show?: Record<string, unknown>
hide?: Record<string, unknown>
display?: boolean
array?: InputParam[] // Sub-field definitions for array-type params
}
export interface EdgeData {
+1 -1
View File
@@ -1,5 +1,5 @@
// Node factory - Node ID generation, labeling, and initialization
export { getUniqueNodeId, getUniqueNodeLabel, initNode } from './nodeFactory'
export { getUniqueNodeId, getUniqueNodeLabel, initNode, resolveNodeType } from './nodeFactory'
// Flow export utilities
export { generateExportFlowData } from './flowExport'
@@ -1,5 +1,21 @@
import type { FlowNode, NodeData } from '../types'
/**
* Map from NodeData.type to the ReactFlow node type key.
* Any type not listed here defaults to 'agentflowNode'.
*/
const NODE_TYPE_MAP: Record<string, string> = {
Iteration: 'iteration',
StickyNote: 'stickyNote'
}
/**
* Resolve the ReactFlow node type from a NodeData type string.
*/
export function resolveNodeType(nodeDataType: string): string {
return NODE_TYPE_MAP[nodeDataType] ?? 'agentflowNode'
}
/**
* Generate a unique node ID based on existing nodes
*/
@@ -0,0 +1,149 @@
import { makeFlowNode } from '@test-utils/factories'
import {
checkHumanInputInIteration,
checkNestedIteration,
checkNodePlacementConstraints,
checkSingleStartNode,
findParentIterationNode
} from './constraintValidation'
describe('checkSingleStartNode', () => {
it('should reject when a start node already exists', () => {
const nodes = [makeFlowNode('a', { data: { id: 'a', name: 'startAgentflow', label: 'Start' } })]
const result = checkSingleStartNode(nodes, 'startAgentflow')
expect(result.valid).toBe(false)
expect(result.message).toContain('Only one start node')
})
it('should allow when no start node exists', () => {
const nodes = [makeFlowNode('a', { data: { id: 'a', name: 'llmAgentflow', label: 'LLM' } })]
const result = checkSingleStartNode(nodes, 'startAgentflow')
expect(result.valid).toBe(true)
})
it('should allow non-start nodes regardless', () => {
const nodes = [makeFlowNode('a', { data: { id: 'a', name: 'startAgentflow', label: 'Start' } })]
const result = checkSingleStartNode(nodes, 'llmAgentflow')
expect(result.valid).toBe(true)
})
})
describe('checkNestedIteration', () => {
it('should reject iteration inside iteration', () => {
const parent = makeFlowNode('iter', { type: 'iteration', data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' } })
const result = checkNestedIteration('iterationAgentflow', parent)
expect(result.valid).toBe(false)
expect(result.message).toContain('Nested iteration')
})
it('should allow iteration outside iteration', () => {
const result = checkNestedIteration('iterationAgentflow', null)
expect(result.valid).toBe(true)
})
it('should allow non-iteration nodes inside iteration', () => {
const parent = makeFlowNode('iter', { type: 'iteration', data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' } })
const result = checkNestedIteration('llmAgentflow', parent)
expect(result.valid).toBe(true)
})
})
describe('checkHumanInputInIteration', () => {
it('should reject human input inside iteration', () => {
const parent = makeFlowNode('iter', { type: 'iteration', data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' } })
const result = checkHumanInputInIteration('humanInputAgentflow', parent)
expect(result.valid).toBe(false)
expect(result.message).toContain('Human input node')
})
it('should allow human input outside iteration', () => {
const result = checkHumanInputInIteration('humanInputAgentflow', null)
expect(result.valid).toBe(true)
})
it('should allow non-human-input nodes inside iteration', () => {
const parent = makeFlowNode('iter', { type: 'iteration', data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' } })
const result = checkHumanInputInIteration('llmAgentflow', parent)
expect(result.valid).toBe(true)
})
})
describe('checkNodePlacementConstraints', () => {
it('should reject duplicate start node', () => {
const nodes = [makeFlowNode('a', { data: { id: 'a', name: 'startAgentflow', label: 'Start' } })]
const result = checkNodePlacementConstraints(nodes, 'startAgentflow')
expect(result.valid).toBe(false)
expect(result.message).toContain('Only one start node')
})
it('should reject nested iteration when position is inside an iteration node', () => {
const iterNode = makeFlowNode('iter', {
type: 'iteration',
position: { x: 100, y: 100 },
width: 400,
height: 300,
data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' }
})
const result = checkNodePlacementConstraints([iterNode], 'iterationAgentflow', { x: 200, y: 200 })
expect(result.valid).toBe(false)
expect(result.message).toContain('Nested iteration')
})
it('should reject human input inside iteration', () => {
const iterNode = makeFlowNode('iter', {
type: 'iteration',
position: { x: 100, y: 100 },
width: 400,
height: 300,
data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' }
})
const result = checkNodePlacementConstraints([iterNode], 'humanInputAgentflow', { x: 200, y: 200 })
expect(result.valid).toBe(false)
expect(result.message).toContain('Human input node')
})
it('should pass when all constraints are satisfied', () => {
const nodes = [makeFlowNode('a', { data: { id: 'a', name: 'llmAgentflow', label: 'LLM' } })]
const result = checkNodePlacementConstraints(nodes, 'llmAgentflow', { x: 50, y: 50 })
expect(result.valid).toBe(true)
})
it('should pass when no position is provided', () => {
const nodes = [makeFlowNode('a', { data: { id: 'a', name: 'llmAgentflow', label: 'LLM' } })]
const result = checkNodePlacementConstraints(nodes, 'llmAgentflow')
expect(result.valid).toBe(true)
})
})
describe('findParentIterationNode', () => {
it('should return iteration node when position is inside', () => {
const iterNode = makeFlowNode('iter', {
type: 'iteration',
position: { x: 100, y: 100 },
width: 400,
height: 300,
data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' }
})
const result = findParentIterationNode([iterNode], { x: 200, y: 200 })
expect(result).toBe(iterNode)
})
it('should return null when position is outside all iteration nodes', () => {
const iterNode = makeFlowNode('iter', {
type: 'iteration',
position: { x: 100, y: 100 },
width: 400,
height: 300,
data: { id: 'iter', name: 'iterationAgentflow', label: 'Iteration' }
})
const result = findParentIterationNode([iterNode], { x: 600, y: 600 })
expect(result).toBeNull()
})
it('should return null when there are no iteration nodes', () => {
const node = makeFlowNode('a', { data: { id: 'a', name: 'llmAgentflow', label: 'LLM' } })
const result = findParentIterationNode([node], { x: 0, y: 0 })
expect(result).toBeNull()
})
})
@@ -0,0 +1,85 @@
import type { FlowNode } from '../types'
export interface ConstraintResult {
valid: boolean
message?: string
}
/**
* Check that only one start node exists in the flow
*/
export function checkSingleStartNode(nodes: FlowNode[], newNodeName: string): ConstraintResult {
if (newNodeName === 'startAgentflow' && nodes.some((n) => n.data.name === 'startAgentflow')) {
return { valid: false, message: 'Only one start node is allowed' }
}
return { valid: true }
}
/**
* Check that iteration nodes are not nested inside other iteration nodes
*/
export function checkNestedIteration(newNodeName: string, parentNode: FlowNode | null): ConstraintResult {
if (newNodeName === 'iterationAgentflow' && parentNode?.type === 'iteration') {
return { valid: false, message: 'Nested iteration nodes are not supported' }
}
return { valid: true }
}
/**
* Check that human input nodes are not placed inside iteration nodes
*/
export function checkHumanInputInIteration(newNodeName: string, parentNode: FlowNode | null): ConstraintResult {
if (newNodeName === 'humanInputAgentflow' && parentNode?.type === 'iteration') {
return { valid: false, message: 'Human input node is not supported inside Iteration node' }
}
return { valid: true }
}
/**
* Check all placement constraints for a node being added to the canvas.
* Returns the first failing constraint, or a valid result if all pass.
*/
export function checkNodePlacementConstraints(
nodes: FlowNode[],
nodeType: string,
position?: { x: number; y: number } | null
): ConstraintResult {
const startCheck = checkSingleStartNode(nodes, nodeType)
if (!startCheck.valid) return startCheck
if (position) {
const parentNode = findParentIterationNode(nodes, position)
if (parentNode) {
const nestedCheck = checkNestedIteration(nodeType, parentNode)
if (!nestedCheck.valid) return nestedCheck
const humanInputCheck = checkHumanInputInIteration(nodeType, parentNode)
if (!humanInputCheck.valid) return humanInputCheck
}
}
return { valid: true }
}
/**
* Find the iteration node that contains the given position, if any
*/
export function findParentIterationNode(nodes: FlowNode[], position: { x: number; y: number }): FlowNode | null {
const iterationNodes = nodes.filter((node) => node.type === 'iteration')
for (const iterationNode of iterationNodes) {
const nodeWidth = iterationNode.width || 300
const nodeHeight = iterationNode.height || 250
const nodeLeft = iterationNode.position.x
const nodeRight = nodeLeft + nodeWidth
const nodeTop = iterationNode.position.y
const nodeBottom = nodeTop + nodeHeight
if (position.x >= nodeLeft && position.x <= nodeRight && position.y >= nodeTop && position.y <= nodeBottom) {
return iterationNode
}
}
return null
}
@@ -1,4 +1,4 @@
import { makeFlowEdge, makeFlowNode } from '@test-utils/factories'
import { makeFlowEdge, makeFlowNode, makeNodeData } from '@test-utils/factories'
import type { FlowEdge, FlowNode } from '../types'
@@ -36,42 +36,18 @@ describe('validateFlow', () => {
expect(result.valid).toBe(true)
})
it('should warn when start node has no outgoing connections', () => {
const nodes = [makeNode('a', 'startAgentflow')]
const result = validateFlow(nodes, [])
expect(result.errors).toContainEqual(
expect.objectContaining({ type: 'warning', message: expect.stringContaining('outgoing connection') })
)
// Warnings don't make the flow invalid
expect(result.valid).toBe(true)
})
it('should warn for disconnected non-start nodes with no incoming edges', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'llmAgentflow', 'LLM')]
it('should warn for disconnected nodes (not in any edge)', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'llmAgentflow')]
const edges: FlowEdge[] = []
const result = validateFlow(nodes, edges)
expect(result.errors).toContainEqual(
expect.objectContaining({ nodeId: 'b', type: 'warning', message: expect.stringContaining('no incoming') })
expect.objectContaining({ nodeId: 'a', type: 'warning', message: 'This node is not connected to anything' })
)
})
it('should warn for non-end nodes with no outgoing edges', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'llmAgentflow', 'LLM')]
const edges = [makeEdge('a', 'b')]
const result = validateFlow(nodes, edges)
expect(result.errors).toContainEqual(
expect.objectContaining({ nodeId: 'b', type: 'warning', message: expect.stringContaining('no outgoing') })
expect.objectContaining({ nodeId: 'b', type: 'warning', message: 'This node is not connected to anything' })
)
})
it('should not warn about outgoing edges for end nodes (directReplyAgentflow)', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'directReplyAgentflow')]
const edges = [makeEdge('a', 'b')]
const result = validateFlow(nodes, edges)
const outgoingWarnings = result.errors.filter((e) => e.nodeId === 'b' && e.message.includes('no outgoing'))
expect(outgoingWarnings).toHaveLength(0)
})
it('should ignore sticky notes in disconnection checks', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'stickyNoteAgentflow')]
const edges: FlowEdge[] = []
@@ -87,18 +63,55 @@ describe('validateFlow', () => {
expect(result.valid).toBe(false)
expect(result.errors).toContainEqual(expect.objectContaining({ message: expect.stringContaining('cycle') }))
})
// --- Hanging edge detection ---
it('should warn about hanging edge with missing source', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'llmAgentflow')]
const edges = [makeEdge('a', 'b'), makeEdge('nonexistent', 'b')]
const result = validateFlow(nodes, edges)
expect(result.errors).toContainEqual(
expect.objectContaining({
nodeId: 'b',
message: expect.stringContaining('non-existent source node')
})
)
})
it('should warn about hanging edge with missing target', () => {
const nodes = [makeNode('a', 'startAgentflow'), makeNode('b', 'llmAgentflow')]
const edges = [makeEdge('a', 'b'), makeEdge('a', 'nonexistent')]
const result = validateFlow(nodes, edges)
expect(result.errors).toContainEqual(
expect.objectContaining({
nodeId: 'a',
message: expect.stringContaining('non-existent target node')
})
)
})
it('should warn about hanging edge with both missing', () => {
const nodes = [makeNode('a', 'startAgentflow')]
const edges = [makeEdge('x', 'y')]
const result = validateFlow(nodes, edges)
expect(result.errors).toContainEqual(
expect.objectContaining({
edgeId: 'x-y',
message: 'Disconnected edge - both source and target nodes do not exist'
})
)
})
})
describe('validateNode', () => {
it('should return error for node with no name', () => {
const node = makeNode('a', '')
const errors = validateNode(node, [])
const errors = validateNode(node)
expect(errors).toContainEqual(expect.objectContaining({ type: 'error', message: expect.stringContaining('missing a name') }))
})
it('should return no errors for valid node', () => {
const node = makeNode('a', 'llmAgentflow')
expect(validateNode(node, [])).toHaveLength(0)
expect(validateNode(node)).toHaveLength(0)
})
it('should warn about missing required inputs', () => {
@@ -112,8 +125,8 @@ describe('validateNode', () => {
inputValues: {}
}
}
const errors = validateNode(node, [])
expect(errors).toContainEqual(expect.objectContaining({ type: 'warning', message: expect.stringContaining('Model') }))
const errors = validateNode(node)
expect(errors).toContainEqual(expect.objectContaining({ type: 'warning', message: 'Model is required' }))
})
it('should not warn about optional inputs', () => {
@@ -127,7 +140,111 @@ describe('validateNode', () => {
inputValues: {}
}
}
const errors = validateNode(node, [])
const errors = validateNode(node)
expect(errors).toHaveLength(0)
})
it('should skip hidden fields (show condition not met)', () => {
const node: FlowNode = {
...makeNode('a', 'llmAgentflow'),
data: {
id: 'a',
name: 'llmAgentflow',
label: 'LLM',
inputs: [{ id: 'p1', name: 'apiKey', label: 'API Key', type: 'string', optional: false, show: { mode: 'api' } }],
inputValues: { mode: 'local' }
}
}
const errors = validateNode(node)
const apiKeyErrors = errors.filter((e) => e.message.includes('API Key'))
expect(apiKeyErrors).toHaveLength(0)
})
// --- Credential validation ---
it('should warn about missing required credential', () => {
const node: FlowNode = {
...makeNode('a', 'llmAgentflow'),
data: {
id: 'a',
name: 'llmAgentflow',
label: 'LLM',
inputs: [{ id: 'cred', name: 'credential', label: 'Credential', type: 'string', optional: false }],
inputValues: {}
}
}
const errors = validateNode(node)
expect(errors).toContainEqual(expect.objectContaining({ message: 'Credential is required' }))
// Should produce exactly one error, not a duplicate from the general required-field check
const credErrors = errors.filter((e) => e.message.includes('required'))
expect(credErrors).toHaveLength(1)
})
it('should not warn about credential when value is set', () => {
const node: FlowNode = {
...makeNode('a', 'llmAgentflow'),
data: {
id: 'a',
name: 'llmAgentflow',
label: 'LLM',
inputs: [{ id: 'cred', name: 'credential', label: 'Credential', type: 'string', optional: false }],
inputValues: { credential: 'some-credential-id' }
}
}
const errors = validateNode(node)
const credErrors = errors.filter((e) => e.message === 'Credential is required')
expect(credErrors).toHaveLength(0)
})
// --- Array sub-field validation ---
it('should validate required array sub-fields', () => {
const node: FlowNode = {
...makeNode('a', 'conditionAgentflow'),
data: {
id: 'a',
name: 'conditionAgentflow',
label: 'Condition',
inputs: [
{
id: 'conds',
name: 'conditions',
label: 'Conditions',
type: 'array',
array: [{ id: 'f1', name: 'fieldName', label: 'Field Name', type: 'string', optional: false }]
}
],
inputValues: {
conditions: [{ fieldName: '' }, { fieldName: 'valid' }]
}
}
}
const errors = validateNode(node)
expect(errors).toContainEqual(expect.objectContaining({ message: 'Conditions item #1: Field Name is required' }))
const item2Errors = errors.filter((e) => e.message.includes('item #2'))
expect(item2Errors).toHaveLength(0)
})
// --- Nested config validation ---
it('should validate nested component config required fields', () => {
const availableNodes = [
makeNodeData({
name: 'openAIChat',
inputs: [{ id: 'ak', name: 'apiKey', label: 'API Key', type: 'string', optional: false }]
})
]
const node: FlowNode = {
...makeNode('a', 'llmAgentflow'),
data: {
id: 'a',
name: 'llmAgentflow',
label: 'LLM',
inputs: [{ id: 'model', name: 'model', label: 'Chat Model', type: 'string' }],
inputValues: {
model: 'openAIChat',
modelConfig: { apiKey: '' }
}
}
}
const errors = validateNode(node, availableNodes)
expect(errors).toContainEqual(expect.objectContaining({ message: 'Chat Model configuration: API Key is required' }))
})
})
@@ -1,10 +1,15 @@
import type { FlowEdge, FlowNode, ValidationError, ValidationResult } from '../types'
import type { FlowEdge, FlowNode, NodeData, ValidationError, ValidationResult } from '../types'
import { evaluateParamVisibility } from '../utils/fieldVisibility'
/** Check if a value is empty (null, undefined, empty string, or empty rich text) */
function isEmptyValue(value: unknown): boolean {
return value == null || value === '' || value === '<p></p>'
}
/**
* Validate the flow structure
*/
export function validateFlow(nodes: FlowNode[], edges: FlowEdge[]): ValidationResult {
export function validateFlow(nodes: FlowNode[], edges: FlowEdge[], availableNodes?: NodeData[]): ValidationResult {
const errors: ValidationError[] = []
// Check for empty flow
@@ -34,40 +39,19 @@ export function validateFlow(nodes: FlowNode[], edges: FlowEdge[]): ValidationRe
})
}
// Check for disconnected nodes
// Check for disconnected nodes (matching server-side pattern)
const connectedNodes = new Set<string>()
edges.forEach((edge) => {
connectedNodes.add(edge.source)
connectedNodes.add(edge.target)
})
const nonStickyNodes = nodes.filter((n) => n.data.name !== 'stickyNoteAgentflow')
nonStickyNodes.forEach((node) => {
const hasIncoming = edges.some((e) => e.target === node.id)
const hasOutgoing = edges.some((e) => e.source === node.id)
// Start node should only have outgoing connections
if (node.data.name === 'startAgentflow') {
if (!hasOutgoing) {
errors.push({
nodeId: node.id,
message: 'Start node must have at least one outgoing connection',
type: 'warning'
})
}
return
}
// End nodes don't need outgoing connections
const isEndNode = ['directReplyAgentflow'].includes(node.data.name)
if (!hasIncoming) {
if (!connectedNodes.has(node.id)) {
errors.push({
nodeId: node.id,
message: `Node "${node.data.label || node.data.name}" has no incoming connections`,
type: 'warning'
})
}
if (!isEndNode && !hasOutgoing) {
errors.push({
nodeId: node.id,
message: `Node "${node.data.label || node.data.name}" has no outgoing connections`,
message: 'This node is not connected to anything',
type: 'warning'
})
}
@@ -82,6 +66,16 @@ export function validateFlow(nodes: FlowNode[], edges: FlowEdge[]): ValidationRe
})
}
// Validate each node's inputs
nonStickyNodes.forEach((node) => {
const nodeErrors = validateNode(node, availableNodes)
errors.push(...nodeErrors)
})
// Check for hanging edges
const hangingEdgeErrors = detectHangingEdges(nodes, edges)
errors.push(...hangingEdgeErrors)
return {
valid: errors.filter((e) => e.type === 'error').length === 0,
errors
@@ -137,9 +131,48 @@ function detectCycle(nodes: FlowNode[], edges: FlowEdge[]): boolean {
}
/**
* Check if a specific node is valid
* Detect hanging edges where source or target node no longer exists
*/
export function validateNode(node: FlowNode, _edges: FlowEdge[]): ValidationError[] {
function detectHangingEdges(nodes: FlowNode[], edges: FlowEdge[]): ValidationError[] {
const errors: ValidationError[] = []
for (const edge of edges) {
const sourceExists = nodes.some((node) => node.id === edge.source)
const targetExists = nodes.some((node) => node.id === edge.target)
if (!sourceExists || !targetExists) {
if (!sourceExists && targetExists) {
errors.push({
nodeId: edge.target,
message: `Connected to non-existent source node ${edge.source}`,
type: 'warning'
})
} else if (sourceExists && !targetExists) {
errors.push({
nodeId: edge.source,
message: `Connected to non-existent target node ${edge.target}`,
type: 'warning'
})
} else {
errors.push({
edgeId: edge.id,
message: 'Disconnected edge - both source and target nodes do not exist',
type: 'warning'
})
}
}
}
return errors
}
/**
* Check if a specific node is valid.
*
* @param availableNodes Component definitions (not flow node instances) used to look up
* nested config schemas via `availableNodes.find(n => n.name === componentName)`.
*/
export function validateNode(node: FlowNode, availableNodes?: NodeData[]): ValidationError[] {
const errors: ValidationError[] = []
// Check required fields
@@ -151,22 +184,114 @@ export function validateNode(node: FlowNode, _edges: FlowEdge[]): ValidationErro
})
}
// Check required inputs (if defined), skipping hidden params
const inputParams = node.data.inputs || []
const inputValues = node.data.inputValues || {}
for (const param of inputParams) {
if (
!param.optional &&
evaluateParamVisibility(param, inputValues) &&
(inputValues[param.name] == null || inputValues[param.name] === '')
) {
// Credential validation (skip general check to avoid duplicate errors)
if (param.name === 'credential') {
if (!param.optional && !inputValues[param.name]) {
errors.push({
nodeId: node.id,
message: 'Credential is required',
type: 'warning'
})
}
continue
}
// Check required inputs, skipping hidden params
if (!param.optional && evaluateParamVisibility(param, inputValues) && isEmptyValue(inputValues[param.name])) {
errors.push({
nodeId: node.id,
message: `Required input "${param.label || param.name}" is missing`,
message: `${param.label || param.name} is required`,
type: 'warning'
})
}
// Array item sub-field validation
if (param.type === 'array' && Array.isArray(inputValues[param.name]) && param.array) {
const arrayItems = inputValues[param.name] as Record<string, unknown>[]
if (arrayItems.length > 0) {
arrayItems.forEach((item, index) => {
for (const arrayParam of param.array!) {
// Evaluate visibility with array index for $index-based conditions
const shouldValidate = evaluateParamVisibility(arrayParam, item as Record<string, unknown>, index)
if (shouldValidate && !arrayParam.optional) {
const value = item[arrayParam.name]
if (isEmptyValue(value)) {
errors.push({
nodeId: node.id,
message: `${param.label} item #${index + 1}: ${arrayParam.label} is required`,
type: 'warning'
})
}
}
}
})
}
}
// Nested component config validation
const configKey = `${param.name}Config`
if (inputValues[configKey] && inputValues[param.name] && availableNodes) {
const componentName = inputValues[param.name] as string
const configValue = inputValues[configKey] as Record<string, unknown>
const componentDef = availableNodes.find((n) => n.name === componentName)
if (componentDef?.inputs) {
for (const componentParam of componentDef.inputs) {
if (!evaluateParamVisibility(componentParam, configValue)) continue
if (!componentParam.optional) {
const nestedValue = configValue[componentParam.name]
if (isEmptyValue(nestedValue)) {
errors.push({
nodeId: node.id,
message: `${param.label} configuration: ${componentParam.label} is required`,
type: 'warning'
})
}
}
}
}
}
}
return errors
}
/**
* Group validation errors by nodeId into a map of nodeId -> error messages.
* Useful for pushing validationErrors to node data for border highlighting.
*/
export function groupValidationErrorsByNodeId(errors: ValidationError[]): Map<string, string[]> {
const errorsByNodeId = new Map<string, string[]>()
for (const error of errors) {
if (error.nodeId) {
if (!errorsByNodeId.has(error.nodeId)) {
errorsByNodeId.set(error.nodeId, [])
}
errorsByNodeId.get(error.nodeId)!.push(error.message)
}
}
return errorsByNodeId
}
/**
* Apply validation errors to node data for border highlighting.
* Returns the updated nodes array (new references only for nodes whose errors changed).
*/
export function applyValidationErrorsToNodes(nodes: FlowNode[], errors: ValidationError[]): FlowNode[] {
const errorsByNodeId = groupValidationErrorsByNodeId(errors)
return nodes.map((node) => {
const nodeErrors = errorsByNodeId.get(node.id)
const hadErrors = (node.data.validationErrors?.length ?? 0) > 0
if (nodeErrors || hadErrors) {
return { ...node, data: { ...node.data, validationErrors: nodeErrors } }
}
return node
})
}
@@ -2,4 +2,14 @@
export { isValidConnectionAgentflowV2 } from './connectionValidation'
// Flow validation utilities
export { validateFlow, validateNode } from './flowValidation'
export { applyValidationErrorsToNodes, groupValidationErrorsByNodeId, validateFlow, validateNode } from './flowValidation'
// Constraint validation utilities
export type { ConstraintResult } from './constraintValidation'
export {
checkHumanInputInIteration,
checkNestedIteration,
checkNodePlacementConstraints,
checkSingleStartNode,
findParentIterationNode
} from './constraintValidation'
@@ -0,0 +1,277 @@
import { memo, useCallback, useEffect, useRef, useState } from 'react'
import { Alert, Box, Button, ClickAwayListener, Fab, Paper, Snackbar, Typography } from '@mui/material'
import { alpha, darken, lighten, useTheme } from '@mui/material/styles'
import { IconChecklist, IconExclamationCircle, IconX } from '@tabler/icons-react'
import validateEmptyImage from '@/assets/images/validate_empty.svg'
import { getAgentflowIcon } from '@/core/node-config'
import type { FlowEdge, FlowNode, NodeData, ValidationError } from '@/core/types'
import { applyValidationErrorsToNodes, validateFlow } from '@/core/validation'
import { useConfigContext } from '@/infrastructure/store'
/** Validation result grouped by node */
interface NodeValidationResult {
id: string
label: string
name: string
issues: string[]
}
export interface ValidationFeedbackProps {
nodes: FlowNode[]
edges: FlowEdge[]
availableNodes?: NodeData[]
setNodes: React.Dispatch<React.SetStateAction<FlowNode[]>>
}
function groupErrorsByNode(errors: ValidationError[], nodes: FlowNode[]): NodeValidationResult[] {
const nodeMap = new Map<string, NodeValidationResult>()
for (const error of errors) {
const id = error.nodeId || error.edgeId || 'flow'
if (!nodeMap.has(id)) {
const node = nodes.find((n) => n.id === id)
let label = `Edge ${id}`
let name = 'edge'
if (node) {
label = node.data.label || node.data.name
name = node.data.name
} else if (id === 'flow') {
label = 'Flow'
name = 'flow'
}
nodeMap.set(id, {
id,
label,
name,
issues: []
})
}
nodeMap.get(id)!.issues.push(error.message)
}
return Array.from(nodeMap.values())
}
function ValidationFeedbackComponent({ nodes, edges, availableNodes, setNodes }: ValidationFeedbackProps) {
const theme = useTheme()
const { isDarkMode } = useConfigContext()
const [open, setOpen] = useState(false)
const [results, setResults] = useState<NodeValidationResult[]>([])
const [hasValidated, setHasValidated] = useState(false)
const [successSnackbar, setSuccessSnackbar] = useState(false)
const containerRef = useRef<HTMLDivElement>(null)
const handleToggle = useCallback(() => {
setOpen((prev) => !prev)
}, [])
const handleClose = useCallback(() => {
setOpen(false)
}, [])
const handleValidate = useCallback(() => {
const result = validateFlow(nodes, edges, availableNodes)
const grouped = groupErrorsByNode(result.errors, nodes)
setResults(grouped)
setHasValidated(true)
// Show green success toast when no issues found
if (grouped.length === 0) {
setSuccessSnackbar(true)
}
// Push validation errors to node data for border highlighting
setNodes((prev) => applyValidationErrorsToNodes(prev, result.errors))
}, [nodes, edges, availableNodes, setNodes])
const getNodeIcon = (item: NodeValidationResult) => {
const foundIcon = getAgentflowIcon(item.name)
if (foundIcon) {
const IconComp = foundIcon.icon
return (
<Box
sx={{
width: 28,
height: 28,
borderRadius: '4px',
backgroundColor: foundIcon.color,
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
color: 'white',
flexShrink: 0
}}
>
<IconComp size={16} />
</Box>
)
}
return (
<Box
sx={{
width: 28,
height: 28,
borderRadius: '4px',
backgroundColor: '#9e9e9e',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
color: 'white',
flexShrink: 0
}}
>
<IconExclamationCircle size={16} />
</Box>
)
}
// Reset stale validation state when flow changes
useEffect(() => {
if (hasValidated) {
setHasValidated(false)
setResults([])
}
}, [nodes.length, edges.length]) // eslint-disable-line react-hooks/exhaustive-deps
return (
<>
<ClickAwayListener onClickAway={handleClose}>
<div ref={containerRef} style={{ position: 'absolute', right: 20, top: 20, zIndex: 1001 }}>
<Fab
size='small'
aria-label='validation'
title='Validate flow'
onClick={handleToggle}
sx={{
color: 'white',
backgroundColor: 'primary.main',
'&:hover': {
backgroundColor: 'primary.main',
backgroundImage: 'linear-gradient(rgb(0 0 0/10%) 0 0)'
}
}}
>
{open ? <IconX /> : <IconChecklist />}
</Fab>
{open && (
<Paper
elevation={16}
sx={{
position: 'absolute',
top: '100%',
right: 0,
mt: 1.5,
width: 400,
zIndex: 1200
}}
>
<Box sx={{ p: 2 }}>
<Typography variant='h6' sx={{ mt: 1, mb: 2, fontWeight: 600 }}>
Checklist ({results.reduce((sum, r) => sum + r.issues.length, 0)})
</Typography>
<Box sx={{ maxHeight: '60vh', overflowY: 'auto', pr: 1, mr: -1 }}>
{results.length > 0 ? (
results.map((item, index) => (
<Paper
key={index}
elevation={0}
sx={{
p: 2,
mb: 2,
backgroundColor: isDarkMode ? theme.palette.background.paper : theme.palette.grey[50],
borderRadius: '8px',
border: `1px solid ${alpha('#FFB938', isDarkMode ? 0.3 : 0.5)}`
}}
>
<Box sx={{ display: 'flex', alignItems: 'center', gap: 1, mb: 1 }}>
{getNodeIcon(item)}
<Typography sx={{ fontWeight: 500 }}>{item.label || item.name}</Typography>
</Box>
{item.issues.map((issue, issueIndex) => (
<Box
key={issueIndex}
sx={{
pt: 1.5,
px: 2,
pb: issueIndex === item.issues.length - 1 ? 1.5 : 0.5,
backgroundColor: isDarkMode ? darken('#FFB938', 0.85) : lighten('#FFB938', 0.9),
display: 'flex',
alignItems: 'center',
gap: 1.5,
borderTopLeftRadius: issueIndex === 0 ? '8px' : 0,
borderTopRightRadius: issueIndex === 0 ? '8px' : 0,
borderBottomLeftRadius: issueIndex === item.issues.length - 1 ? '8px' : 0,
borderBottomRightRadius: issueIndex === item.issues.length - 1 ? '8px' : 0
}}
>
<IconExclamationCircle
color='#FFB938'
size={20}
style={{ minWidth: 20, flexShrink: 0 }}
/>
<Typography variant='body2'>{issue}</Typography>
</Box>
))}
</Paper>
))
) : (
<Box sx={{ p: 3, textAlign: 'center' }}>
<img
style={{ objectFit: 'cover', height: '15vh', width: 'auto' }}
src={validateEmptyImage}
alt='Illustration of a checklist with no items, indicating no issues found'
/>
{hasValidated ? (
<Typography variant='body2' color='success.main' sx={{ mt: 2, fontWeight: 500 }}>
No issues found in your flow!
</Typography>
) : (
<Typography variant='body2' color='text.secondary' sx={{ mt: 2 }}>
Click &quot;Validate flow&quot; to check for issues
</Typography>
)}
</Box>
)}
</Box>
<Box sx={{ display: 'flex', justifyContent: 'center', mt: 2, mb: 1 }}>
<Button
variant='contained'
color='primary'
onClick={handleValidate}
startIcon={<IconChecklist size={18} />}
sx={{ minWidth: 120, textTransform: 'none' }}
>
Validate flow
</Button>
</Box>
</Box>
</Paper>
)}
</div>
</ClickAwayListener>
{/* Success toast */}
<Snackbar
open={successSnackbar}
autoHideDuration={3000}
onClose={() => setSuccessSnackbar(false)}
anchorOrigin={{ vertical: 'bottom', horizontal: 'left' }}
>
<Alert onClose={() => setSuccessSnackbar(false)} severity='success' variant='filled' sx={{ width: '100%' }}>
No issues found in your flow!
</Alert>
</Snackbar>
</>
)
}
export const ValidationFeedback = memo(ValidationFeedbackComponent)
@@ -8,3 +8,4 @@ export { NodeModelConfigs } from './NodeModelConfigs'
export { getMinimumNodeHeight, NodeOutputHandles } from './NodeOutputHandles'
export { NodeStatusIndicator, NodeWarningIndicator } from './NodeStatusIndicator'
export { NodeToolbarActions } from './NodeToolbarActions'
export { ValidationFeedback } from './ValidationFeedback'
@@ -47,16 +47,16 @@ function AgentFlowNodeComponent({ data }: AgentFlowNodeProps) {
openNodeEditor(data.id)
}
const hasValidationErrors = (data.validationErrors?.length ?? 0) > 0
const outputAnchors = data.outputAnchors ?? []
const minHeight = getMinimumNodeHeight(outputAnchors.length)
useEffect(() => {
if (data.warning) {
setWarningMessage(data.warning)
} else {
setWarningMessage('')
}
}, [data.name, data.version, data.warning])
const messages: string[] = []
if (data.warning) messages.push(data.warning)
if (data.validationErrors?.length) messages.push(...data.validationErrors)
setWarningMessage(messages.join('\n'))
}, [data.name, data.version, data.warning, data.validationErrors])
return (
<div ref={ref} onMouseEnter={() => setIsHovered(true)} onMouseLeave={() => setIsHovered(false)} onDoubleClick={handleDoubleClick}>
@@ -70,8 +70,8 @@ function AgentFlowNodeComponent({ data }: AgentFlowNodeProps) {
<CardWrapper
content={false}
sx={{
borderColor: stateColor,
borderWidth: '1px',
borderColor: hasValidationErrors ? '#FFB938' : stateColor,
borderWidth: hasValidationErrors ? '2px' : '1px',
boxShadow: data.selected ? `0 0 0 1px ${stateColor} !important` : 'none',
minHeight,
height: 'auto',
@@ -2,6 +2,7 @@ import { makeFlowNode, makeNodeData } from '@test-utils/factories'
import { act, renderHook } from '@testing-library/react'
import type { FlowNode, NodeData } from '@/core/types'
import { checkNodePlacementConstraints } from '@/core/validation'
// --- Tests ---
import { DROP_OFFSET_X, DROP_OFFSET_Y, useDragAndDrop } from './useDragAndDrop'
@@ -26,7 +27,13 @@ jest.mock('@/infrastructure/store', () => ({
jest.mock('@/core', () => ({
getUniqueNodeId: jest.fn((_data: NodeData, _nodes: FlowNode[]) => 'new-node-1'),
getUniqueNodeLabel: jest.fn((_data: NodeData, _nodes: FlowNode[]) => 'New Node 1'),
initNode: jest.fn((data: NodeData, id: string) => ({ ...data, id }))
initNode: jest.fn((data: NodeData, id: string) => ({ ...data, id })),
resolveNodeType: jest.fn(() => 'agentflowNode')
}))
jest.mock('@/core/validation', () => ({
checkNodePlacementConstraints: jest.fn(() => ({ valid: true })),
findParentIterationNode: jest.fn(() => null)
}))
function makeDragEvent(data?: string): React.DragEvent {
@@ -57,8 +64,8 @@ describe('useDragAndDrop', () => {
}
})
function renderUseDragAndDrop() {
return renderHook(() => useDragAndDrop({ nodes, setLocalNodes, reactFlowWrapper }))
function renderUseDragAndDrop(overrides = {}) {
return renderHook(() => useDragAndDrop({ nodes, setLocalNodes, reactFlowWrapper, ...overrides }))
}
describe('handleDragOver', () => {
@@ -137,6 +144,20 @@ describe('useDragAndDrop', () => {
expect(mockSetDirty).not.toHaveBeenCalled()
})
it('should not add node when placement constraint fails', () => {
;(checkNodePlacementConstraints as jest.Mock).mockReturnValueOnce({ valid: false, message: 'Only one start node' })
const onConstraintViolation = jest.fn()
const { result } = renderUseDragAndDrop({ onConstraintViolation })
const event = makeDragEvent(JSON.stringify(nodeData))
act(() => {
result.current.handleDrop(event)
})
expect(onConstraintViolation).toHaveBeenCalledWith('Only one start node')
expect(setLocalNodes).not.toHaveBeenCalled()
})
it('should catch and log JSON parse errors', () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
const { result } = renderUseDragAndDrop()
@@ -1,8 +1,9 @@
import { RefObject, useCallback } from 'react'
import { useReactFlow } from 'reactflow'
import { getUniqueNodeId, getUniqueNodeLabel, initNode } from '@/core'
import { getUniqueNodeId, getUniqueNodeLabel, initNode, resolveNodeType } from '@/core'
import type { FlowNode, NodeData } from '@/core/types'
import { checkNodePlacementConstraints, findParentIterationNode } from '@/core/validation'
import { useAgentflowContext } from '@/infrastructure/store'
// Offset to center the dropped node on the cursor position.
@@ -14,12 +15,13 @@ interface UseDragAndDropProps {
nodes: FlowNode[]
setLocalNodes: React.Dispatch<React.SetStateAction<FlowNode[]>>
reactFlowWrapper: RefObject<HTMLDivElement>
onConstraintViolation?: (message: string) => void
}
/**
* Hook for handling drag and drop of nodes onto the canvas
*/
export function useDragAndDrop({ nodes, setLocalNodes, reactFlowWrapper }: UseDragAndDropProps) {
export function useDragAndDrop({ nodes, setLocalNodes, reactFlowWrapper, onConstraintViolation }: UseDragAndDropProps) {
const { setDirty } = useAgentflowContext()
const reactFlowInstance = useReactFlow()
@@ -52,14 +54,26 @@ export function useDragAndDrop({ nodes, setLocalNodes, reactFlowWrapper }: UseDr
y: event.clientY - reactFlowBounds.top - DROP_OFFSET_Y
})
// Check placement constraints (start node, nested iteration, human input in iteration)
const constraintCheck = checkNodePlacementConstraints(nodes, nodeData.name, position)
if (!constraintCheck.valid) {
onConstraintViolation?.(constraintCheck.message!)
return
}
// Determine if dropped inside an iteration node
const parentNode = findParentIterationNode(nodes, position)
const newId = getUniqueNodeId(nodeData, nodes)
const newLabel = getUniqueNodeLabel(nodeData, nodes)
const initializedData = initNode(nodeData, newId, true)
const newNode: FlowNode = {
id: newId,
type: 'agentflowNode',
position,
data: { ...initializedData, label: newLabel }
type: resolveNodeType(nodeData.type ?? ''),
position: parentNode ? { x: position.x - parentNode.position.x, y: position.y - parentNode.position.y } : position,
data: { ...initializedData, label: newLabel },
...(parentNode ? { parentNode: parentNode.id, extent: 'parent' as const } : {})
}
setLocalNodes((nds) => [...nds, newNode])
@@ -68,7 +82,7 @@ export function useDragAndDrop({ nodes, setLocalNodes, reactFlowWrapper }: UseDr
console.error('[Agentflow] Failed to parse dropped node data:', error)
}
},
[nodes, reactFlowInstance, setLocalNodes, setDirty, reactFlowWrapper]
[nodes, reactFlowInstance, setLocalNodes, setDirty, reactFlowWrapper, onConstraintViolation]
)
return {
@@ -3,7 +3,9 @@ import type { Node } from 'reactflow'
import { makeFlowEdge, makeFlowNode, makeNodeData } from '@test-utils/factories'
import { act, renderHook } from '@testing-library/react'
import { isValidConnectionAgentflowV2 } from '@/core'
import type { FlowEdge, FlowNode, NodeData } from '@/core/types'
import { checkNodePlacementConstraints } from '@/core/validation'
import { useFlowHandlers } from './useFlowHandlers'
@@ -29,7 +31,12 @@ jest.mock('@/core', () => ({
getUniqueNodeId: jest.fn((_data: NodeData, _nodes: FlowNode[]) => 'new-node-1'),
getUniqueNodeLabel: jest.fn((_data: NodeData, _nodes: FlowNode[]) => 'New Node 1'),
initNode: jest.fn((data: NodeData, id: string) => ({ ...data, id })),
isValidConnectionAgentflowV2: jest.fn(() => true)
isValidConnectionAgentflowV2: jest.fn(() => true),
resolveNodeType: jest.fn(() => 'agentflowNode')
}))
jest.mock('@/core/validation', () => ({
checkNodePlacementConstraints: jest.fn(() => ({ valid: true }))
}))
describe('useFlowHandlers', () => {
@@ -100,6 +107,19 @@ describe('useFlowHandlers', () => {
expect(mockSetDirty).toHaveBeenCalledWith(true)
})
it('should not connect when isValidConnectionAgentflowV2 returns false', () => {
;(isValidConnectionAgentflowV2 as jest.Mock).mockReturnValueOnce(false)
const { result } = renderUseFlowHandlers()
act(() => {
result.current.handleConnect({ source: 'a', target: 'b', sourceHandle: null, targetHandle: null })
})
expect(setLocalEdges).not.toHaveBeenCalled()
expect(mockSetDirty).not.toHaveBeenCalled()
expect(onFlowChange).not.toHaveBeenCalled()
})
it('should not call onFlowChange when source or target is missing', () => {
const { result } = renderUseFlowHandlers()
@@ -287,6 +307,19 @@ describe('useFlowHandlers', () => {
expect(mockSetDirty).toHaveBeenCalledWith(true)
})
it('should not add node when placement constraint fails', () => {
;(checkNodePlacementConstraints as jest.Mock).mockReturnValueOnce({ valid: false, message: 'Only one start node' })
const onConstraintViolation = jest.fn()
const { result } = renderUseFlowHandlers({ availableNodes, onConstraintViolation })
act(() => {
result.current.handleAddNode('llmAgentflow')
})
expect(onConstraintViolation).toHaveBeenCalledWith('Only one start node')
expect(setLocalNodes).not.toHaveBeenCalled()
})
it('should do nothing when nodeType is not found in availableNodes', () => {
const { result } = renderUseFlowHandlers({ availableNodes })
@@ -1,8 +1,9 @@
import { useCallback, useRef } from 'react'
import { addEdge, applyEdgeChanges, applyNodeChanges, Connection, EdgeChange, Node, NodeChange } from 'reactflow'
import { getNodeColor, getUniqueNodeId, getUniqueNodeLabel, initNode, isValidConnectionAgentflowV2 } from '@/core'
import { getNodeColor, getUniqueNodeId, getUniqueNodeLabel, initNode, isValidConnectionAgentflowV2, resolveNodeType } from '@/core'
import type { FlowDataCallback, FlowEdge, FlowNode, NodeData } from '@/core/types'
import { checkNodePlacementConstraints } from '@/core/validation'
import { useAgentflowContext } from '@/infrastructure/store'
interface UseFlowHandlersProps {
@@ -14,6 +15,7 @@ interface UseFlowHandlersProps {
onEdgesChange: (changes: EdgeChange[]) => void
onFlowChange?: FlowDataCallback
availableNodes: NodeData[]
onConstraintViolation?: (message: string) => void
}
/**
@@ -27,7 +29,8 @@ export function useFlowHandlers({
onNodesChange,
onEdgesChange,
onFlowChange,
availableNodes
availableNodes,
onConstraintViolation
}: UseFlowHandlersProps) {
const { state, setDirty } = useAgentflowContext()
@@ -148,12 +151,20 @@ export function useFlowHandlers({
const nodeData = availableNodes.find((n) => n.name === nodeType)
if (!nodeData) return
// Check placement constraints (start node, nested iteration, human input in iteration)
const constraintCheck = checkNodePlacementConstraints(nodes, nodeType, position)
if (!constraintCheck.valid) {
onConstraintViolation?.(constraintCheck.message!)
return
}
const newId = getUniqueNodeId(nodeData, nodes)
const newLabel = getUniqueNodeLabel(nodeData, nodes)
const initializedData = initNode(nodeData, newId, true)
const newNode: FlowNode = {
id: newId,
type: 'agentflowNode',
type: resolveNodeType(nodeData.type ?? ''),
position: position || { x: 100, y: 100 },
data: { ...initializedData, label: newLabel }
}
@@ -173,7 +184,7 @@ export function useFlowHandlers({
viewport: getViewport()
})
},
[availableNodes, nodes, edges, setLocalNodes, setDirty, getViewport]
[availableNodes, nodes, edges, setLocalNodes, setDirty, getViewport, onConstraintViolation]
)
return {
+1
View File
@@ -0,0 +1 @@
/// <reference types="vite/client" />