From 55e3edd89c42baf931db8695367e3555bfa1db91 Mon Sep 17 00:00:00 2001 From: Oleg Ivaniv Date: Tue, 5 Nov 2024 17:57:11 +0100 Subject: [PATCH 1/4] WIP: Only auto-parse when parsing fails --- .../agents/Agent/agents/ToolsAgent/execute.ts | 17 +++++++++++++---- .../output_parsers/N8nOutputFixingParser.ts | 7 ++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts index 84d775d0f53e2..5f4260513c574 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts @@ -206,10 +206,19 @@ export async function toolsAgentExecute(this: IExecuteFunctions): Promise; + let parserInput: string; + + if (finalResponse instanceof Object) { + if ('output' in finalResponse) { + parserInput = JSON.stringify({ output: jsonParse(finalResponse.output) }); + } else { + parserInput = JSON.stringify(finalResponse); + } + } else { + parserInput = finalResponse; + } + + const returnValues = (await outputParser.parse(parserInput)) as Record; return handleParsedStepOutput(returnValues); } return handleAgentFinishOutput(steps); diff --git a/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts b/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts index eec3b0c1871d9..8492f953bde6a 100644 --- a/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts +++ b/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts @@ -1,7 +1,7 @@ import type { Callbacks } from '@langchain/core/callbacks/manager'; import type { BaseLanguageModel } from '@langchain/core/language_models/base'; import type { AIMessage } from '@langchain/core/messages'; -import { BaseOutputParser } from '@langchain/core/output_parsers'; +import { BaseOutputParser, OutputParserException } from '@langchain/core/output_parsers'; import type { ISupplyDataFunctions } from 'n8n-workflow'; import { NodeConnectionType } from 'n8n-workflow'; @@ -47,6 +47,10 @@ export class N8nOutputFixingParser extends BaseOutputParser { return response; } catch (error) { + if (error instanceof OutputParserException) { + console.log('Is output parser exception!'); + } + console.log('Got error while parsing', error); try { // Second attempt: use retry chain to fix the output const result = (await this.getRetryChain().invoke({ @@ -65,6 +69,7 @@ export class N8nOutputFixingParser extends BaseOutputParser { return parsed; } catch (autoParseError) { + console.log('Got final error while parsing', autoParseError); // If both attempts fail, add the error to the output and throw this.context.addOutputData(NodeConnectionType.AiOutputParser, index, autoParseError); throw autoParseError; From fb6a0785181adf610cd4a73a13117a440cf04629 Mon Sep 17 00:00:00 2001 From: Oleg Ivaniv Date: Tue, 5 Nov 2024 18:50:42 +0100 Subject: [PATCH 2/4] fix(Auto-fixing Output Parser Node): Only run retry chain on parsing errors --- .../agents/Agent/agents/ToolsAgent/execute.ts | 6 +- .../OutputParserAutofixing.node.ts | 36 +++- .../OutputParserAutofixing/prompt.ts | 16 ++ .../test/OutputParserAutofixing.node.test.ts | 170 ++++++++++++------ .../output_parsers/N8nOutputFixingParser.ts | 13 +- 5 files changed, 181 insertions(+), 60 deletions(-) create mode 100644 packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserAutofixing/prompt.ts diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts index 5f4260513c574..405928b19ac29 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts @@ -210,7 +210,11 @@ export async function toolsAgentExecute(this: IExecuteFunctions): Promise { let outputParser: OutputParserAutofixing; @@ -34,6 +38,13 @@ describe('OutputParserAutofixing', () => { throw new ApplicationError('Unexpected connection type'); }); + thisArg.getNodeParameter.mockReset(); + thisArg.getNodeParameter.mockImplementation((parameterName) => { + if (parameterName === 'options.prompt') { + return NAIVE_FIX_PROMPT; + } + throw new ApplicationError('Not implemented'); + }); }); afterEach(() => { @@ -48,73 +59,132 @@ describe('OutputParserAutofixing', () => { }); } - it('should successfully parse valid output without needing to fix it', async () => { - const validOutput = { name: 'Alice', age: 25 }; - - mockStructuredOutputParser.parse.mockResolvedValueOnce(validOutput); + describe('Configuration', () => { + it('should throw error when prompt template does not contain {error} placeholder', async () => { + thisArg.getNodeParameter.mockImplementation((parameterName) => { + if (parameterName === 'options.prompt') { + return 'Invalid prompt without error placeholder'; + } + throw new ApplicationError('Not implemented'); + }); + + await expect(outputParser.supplyData.call(thisArg, 0)).rejects.toThrow( + new NodeOperationError( + thisArg.getNode(), + 'Auto-fixing parser prompt has to contain {error} placeholder', + ), + ); + }); - const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { - response: N8nOutputFixingParser; - }; + it('should throw error when prompt template is empty', async () => { + thisArg.getNodeParameter.mockImplementation((parameterName) => { + if (parameterName === 'options.prompt') { + return ''; + } + throw new ApplicationError('Not implemented'); + }); + + await expect(outputParser.supplyData.call(thisArg, 0)).rejects.toThrow( + new NodeOperationError( + thisArg.getNode(), + 'Auto-fixing parser prompt has to contain {error} placeholder', + ), + ); + }); - // Ensure the response contains the output-fixing parser - expect(response).toBeDefined(); - expect(response).toBeInstanceOf(N8nOutputFixingParser); + it('should use default prompt when none specified', async () => { + thisArg.getNodeParameter.mockImplementation((parameterName) => { + if (parameterName === 'options.prompt') { + return NAIVE_FIX_PROMPT; + } + throw new ApplicationError('Not implemented'); + }); - const result = await response.parse('{"name": "Alice", "age": 25}'); + const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { + response: N8nOutputFixingParser; + }; - // Validate that the parser succeeds without retry - expect(result).toEqual(validOutput); - expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(1); // Only one call to parse + expect(response).toBeDefined(); + }); }); - it('should throw an error when both structured parser and fixing parser fail', async () => { - mockStructuredOutputParser.parse - .mockRejectedValueOnce(new Error('Invalid JSON')) // First attempt fails - .mockRejectedValueOnce(new Error('Fixing attempt failed')); // Second attempt fails + describe('Parsing', () => { + it('should successfully parse valid output without needing to fix it', async () => { + const validOutput = { name: 'Alice', age: 25 }; - const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { - response: N8nOutputFixingParser; - }; + mockStructuredOutputParser.parse.mockResolvedValueOnce(validOutput); - response.getRetryChain = getMockedRetryChain('{}'); + const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { + response: N8nOutputFixingParser; + }; - await expect(response.parse('Invalid JSON string')).rejects.toThrow('Fixing attempt failed'); - expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(2); - }); + const result = await response.parse('{"name": "Alice", "age": 25}'); - it('should reject on the first attempt and succeed on retry with the parsed content', async () => { - const validOutput = { name: 'Bob', age: 28 }; + expect(result).toEqual(validOutput); + expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(1); + }); - mockStructuredOutputParser.parse.mockRejectedValueOnce(new Error('Invalid JSON')); + it('should not retry on non-OutputParserException errors', async () => { + const error = new Error('Some other error'); + mockStructuredOutputParser.parse.mockRejectedValueOnce(error); - const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { - response: N8nOutputFixingParser; - }; + const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { + response: N8nOutputFixingParser; + }; - response.getRetryChain = getMockedRetryChain(JSON.stringify(validOutput)); + await expect(response.parse('Invalid JSON string')).rejects.toThrow(error); + expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(1); + }); - mockStructuredOutputParser.parse.mockResolvedValueOnce(validOutput); + it('should retry on OutputParserException and succeed', async () => { + const validOutput = { name: 'Bob', age: 28 }; - const result = await response.parse('Invalid JSON string'); + mockStructuredOutputParser.parse + .mockRejectedValueOnce(new OutputParserException('Invalid JSON')) + .mockResolvedValueOnce(validOutput); - expect(result).toEqual(validOutput); - expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(2); // First fails, second succeeds - }); + const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { + response: N8nOutputFixingParser; + }; - it('should handle non-JSON formatted response from fixing parser', async () => { - mockStructuredOutputParser.parse.mockRejectedValueOnce(new Error('Invalid JSON')); + response.getRetryChain = getMockedRetryChain(JSON.stringify(validOutput)); - const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { - response: N8nOutputFixingParser; - }; + const result = await response.parse('Invalid JSON string'); - response.getRetryChain = getMockedRetryChain('This is not JSON'); + expect(result).toEqual(validOutput); + expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(2); + }); - mockStructuredOutputParser.parse.mockRejectedValueOnce(new Error('Unexpected token')); + it('should handle failed retry attempt', async () => { + mockStructuredOutputParser.parse + .mockRejectedValueOnce(new OutputParserException('Invalid JSON')) + .mockRejectedValueOnce(new Error('Still invalid JSON')); - // Expect the structured parser to throw an error on invalid JSON from retry - await expect(response.parse('Invalid JSON string')).rejects.toThrow('Unexpected token'); - expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(2); // First fails, second tries and fails + const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { + response: N8nOutputFixingParser; + }; + + response.getRetryChain = getMockedRetryChain('Still not valid JSON'); + + await expect(response.parse('Invalid JSON string')).rejects.toThrow('Still invalid JSON'); + expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(2); + }); + + it('should throw non-OutputParserException errors immediately without retry', async () => { + const customError = new Error('Database connection error'); + const retryChainSpy = jest.fn(); + + mockStructuredOutputParser.parse.mockRejectedValueOnce(customError); + + const { response } = (await outputParser.supplyData.call(thisArg, 0)) as { + response: N8nOutputFixingParser; + }; + + response.getRetryChain = retryChainSpy; + + await expect(response.parse('Some input')).rejects.toThrow('Database connection error'); + expect(mockStructuredOutputParser.parse).toHaveBeenCalledTimes(1); + expect(retryChainSpy).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts b/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts index 8492f953bde6a..de07bfc7cf971 100644 --- a/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts +++ b/packages/@n8n/nodes-langchain/utils/output_parsers/N8nOutputFixingParser.ts @@ -2,11 +2,11 @@ import type { Callbacks } from '@langchain/core/callbacks/manager'; import type { BaseLanguageModel } from '@langchain/core/language_models/base'; import type { AIMessage } from '@langchain/core/messages'; import { BaseOutputParser, OutputParserException } from '@langchain/core/output_parsers'; +import type { PromptTemplate } from '@langchain/core/prompts'; import type { ISupplyDataFunctions } from 'n8n-workflow'; import { NodeConnectionType } from 'n8n-workflow'; import type { N8nStructuredOutputParser } from './N8nStructuredOutputParser'; -import { NAIVE_FIX_PROMPT } from './prompt'; import { logAiEvent } from '../helpers'; export class N8nOutputFixingParser extends BaseOutputParser { @@ -16,12 +16,13 @@ export class N8nOutputFixingParser extends BaseOutputParser { private context: ISupplyDataFunctions, private model: BaseLanguageModel, private outputParser: N8nStructuredOutputParser, + private fixPromptTemplate: PromptTemplate, ) { super(); } getRetryChain() { - return NAIVE_FIX_PROMPT.pipe(this.model); + return this.fixPromptTemplate.pipe(this.model); } /** @@ -47,15 +48,14 @@ export class N8nOutputFixingParser extends BaseOutputParser { return response; } catch (error) { - if (error instanceof OutputParserException) { - console.log('Is output parser exception!'); + if (!(error instanceof OutputParserException)) { + throw error; } - console.log('Got error while parsing', error); try { // Second attempt: use retry chain to fix the output const result = (await this.getRetryChain().invoke({ completion, - error, + error: error.message, instructions: this.getFormatInstructions(), })) as AIMessage; @@ -69,7 +69,6 @@ export class N8nOutputFixingParser extends BaseOutputParser { return parsed; } catch (autoParseError) { - console.log('Got final error while parsing', autoParseError); // If both attempts fail, add the error to the output and throw this.context.addOutputData(NodeConnectionType.AiOutputParser, index, autoParseError); throw autoParseError; From f0f9762680d8eed8c8a13195768712d77a5674cd Mon Sep 17 00:00:00 2001 From: Oleg Ivaniv Date: Tue, 5 Nov 2024 18:55:58 +0100 Subject: [PATCH 3/4] Add comment --- .../nodes/agents/Agent/agents/ToolsAgent/execute.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts index 405928b19ac29..25e6e789846e9 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts @@ -211,11 +211,16 @@ export async function toolsAgentExecute(this: IExecuteFunctions): Promise Date: Wed, 6 Nov 2024 12:11:26 +0100 Subject: [PATCH 4/4] Improve hint and description --- .../OutputParserAutofixing/OutputParserAutofixing.node.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserAutofixing/OutputParserAutofixing.node.ts b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserAutofixing/OutputParserAutofixing.node.ts index 68246a5caad41..4f385e1770014 100644 --- a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserAutofixing/OutputParserAutofixing.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserAutofixing/OutputParserAutofixing.node.ts @@ -75,13 +75,16 @@ export class OutputParserAutofixing implements INodeType { default: {}, options: [ { - displayName: 'Fix Prompt', + displayName: 'Retry Prompt', name: 'prompt', type: 'string', default: NAIVE_FIX_PROMPT, typeOptions: { rows: 10, }, + hint: 'Should include "{error}", "{instructions}", and "{completion}" placeholders', + description: + 'Prompt template used for fixing the output. Uses placeholders: "{instructions}" for parsing rules, "{completion}" for the failed attempt, and "{error}" for the validation error message.', }, ], },