Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Remove run data of utility nodes for partial executions v2 #12673

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
import type { IRunData } from 'n8n-workflow';
// NOTE: Diagrams in this file have been created with https://asciiflow.com/#/
// If you update the tests, please update the diagrams as well.
// If you add a test, please create a new diagram.
//
// Map
// 0 means the output has no run data
// 1 means the output has run data
// ►► denotes the node that the user wants to execute to
// XX denotes that the node is disabled
// PD denotes that the node has pinned data

import { NodeConnectionType, type IRunData } from 'n8n-workflow';

import { createNodeData, toITaskData } from './helpers';
import { cleanRunData } from '../clean-run-data';
Expand Down Expand Up @@ -111,4 +122,73 @@ describe('cleanRunData', () => {
[node1.name]: [toITaskData([{ data: { value: 1 } }])],
});
});

// ┌─────┐ ┌────────┐
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth it to test for multi-root case?

Capture 2025-01-20 at 11 30 52@2x

// │node1├─────►rootNode│
// └─────┘ └───▲────┘
// │
// ┌───┴───┐
// │subNode│
// └───────┘
test('removes run data of sub nodes when the start node is a root node', () => {
// ARRANGE
const node1 = createNodeData({ name: 'Node1' });
const rootNode = createNodeData({ name: 'Root Node' });
const subNode = createNodeData({ name: 'Sub Node' });
const graph = new DirectedGraph()
.addNodes(node1, rootNode, subNode)
.addConnections(
{ from: node1, to: rootNode },
{ from: subNode, to: rootNode, type: NodeConnectionType.AiLanguageModel },
);
const runData: IRunData = {
[node1.name]: [toITaskData([{ data: { value: 1 } }])],
[rootNode.name]: [toITaskData([{ data: { value: 2 } }])],
[subNode.name]: [toITaskData([{ data: { value: 3 } }])],
};

// ACT
const newRunData = cleanRunData(runData, graph, new Set([rootNode]));

// ASSERT
expect(newRunData).toEqual({
[node1.name]: [toITaskData([{ data: { value: 1 } }])],
});
});

// ┌─────┐ ┌─────┐ ┌────────┐
// │node1├───►node2├────►rootNode│
// └─────┘ └─────┘ └───▲────┘
// │
// ┌───┴───┐
// │subNode│
// └───────┘
test('removes run data of sub nodes for root nodes downstream of the start node', () => {
// ARRANGE
const node1 = createNodeData({ name: 'Node1' });
const node2 = createNodeData({ name: 'Node2' });
const rootNode = createNodeData({ name: 'Root Node' });
const subNode = createNodeData({ name: 'Sub Node' });
const graph = new DirectedGraph()
.addNodes(node1, node2, rootNode, subNode)
.addConnections(
{ from: node1, to: node2 },
{ from: node2, to: rootNode },
{ from: subNode, to: rootNode, type: NodeConnectionType.AiLanguageModel },
);
const runData: IRunData = {
[node1.name]: [toITaskData([{ data: { value: 1 } }])],
[node2.name]: [toITaskData([{ data: { value: 1 } }])],
[rootNode.name]: [toITaskData([{ data: { value: 2 } }])],
[subNode.name]: [toITaskData([{ data: { value: 3 } }])],
};

// ACT
const newRunData = cleanRunData(runData, graph, new Set([node2]));

// ASSERT
expect(newRunData).toEqual({
[node1.name]: [toITaskData([{ data: { value: 1 } }])],
});
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { INode, IRunData } from 'n8n-workflow';
import { NodeConnectionType, type INode, type IRunData } from 'n8n-workflow';

import type { DirectedGraph } from './directed-graph';

Expand All @@ -16,10 +16,22 @@ export function cleanRunData(

for (const startNode of startNodes) {
delete newRunData[startNode.name];

const children = graph.getChildren(startNode);
for (const node of [startNode, ...children]) {
delete newRunData[node.name];

// Delete runData for subNodes
const subNodeConnections = graph.getParentConnections(node);
for (const subNodeConnection of subNodeConnections) {
// Sub nodes never use the Main connection type, so this filters our
// the connection that goes upstream of the startNode.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Sub nodes never use the Main connection type, so this filters our
// Sub nodes never use the Main connection type, so this filters

if (subNodeConnection.type === NodeConnectionType.Main) {
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Subnodes can use any connection type other than Main? Or they can use only one or more specific connection types? If the latter, maybe we should check for those rather than leaving it open.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK main connections are always the ones that connect nodes with one another, and all other types currently are used by the AI root nodes to connect sub nodes:

export const enum NodeConnectionType {
AiAgent = 'ai_agent',
AiChain = 'ai_chain',
AiDocument = 'ai_document',
AiEmbedding = 'ai_embedding',
AiLanguageModel = 'ai_languageModel',
AiMemory = 'ai_memory',
AiOutputParser = 'ai_outputParser',
AiRetriever = 'ai_retriever',
AiTextSplitter = 'ai_textSplitter',
AiTool = 'ai_tool',
AiVectorStore = 'ai_vectorStore',
Main = 'main',
}

@OlegIvaniv is that correct?

}

for (const child of children) {
delete newRunData[child.name];
delete newRunData[subNodeConnection.from.name];
}
}
}

Expand Down
Loading