Skip to content

feat: Add model selector node #16371

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

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

schrothbn
Copy link
Contributor

Summary

Adds a new node that selects one of the connected language models depending on customizable conditions.

To make that possible some adjustments needed to be made in the core:

  • Support multiple connection definitions of the same type for sub node connections (including validating required and maxConnections)
  • N8nLlmTracing now (optionally) keeps track of the parent run index to connect dependant runs correctly
  • Adjusted filtering in the LogPanel to correctly associate runs between sub nodes of sub nodes.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/AI-982/choosing-model-based-on-expression-having-a-fallback-model-when-a

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)

@schrothbn schrothbn requested a review from a team June 16, 2025 07:06
@schrothbn schrothbn self-assigned this Jun 16, 2025
@schrothbn schrothbn requested review from ggozad and removed request for a team June 16, 2025 07:06
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic found 5 issues across 14 files. Review them in cubic.dev

React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.

Copy link

codecov bot commented Jun 16, 2025

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jun 16, 2025
@schrothbn schrothbn requested review from OlegIvaniv and removed request for ggozad June 16, 2025 08:18
Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

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

Nice one! Seems to be mostly working as expected, there's just a couple of issues I noticed when traversing nodes from NDV:

  1. Model selector icon looks too dark:
    CleanShot 2025-06-17 at 09 29 34@2x
  2. The connected models are rendering for both inputs, not just the connected one:
    CleanShot 2025-06-17 at 09 29 42@2x

}
models.reverse();

const rules = this.getNodeParameter('rules.rule', itemIndex, []) as any[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we improve the type here? Something like:

{
			modelIndex: number;
			conditions: {
				options: {
					caseSensitive: boolean;
					leftValue: string;
					typeValidation: string;
					version: number;
				};
				conditions: Array<{
					id: string;
					leftValue: string;
					rightValue: string;
					operator: {
						type: string;
						operation: string;
						name: string;
					};
				}>;
				combinator: string;
			};
		}

Comment on lines +141 to +144
const rule = rules[i] as {
modelIndex: string;
};
const modelIndex = parseInt(rule.modelIndex, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does modelIndex really come as a string? On my instance I see it as number, so we might not even need to parseInt it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants