fix(core): Optimize connection type lookups #17585

cstuncsik · 2025-07-23T13:00:15Z

Summary

Fixes performance bottleneck in workflow runners by optimizing the hasPath method to avoid unnecessary node type loading during expression evaluation.

Related Linear tickets, Github issues, and Community forum posts

PAY-3158

Review / Merge checklist

PR title and summary are descriptive. (conventions)
Docs updated or follow-up ticket created.
Tests included.
PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

cubic-dev-ai

cubic analysis

2 issues found across 1 file • Review in cubic

_{React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-07-23T13:04:58Z

packages/workflow/src/workflow.ts

+			// Collect connection types from source connections
+			for (const nodeConnections of Object.values(this.connectionsBySourceNode)) {
+				for (const connectionType of Object.keys(nodeConnections)) {
+					connectionTypes.add(connectionType as NodeConnectionType);


Rule violated: Prefer Typeguards over Type casting

Directly casting `connectionType` to `NodeConnectionType` violates the "Prefer Typeguards over Type casting" rule. Replace the `as` assertion with a type guard or collect keys into a `Set<string>` and validate before use.

Prompt for AI agents

Address the following comment on packages/workflow/src/workflow.ts at line 1041: <comment>Directly casting `connectionType` to `NodeConnectionType` violates the "Prefer Typeguards over Type casting" rule. Replace the `as` assertion with a type guard or collect keys into a `Set<string>` and validate before use.</comment> <file context> @@ -1032,16 +1032,24 @@ export class Workflow { visited.add(nodeName); - // Check all connection types for this node - const allConnectionTypes = [ - NodeConnectionTypes.Main, - NodeConnectionTypes.AiTool, - NodeConnectionTypes.AiMemory, - NodeConnectionTypes.AiDocument, </file context>

cubic-dev-ai · 2025-07-23T13:04:58Z

packages/workflow/src/workflow.ts

-
-			for (const connectionType of allConnectionTypes) {
+			// Get connection types that actually exist in this workflow
+			const connectionTypes = new Set<NodeConnectionType>();


connectionTypes is recomputed on every BFS iteration, iterating over the entire connection map each time and causing avoidable O(N²) overhead; compute it once outside the while-loop instead.

Prompt for AI agents

Address the following comment on packages/workflow/src/workflow.ts at line 1036: <comment>`connectionTypes` is recomputed on every BFS iteration, iterating over the entire connection map each time and causing avoidable O(N²) overhead; compute it once outside the while-loop instead.</comment> <file context> @@ -1032,16 +1032,24 @@ export class Workflow { visited.add(nodeName); - // Check all connection types for this node - const allConnectionTypes = [ - NodeConnectionTypes.Main, - NodeConnectionTypes.AiTool, - NodeConnectionTypes.AiMemory, - NodeConnectionTypes.AiDocument, </file context>

…trieval for more performance

codecov · 2025-07-23T13:38:43Z

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
packages/workflow/src/workflow.ts	94.11%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

…rieval efficiency

…main output handling

…thod to cover various node connection scenarios

…nodes and streamline connection retrieval

cstuncsik · 2025-07-23T15:57:47Z

packages/workflow/src/workflow.ts

+		for (const connectionType of nonMainConnectionTypes) {
+			const connections = nodeConnections[connectionType] || [];
+			for (const connectionGroup of connections) {
+				for (const connection of connectionGroup || []) {
+					if (connection?.node) {
+						const returnNode = this.getNode(connection.node);
+						if (!returnNode) {
+							throw new ApplicationError(`Node "${connection.node}" not found`);
+						}
+						return this.getParentMainInputNode(returnNode);
+					}
 				}
 			}
 		}


I prefer the more declarative (functional programming) to the iterative way and if it doesn't decrease performance that much I'd go with this

Suggested change

for (const connectionType of nonMainConnectionTypes) {

const connections = nodeConnections[connectionType] || [];

for (const connectionGroup of connections) {

for (const connection of connectionGroup || []) {

if (connection?.node) {

const returnNode = this.getNode(connection.node);

if (!returnNode) {

throw new ApplicationError(`Node "${connection.node}" not found`);

}

return this.getParentMainInputNode(returnNode);

}

}

}

}

const parentNode = nonMainConnectionTypes

.flatMap((connectionType) => nodeConnections[connectionType] || [])

.flatMap((connectionGroup) => connectionGroup || [])

.map((connection) => {

if (connection?.node) {

const returnNode = this.getNode(connection.node);

if (!returnNode) {

throw new ApplicationError(`Node "${connection.node}" not found`);

}

return this.getParentMainInputNode(returnNode);

}

})

.find(Boolean);

return parentNode || node;

Did a quick benchmark, and the iterative will perform better on complex graphs

Simple Chain (5 nodes): Declarative was 63.6% faster - this is likely due to JIT optimization on simple cases

Complex Graph (20 nodes): Declarative was 11.6% slower

Deep Nesting (10 levels): Declarative was 18.6% slower

Single Node (baseline): Declarative was 38.3% slower

Average performance ratio: 1.013 (declarative slightly slower overall)

ivov · 2025-07-24T15:54:24Z

packages/workflow/src/workflow.ts

-		if (node) {
-			const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
-			const outputs = NodeHelpers.getNodeOutputs(this, node, nodeType.description);
+		if (!node) return node;


Either the argument type should reflect this is nullish, or this check is unnecessary?

ivov · 2025-07-24T15:54:46Z

packages/workflow/src/workflow.ts

-			const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
-			const outputs = NodeHelpers.getNodeOutputs(this, node, nodeType.description);


Is it safe not to check outputs anymore?

Since I'm not an expert on this part, I discussed it with claude code and it said yes.
I can share the reasoning with you if you're interested

Nobody is an expert on this ancient stuff :)

Since this is the core of the PR, can you please verify? I trust your judgment, not Claude's.

Do you think we still don't have enough tests to catch if something is wrong with this?

packages/workflow/src/workflow.ts

ivov · 2025-07-24T15:57:54Z

packages/workflow/src/workflow.ts

+				...Object.values(this.connectionsBySourceNode),
+				...Object.values(this.connectionsByDestinationNode),
+			].flatMap((nodeConnections) =>
+				Object.keys(nodeConnections).filter((key): key is NodeConnectionType => !!key),


Is this filter a no-op? Should it be like this?

].flatMap((nodeConnections) => Object.keys(nodeConnections) as NodeConnectionType[]),

I was to avoid type casting
I added them back
I just remember Mutasem setting up some rule to avoid using it

There are many cases where asserting (not casting) is reasonable if we know the keys. Nice article about it here.

packages/workflow/src/workflow.ts

ivov · 2025-07-24T16:06:28Z

packages/workflow/test/workflow.test.ts

 			expect(result).toBe(set1Node);
 		});
+
+		describe('nodes with only main outputs', () => {


Hats off, nice coverage!

…ndefined nodes and improve connection type retrieval

…al null parentMainInputNode

…otential null results

…m getParentMainInputNode

bundlemon · 2025-07-25T11:13:20Z

BundleMon

Unchanged files (2)

Status	Path	Size	Limits
✅	WASM Dependencies _{tree-sitter-bash.wasm}	181.26KB	-
✅	WASM Dependencies _{tree-sitter.wasm}	74.47KB	-

No change in files bundle size

Groups updated (2)

Status	Path	Size	Limits
✅	*/.js	5.3MB (+166.36KB +3.16%)	-
✅	*/.css	184.52KB (+5.47KB +3.06%)	-

Final result: ✅

View report in BundleMon website ➡️

Current branch size history | Target branch size history

ivov

https://n8nio.slack.com/archives/C0970H8FLLF/p1753445408751269?thread_ts=1753437427.196029&cid=C0970H8FLLF

ivov

Haven't tested, change looks clean.

currents-bot · 2025-07-25T13:21:27Z

✅

E2E Tests: n8n tests passed after 5m 27.4s

🟢 521 · 🔴 0 · ⚪️ 1

View Run Details

Run Details

Project: n8n
Groups: 3
Framework: Playwright
Run Status: Passed
Commit: 69b456a
Spec files: 110
Overall tests: 522
Duration: 5m 27.4s
Parallelization: 3

Groups

GroupId	Results	Spec Files Progress
mode:standard - Parallel	🟢 9 · 🔴 0 · ⚪️ 1	3 / 3
mode:standard - Sequential	🟢 4 · 🔴 0 · ⚪️ 0	2 / 2
No name	🟢 508 · 🔴 0 · ⚪️ 0	105 / 105

This message was posted automatically by currents.dev | Integration Settings

janober · 2025-07-28T15:13:23Z

Got released with n8n@1.105.0

This reverts commit 70eab1b.

… (#18382)

…io#17759, and n8n-io#17585 (n8n-io#18382)

fix(workflow): Optimize hasPath method to use existing connection types

31583eb

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jul 23, 2025

cubic-dev-ai bot reviewed Jul 23, 2025

View reviewed changes

cstuncsik requested a review from ivov July 23, 2025 13:08

fix(workflow): Refactor hasPath method to optimize connection type re…

bb9661f

…trieval for more performance

cstuncsik added 4 commits July 23, 2025 15:51

fix(workflow): Simplify hasPath method to enhance connection type ret…

1c0f361

…rieval efficiency

fix(workflow): Optimize getParentMainInputNode method to improve non-…

7c18d22

…main output handling

test(workflow): Add comprehensive tests for getParentMainInputNode me…

a0bd9d8

…thod to cover various node connection scenarios

fix(workflow): Optimize getParentMainInputNode method to handle null …

1fa665b

…nodes and streamline connection retrieval

cstuncsik commented Jul 23, 2025

View reviewed changes

cstuncsik changed the title ~~fix(core): Optimize hasPath method to use existing connection types~~ fix(core): Optimize ensureValidPath method to use existing connection types Jul 23, 2025

cstuncsik changed the title ~~fix(core): Optimize ensureValidPath method to use existing connection types~~ fix(core): Optimize connection type lookups Jul 23, 2025

ivov reviewed Jul 24, 2025

View reviewed changes

cstuncsik added 4 commits July 25, 2025 12:31

fix(workflow): Enhance getParentMainInputNode method to handle null/u…

74bc19d

…ndefined nodes and improve connection type retrieval

fix(workflow): Update contextNode assignment to safely handle potenti…

39954d4

…al null parentMainInputNode

fix(workflow): Update getParentMainInputNode tests to safely handle p…

18f9ab4

…otential null results

fix(workflow): Ensure contextNode assignment handles null results fro…

69b456a

…m getParentMainInputNode

cstuncsik requested a review from ivov July 25, 2025 11:22

ivov reviewed Jul 25, 2025

View reviewed changes

ivov approved these changes Jul 25, 2025

View reviewed changes

cstuncsik merged commit 70eab1b into master Jul 25, 2025
78 checks passed

cstuncsik deleted the pay-3158-optimize-haspath-method-to-avoid-unnecessary-node-type branch July 25, 2025 14:12

This was referenced Jul 28, 2025

🚀 Release 1.105.0 #17745

Closed

🚀 Release 1.105.0 #17752

Merged

janober added the Released label Jul 28, 2025

cstuncsik added a commit that referenced this pull request Aug 1, 2025

fix(core): Optimize connection type lookups (#17585)

dc9422c

cstuncsik added a commit that referenced this pull request Aug 1, 2025

fix(core): Optimize connection type lookups (#17585)

63c7e93

despairblue pushed a commit that referenced this pull request Aug 4, 2025

fix(core): Optimize connection type lookups (#17585)

bc0a7b0

r00gm added a commit that referenced this pull request Aug 15, 2025

Revert "fix(core): Optimize connection type lookups (#17585)"

65fcf98

This reverts commit 70eab1b.

r00gm mentioned this pull request Aug 15, 2025

fix(editor): Revert changes from PRs #16059, #17959, #17759, and #17585 #18382

Merged

4 tasks

tomi pushed a commit that referenced this pull request Aug 15, 2025

fix(editor): Revert changes from PRs #16059, #17959, #17759, and #17585…

62f4361

… (#18382)

r00gm added a commit that referenced this pull request Aug 15, 2025

fix(editor): Revert changes from PRs #16059, #17959, #17759, and #17585…

27bda02

… (#18382)

This was referenced Aug 15, 2025

🚀 Release 1.107.2 #18412

Merged

🚀 Release 1.108.0 #18487

Closed

🚀 Release 1.108.0 #18499

Merged

pllusin pushed a commit to pllusin/n8n that referenced this pull request Sep 30, 2025

fix(editor): Revert changes from PRs n8n-io#16059, n8n-io#17959, n8n-…

9511100

…io#17759, and n8n-io#17585 (n8n-io#18382)

		const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
		const outputs = NodeHelpers.getNodeOutputs(this, node, nodeType.description);

fix(core): Optimize connection type lookups #17585

fix(core): Optimize connection type lookups #17585

Uh oh!

Conversation

cstuncsik commented Jul 23, 2025

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

cubic analysis

Uh oh!

cubic-dev-ai bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cstuncsik Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bundlemon bot commented Jul 25, 2025

BundleMon

Uh oh!

ivov left a comment

Choose a reason for hiding this comment

Uh oh!

ivov left a comment

Choose a reason for hiding this comment

Uh oh!

currents-bot bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

janober commented Jul 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jul 23, 2025 •

edited

Loading

cstuncsik Jul 23, 2025 •

edited

Loading

currents-bot bot commented Jul 25, 2025 •

edited

Loading