fix(editor): Fix node graph generation for evaluation node in set metrics mode (#18344)

packages/frontend/editor-ui/src/composables/useCanvasOperations.test.ts

@@ -51,11 +51,15 @@ import { useTelemetry } from './useTelemetry';
 import { useToast } from '@/composables/useToast';
 import * as nodeHelpers from '@/composables/useNodeHelpers';
 
+import { TelemetryHelpers } from 'n8n-workflow';
+
 vi.mock('n8n-workflow', async (importOriginal) => {
-	const actual = await importOriginal<{}>();
+	// eslint-disable-next-line @typescript-eslint/consistent-type-imports
+	const actual = await importOriginal<typeof import('n8n-workflow')>();
 	return {
 		...actual,
 		TelemetryHelpers: {
+			...actual.TelemetryHelpers,
 			generateNodesGraph: vi.fn().mockReturnValue({
 				nodeGraph: {
 					nodes: [],
@@ -2726,34 +2730,6 @@ describe('useCanvasOperations', () => {
 		});
 	});
 
-	describe('duplicateNodes', () => {
-		it('should duplicate nodes', async () => {
-			const workflowsStore = mockedStore(useWorkflowsStore);
-			const nodeTypesStore = useNodeTypesStore();
-			const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE });
-
-			nodeTypesStore.nodeTypes = {
-				[SET_NODE_TYPE]: { 1: nodeTypeDescription },
-			};
-
-			const nodes = buildImportNodes();
-			workflowsStore.setNodes(nodes);
-			workflowsStore.getNodesByIds.mockReturnValue(nodes);
-			workflowsStore.outgoingConnectionsByNodeName.mockReturnValue({});
-
-			const workflowObject = createTestWorkflowObject(workflowsStore.workflow);
-			workflowsStore.workflowObject = workflowObject;
-			workflowsStore.createWorkflowObject.mockReturnValue(workflowObject);
-
-			const canvasOperations = useCanvasOperations();
-			const duplicatedNodeIds = await canvasOperations.duplicateNodes(['1', '2']);
-
-			expect(duplicatedNodeIds.length).toBe(2);
-			expect(duplicatedNodeIds).not.toContain('1');
-			expect(duplicatedNodeIds).not.toContain('2');
-		});
-	});
-
 	describe('copyNodes', () => {
 		it('should copy nodes', async () => {
 			const workflowsStore = mockedStore(useWorkflowsStore);
@@ -3420,6 +3396,69 @@ describe('useCanvasOperations', () => {
 		});
 	});
 
+	describe('duplicateNodes', () => {
+		it('should duplicate nodes', async () => {
+			const workflowsStore = mockedStore(useWorkflowsStore);
+			const nodeTypesStore = useNodeTypesStore();
+			const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE });
+
+			nodeTypesStore.nodeTypes = {
+				[SET_NODE_TYPE]: { 1: nodeTypeDescription },
+			};
+
+			const nodes = buildImportNodes();
+			workflowsStore.setNodes(nodes);
+			workflowsStore.getNodesByIds.mockReturnValue(nodes);
+			workflowsStore.outgoingConnectionsByNodeName.mockReturnValue({});
+
+			const workflowObject = createTestWorkflowObject(workflowsStore.workflow);
+			workflowsStore.workflowObject = workflowObject;
+			workflowsStore.createWorkflowObject.mockReturnValue(workflowObject);
+
+			const canvasOperations = useCanvasOperations();
+			const duplicatedNodeIds = await canvasOperations.duplicateNodes(['1', '2']);
+
+			expect(duplicatedNodeIds.length).toBe(2);
+			expect(duplicatedNodeIds).not.toContain('1');
+			expect(duplicatedNodeIds).not.toContain('2');
+		});
+
+		it('should not crash when TelemetryHelpers.generateNodesGraph throws error', async () => {
+			const workflowsStore = mockedStore(useWorkflowsStore);
+			const telemetry = useTelemetry();
+			const nodeTypesStore = useNodeTypesStore();
+			const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE });
+
+			nodeTypesStore.nodeTypes = {
+				[SET_NODE_TYPE]: { 1: nodeTypeDescription },
+			};
+
+			const nodes = buildImportNodes();
+			workflowsStore.setNodes(nodes);
+			workflowsStore.getNodesByIds.mockReturnValue(nodes);
+			workflowsStore.outgoingConnectionsByNodeName.mockReturnValue({});
+
+			const workflowObject = createTestWorkflowObject(workflowsStore.workflow);
+			workflowsStore.workflowObject = workflowObject;
+			workflowsStore.createWorkflowObject.mockReturnValue(workflowObject);
+
+			// Mock TelemetryHelpers.generateNodesGraph to throw an error for this test
+			vi.mocked(TelemetryHelpers.generateNodesGraph).mockImplementationOnce(() => {
+				throw new Error('Telemetry generation failed');
+			});
+
+			// This should not throw even when telemetry fails
+			const canvasOperations = useCanvasOperations();
+			await expect(canvasOperations.duplicateNodes(['1', '2'])).resolves.not.toThrow();
+
+			// Telemetry should not be tracked when generation fails
+			expect(telemetry.track).not.toHaveBeenCalledWith(
+				'User imported workflow',
+				expect.any(Object),
+			);
+		});
+	});
+
 	describe('importTemplate', () => {
 		it('should import template to canvas', async () => {
 			const projectsStore = mockedStore(useProjectsStore);

packages/frontend/editor-ui/src/composables/useCanvasOperations.ts

@@ -1916,37 +1916,41 @@ export function useCanvasOperations() {
 
 			removeUnknownCredentials(workflowData);
 
-			const nodeGraph = JSON.stringify(
-				TelemetryHelpers.generateNodesGraph(
-					workflowData as IWorkflowBase,
-					workflowHelpers.getNodeTypes(),
-					{
-						nodeIdMap,
-						sourceInstanceId:
-							workflowData.meta && workflowData.meta.instanceId !== rootStore.instanceId
-								? workflowData.meta.instanceId
-								: '',
-						isCloudDeployment: settingsStore.isCloudDeployment,
-					},
-				).nodeGraph,
-			);
+			try {
+				const nodeGraph = JSON.stringify(
+					TelemetryHelpers.generateNodesGraph(
+						workflowData as IWorkflowBase,
+						workflowHelpers.getNodeTypes(),
+						{
+							nodeIdMap,
+							sourceInstanceId:
+								workflowData.meta && workflowData.meta.instanceId !== rootStore.instanceId
+									? workflowData.meta.instanceId
+									: '',
+							isCloudDeployment: settingsStore.isCloudDeployment,
+						},
+					).nodeGraph,
+				);
 
-			if (source === 'paste') {
-				telemetry.track('User pasted nodes', {
-					workflow_id: workflowsStore.workflowId,
-					node_graph_string: nodeGraph,
-				});
-			} else if (source === 'duplicate') {
-				telemetry.track('User duplicated nodes', {
-					workflow_id: workflowsStore.workflowId,
-					node_graph_string: nodeGraph,
-				});
-			} else {
-				telemetry.track('User imported workflow', {
-					source,
-					workflow_id: workflowsStore.workflowId,
-					node_graph_string: nodeGraph,
-				});
+				if (source === 'paste') {
+					telemetry.track('User pasted nodes', {
+						workflow_id: workflowsStore.workflowId,
+						node_graph_string: nodeGraph,
+					});
+				} else if (source === 'duplicate') {
+					telemetry.track('User duplicated nodes', {
+						workflow_id: workflowsStore.workflowId,
+						node_graph_string: nodeGraph,
+					});
+				} else {
+					telemetry.track('User imported workflow', {
+						source,
+						workflow_id: workflowsStore.workflowId,
+						node_graph_string: nodeGraph,
+					});
+				}
+			} catch {
+				// If telemetry fails, don't throw an error
 			}
 
 			// Fix the node position as it could be totally offscreen

packages/workflow/src/telemetry-helpers.ts

@@ -42,6 +42,7 @@ import type {
 import { NodeConnectionTypes } from './interfaces';
 import { getNodeParameters } from './node-helpers';
 import { jsonParse } from './utils';
+import { DEFAULT_EVALUATION_METRIC } from './evaluation-helpers';
 
 const isNodeApiError = (error: unknown): error is NodeApiError =>
 	typeof error === 'object' && error !== null && 'name' in error && error?.name === 'NodeApiError';
@@ -414,11 +415,18 @@ export function generateNodesGraph(
 			options?.isCloudDeployment &&
 			node.parameters?.operation === 'setMetrics'
 		) {
-			const metrics = node.parameters?.metrics as IDataObject;
+			const metrics = node.parameters?.metrics as IDataObject | undefined;
 
-			nodeItem.metric_names = (metrics.assignments as Array<{ name: string }> | undefined)?.map(
-				(metric: { name: string }) => metric.name,
-			);
+			// If metrics are not defined, it means the node is using preconfigured metric
+			if (!metrics) {
+				const predefinedMetricKey =
+					(node.parameters?.metric as string | undefined) ?? DEFAULT_EVALUATION_METRIC;
+				nodeItem.metric_names = [predefinedMetricKey];
+			} else {
+				nodeItem.metric_names = (metrics.assignments as Array<{ name: string }> | undefined)?.map(
+					(metric: { name: string }) => metric.name,
+				);
+			}
 		} else if (node.type === CODE_NODE_TYPE) {
 			const { language } = node.parameters;
 			nodeItem.language =

packages/workflow/test/telemetry-helpers.test.ts

@@ -28,6 +28,7 @@ import {
 	userInInstanceRanOutOfFreeAiCredits,
 } from '../src/telemetry-helpers';
 import { randomInt } from '../src/utils';
+import { DEFAULT_EVALUATION_METRIC } from '../src/evaluation-helpers';
 
 describe('getDomainBase should return protocol plus domain', () => {
 	test('in valid URLs', () => {
@@ -1418,6 +1419,136 @@ describe('generateNodesGraph', () => {
 			evaluationTriggerNodeNames: [],
 		});
 	});
+
+	test('should handle Evaluation node with undefined metrics - uses default predefined metric', () => {
+		const workflow: Partial<IWorkflowBase> = {
+			nodes: [
+				{
+					parameters: {
+						operation: 'setMetrics',
+						// metrics is undefined - should fall back to default metric
+					},
+					id: 'eval-node-id',
+					name: 'Evaluation Node',
+					type: 'n8n-nodes-base.evaluation',
+					typeVersion: 1,
+					position: [100, 100],
+				},
+			],
+			connections: {},
+			pinData: {},
+		};
+
+		expect(generateNodesGraph(workflow, nodeTypes, { isCloudDeployment: true })).toEqual({
+			nodeGraph: {
+				node_types: ['n8n-nodes-base.evaluation'],
+				node_connections: [],
+				nodes: {
+					'0': {
+						id: 'eval-node-id',
+						type: 'n8n-nodes-base.evaluation',
+						version: 1,
+						position: [100, 100],
+						metric_names: [DEFAULT_EVALUATION_METRIC], // Default metric
+					},
+				},
+				notes: {},
+				is_pinned: false,
+			},
+			nameIndices: { 'Evaluation Node': '0' },
+			webhookNodeNames: [],
+			evaluationTriggerNodeNames: [],
+		});
+	});
+
+	test('should handle Evaluation node with custom metric parameter', () => {
+		const workflow: Partial<IWorkflowBase> = {
+			nodes: [
+				{
+					parameters: {
+						operation: 'setMetrics',
+						metric: 'helpfulness',
+						// metrics is undefined but metric parameter is set
+					},
+					id: 'eval-node-id',
+					name: 'Evaluation Node',
+					type: 'n8n-nodes-base.evaluation',
+					typeVersion: 1,
+					position: [100, 100],
+				},
+			],
+			connections: {},
+			pinData: {},
+		};
+
+		expect(generateNodesGraph(workflow, nodeTypes, { isCloudDeployment: true })).toEqual({
+			nodeGraph: {
+				node_types: ['n8n-nodes-base.evaluation'],
+				node_connections: [],
+				nodes: {
+					'0': {
+						id: 'eval-node-id',
+						type: 'n8n-nodes-base.evaluation',
+						version: 1,
+						position: [100, 100],
+						metric_names: ['helpfulness'], // Custom metric from parameter
+					},
+				},
+				notes: {},
+				is_pinned: false,
+			},
+			nameIndices: { 'Evaluation Node': '0' },
+			webhookNodeNames: [],
+			evaluationTriggerNodeNames: [],
+		});
+	});
+
+	test('should handle Evaluation node with valid metrics assignments', () => {
+		const workflow: Partial<IWorkflowBase> = {
+			nodes: [
+				{
+					parameters: {
+						operation: 'setMetrics',
+						metrics: {
+							assignments: [
+								{ name: 'accuracy', value: 0.95 },
+								{ name: 'precision', value: 0.87 },
+								{ name: 'recall', value: 0.92 },
+							],
+						},
+					},
+					id: 'eval-node-id',
+					name: 'Evaluation Node',
+					type: 'n8n-nodes-base.evaluation',
+					typeVersion: 1,
+					position: [100, 100],
+				},
+			],
+			connections: {},
+			pinData: {},
+		};
+
+		expect(generateNodesGraph(workflow, nodeTypes, { isCloudDeployment: true })).toEqual({
+			nodeGraph: {
+				node_types: ['n8n-nodes-base.evaluation'],
+				node_connections: [],
+				nodes: {
+					'0': {
+						id: 'eval-node-id',
+						type: 'n8n-nodes-base.evaluation',
+						version: 1,
+						position: [100, 100],
+						metric_names: ['accuracy', 'precision', 'recall'],
+					},
+				},
+				notes: {},
+				is_pinned: false,
+			},
+			nameIndices: { 'Evaluation Node': '0' },
+			webhookNodeNames: [],
+			evaluationTriggerNodeNames: [],
+		});
+	});
 });
 
 describe('extractLastExecutedNodeCredentialData', () => {