-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
feat(core): Implement Dynamic Parameters within regular nodes used as AI Tools #10862
Conversation
dc0b605
to
636dc14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bravo! This is awesome work!
I have a couple of smaller comments / questions, but one thing that we definitely need to address is the parsing of the arguments to $fromAI
. One thing that came to mind is that we may need to do more proper parsing, because users might be inclined to do things we don't anticipate yet, that are valid javascript.
Also, as discussed, I encountered an issue with Claude, when returning plain results from the HN node; I'm not sure what to do there, but maybe we need to wrap the results in something that's more easily digestible for some models?
if (!(remainingPath[i] in current)) { | ||
current[remainingPath[i]] = {}; | ||
// Handle quotes (single or double) | ||
if (char === "'" || char === '"') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this handle escaped characters properly? I tried it, and got some weird results:
argsString: 'database-filter', 'the filter\\'s keyword to apply to the database', string, ['a\\'b', 'b']
result: {
key: 'database-filter',
description: "the filter\\\\'s keyword to apply to the database', string, ['a\\\\'b",
type: "b']",
defaultValue: undefined
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I improved the parsing method and added a few more tests; could you have another look?
b9e43de
to
34ef547
Compare
34ef547
to
2c20981
Compare
Thanks for the review @jeanpaul! I improved the parsing logic and added a few more tests, could you re-review, pls? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Awesome expansion of the tests as well.
Left another few small comments, but other than that looks great.
continue; | ||
} | ||
|
||
if (char === '"' || char === "'") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add backticks as quote-chars as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated + added unit tests
@jeanpaul I updated the PR with the fixes product and reorganized unit tests + removed some duplicates. Could you have one last look? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small gotcha (and a quick check for my understanding :))
✅ All Cypress E2E specs passed |
n8n Run #7127
Run Properties:
|
Project |
n8n
|
Branch Review |
ai-285-implement-placeholders
|
Run status |
Passed #7127
|
Run duration | 04m 33s |
Commit |
193f913c0d: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 OlegIvaniv 🗃️ e2e/*
|
Committer | Oleg Ivaniv |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
437
|
View all changes introduced in this branch ↗︎ |
Got released with |
Summary
This PR introduces a new feature that allows LLM to dynamically interact with n8n nodes when used as tools. The core of this feature is the new
$fromAI()
function, which allows for the definition of parameters that can be populated by AI models during workflow execution.CleanShot.2024-09-17.at.21.50.55.mp4
Key Changes
$fromAI()
function for defining AI-powered parameters in node configurations.WorkflowDataProxy
to handle$fromAI()
calls and retrieve values passed by LLMs.createNodeAsTool
function to support AI tool conversions, including automatic description generation based on resource and operation.Implementation Details
$fromAI Function
The
$fromAI()
function is implemented in theWorkflowDataProxy
class and accepts four parameters:name
: A string identifier for the parameter (required)description
: A string describing the parameter's purpose (optional)type
: A string specifying the data type (optional, defaults to 'string')defaultValue
: A fallback value if no AI input is provided (optional)Example usage:
Parsing and Schema Generation
parseFromAIArguments
function uses a regex to extract all$fromAI()
calls from the node parameters.FromAIArgument
object using theparseArguments
function.generateZodSchema
function creates a Zod schema based on the parsed arguments, which is used for validation and type-checking.Node Conversion to AI Tools
The
convertNodeToAiTool
function has been updated to:descriptionType
property allowing users to choose between automatic and manual description generation.toolDescription
property for manual description input when needed.WorkflowDataProxy Updates
$fromAI
method inWorkflowDataProxy
retrieves values passed by LLMs from therunExecutionData
.CodeMirror Integration
$fromAI()
to the list of root dollar completions in the CodeMirror plugin.Node Updates
Several core nodes (including Airtable, Baserow, Google Calendar, Gmail, Google Sheets, HackerNews, Postgres, Supabase, Telegram) have been updated with a
usableAsTool: true
property to indicate their compatibility with this new feature.Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)