diff --git a/packages/@n8n/instance-ai/evaluations/harness/scenario-execution.ts b/packages/@n8n/instance-ai/evaluations/harness/scenario-execution.ts index 8aaba0834cf..d2574f95d5a 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/scenario-execution.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/scenario-execution.ts @@ -549,16 +549,20 @@ function buildScenarioContextBlock( ); } for (const req of nr.interceptedRequests) { - if ( - typeof req.mockResponse === 'object' && - req.mockResponse !== null && - '_evalMockError' in (req.mockResponse as Record) - ) { - const msg = (req.mockResponse as Record).message; + if (typeof req.mockResponse !== 'object' || req.mockResponse === null) continue; + const mockResponse = req.mockResponse as Record; + // `_evalMockError` = HTTP-mock generation failure; `evalMockGenerationError` + // = LLM wire-server generation/translation failure. Flag both. + if ('_evalMockError' in mockResponse) { + const msg = mockResponse.message; const msgStr = typeof msg === 'string' ? msg : 'unknown'; preAnalysis.push( `⚠ MOCK ISSUE: "${nodeName}" ${req.method} ${req.url} → mock generation failed: ${msgStr}`, ); + } else if (typeof mockResponse.evalMockGenerationError === 'string') { + preAnalysis.push( + `⚠ MOCK ISSUE: "${nodeName}" ${req.method} ${req.url} → mock generation failed: ${mockResponse.evalMockGenerationError}`, + ); } } } diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/llm-wire-server.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/llm-wire-server.test.ts index abe71346b0f..20e30bbb8e9 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/llm-wire-server.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/llm-wire-server.test.ts @@ -208,7 +208,43 @@ describe('LlmWireServer', () => { expect(intercepts[0].rootName).toBe('LLM Chain'); expect(intercepts[0].method).toBe('POST'); expect(intercepts[0].nodeType).toBe(subNode.type); - expect(intercepts[0].mockResponse).toEqual({ content: 'reply' }); + // The ledger records the wire envelope the SDK received, not the mock + // handler's `{ content }` shorthand — judges read this as the wire body. + expect(intercepts[0].mockResponse).toMatchObject({ + object: 'chat.completion', + choices: [{ message: { role: 'assistant', content: 'reply' } }], + }); + }); + + it('keeps the _evalMockError sentinel at the ledger top level so mock_issue stays attributable', async () => { + const intercepts: InterceptedTurn[] = []; + const mockHandler = vi.fn().mockResolvedValue({ + body: { _evalMockError: true, message: 'Mock generation failed: upstream 429' }, + headers: {}, + statusCode: 200, + }) as unknown as EvalLlmMockHandler; + + server = new LlmWireServer({ + logger: mockLogger, + mockHandler, + rootToSubNode: new Map([['Agent', subNode]]), + onIntercept: (t) => intercepts.push(t), + }); + const url = await server.start(); + + await postChatCompletion(url, '/eval/Agent/v1/chat/completions', { + model: 'gpt-4o', + messages: [{ role: 'user', content: 'ping' }], + }); + + // Consumers test `'_evalMockError' in mockResponse`; a translated chat + // envelope would bury the sentinel in choices[0].message.content and the + // failure would be misattributed to the builder. + expect(intercepts).toHaveLength(1); + expect(intercepts[0].mockResponse).toEqual({ + _evalMockError: true, + message: 'Mock generation failed: upstream 429', + }); }); it('still returns 200 with a valid envelope when onIntercept throws (ledger failure is isolated)', async () => { @@ -567,6 +603,11 @@ describe('LlmWireServer', () => { expect(intercepts).toHaveLength(1); expect(intercepts[0].rootName).toBe('Agent'); + // Streamed turns record the equivalent non-streamed wire envelope. + expect(intercepts[0].mockResponse).toMatchObject({ + object: 'chat.completion', + choices: [{ message: { content: 'streamed' } }], + }); }); it('uses the no-handler stub for streaming when no mock handler is attached', async () => { @@ -945,6 +986,12 @@ describe('LlmWireServer', () => { expect(intercepts).toHaveLength(1); expect(intercepts[0].rootName).toBe('My Agent'); + // The ledger records the canonical Responses envelope the SDK received — + // output[].content[].text — not the handler's internal shorthand. + expect(intercepts[0].mockResponse).toMatchObject({ + object: 'response', + output: [{ type: 'message', content: [{ type: 'output_text', text: 'ok' }] }], + }); // Reverse translator uses the canonical OpenAI URL so mock-handler's // service/endpoint extraction derives `/v1/responses` correctly. expect(intercepts[0].url).toBe('https://api.openai.com/v1/responses'); diff --git a/packages/cli/src/modules/instance-ai/eval/llm-wire-server.ts b/packages/cli/src/modules/instance-ai/eval/llm-wire-server.ts index 6835b896f4f..b9960dce017 100644 --- a/packages/cli/src/modules/instance-ai/eval/llm-wire-server.ts +++ b/packages/cli/src/modules/instance-ai/eval/llm-wire-server.ts @@ -4,6 +4,7 @@ import type { EvalLlmMockHandler, EvalMockHttpResponse } from 'n8n-core'; import type { IHttpRequestOptions, INode } from 'n8n-workflow'; import { type Server } from 'node:http'; +import { isMockErrorSentinel } from './mock-handler'; import { buildOpenAiErrorEnvelope, extractRequestModel, @@ -29,6 +30,17 @@ export interface InterceptedTurn { method: string; nodeType: string; requestBody: unknown; + /** + * Wire-level response body as served to the vendor SDK (post protocol + * translation) — NOT the mock handler's internal shorthand. Judges read this + * field as "what the node received"; recording the pre-translation shorthand + * made them misattribute node-side failures to a malformed mock envelope. + * For streamed turns this is the equivalent non-streamed envelope. + * + * Exception: generation/translation failures keep their sentinel shorthand + * (`_evalMockError` / `evalMockGenerationError`) so consumers can still + * attribute mock_issue — a wire envelope would hide it in message content. + */ mockResponse: unknown; } @@ -232,6 +244,41 @@ export class LlmWireServer { `[EvalMock] wire turn root="${rootName}" stream=${String(stream)} responseHead=${JSON.stringify(mockResponse?.body ?? null).slice(0, 300)}`, ); + // Translate to the wire envelope BEFORE the ledger write so the ledger + // records exactly what the SDK receives (a translator throw records an + // error turn instead of leaking the handler's internal shorthand). + let wireBody: Record; + try { + wireBody = adapter.forwardObject(mockResponse, model); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + this.options.logger.error( + `[EvalMock] Wire-server envelope translation failed for root "${rootName}": ${message}`, + ); + try { + this.options.onIntercept?.({ + rootName, + url: synthetic.url, + method: synthetic.method ?? 'POST', + nodeType: subNode.type, + requestBody: req.body, + mockResponse: { evalMockGenerationError: `envelope translation failed: ${message}` }, + }); + } catch { + // Ledger write must never block the error response. + } + this.respondWithError(adapter, res, message); + return; + } + + // A handler-returned generation failure carries the `_evalMockError` sentinel + // that ledger consumers key on to attribute mock_issue. Translating it into a + // vendor envelope buries the sentinel in assistant message content, so record + // the shorthand verbatim for this branch — a failed generation has no + // meaningful "what the node received" beyond the failure itself. + const ledgerBody = + mockResponse && isMockErrorSentinel(mockResponse) ? mockResponse.body : wireBody; + // Ledger write BEFORE the response so consumers see the entry deterministically // after `await fetch(...)`. `requestBody` is stored by reference (express.json // never re-touches it); callers must not mutate. A thrown `onIntercept` never @@ -243,7 +290,7 @@ export class LlmWireServer { method: synthetic.method ?? 'POST', nodeType: subNode.type, requestBody: req.body, - mockResponse: mockResponse?.body, + mockResponse: ledgerBody, }); } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -254,7 +301,7 @@ export class LlmWireServer { if (stream) { this.writeSseResponse(adapter, req, res, mockResponse, model); } else { - res.status(200).json(adapter.forwardObject(mockResponse, model)); + res.status(200).json(wireBody); } } catch (error) { const message = error instanceof Error ? error.message : String(error); diff --git a/packages/cli/src/modules/instance-ai/eval/mock-handler.ts b/packages/cli/src/modules/instance-ai/eval/mock-handler.ts index edd719649f4..99a20e03015 100644 --- a/packages/cli/src/modules/instance-ai/eval/mock-handler.ts +++ b/packages/cli/src/modules/instance-ai/eval/mock-handler.ts @@ -271,7 +271,7 @@ function buildMockCacheKey( } /** True for the fallback body `generateMockResponse` returns when all attempts failed. */ -function isMockErrorSentinel(response: EvalMockHttpResponse): boolean { +export function isMockErrorSentinel(response: EvalMockHttpResponse): boolean { const body = response.body; return ( typeof body === 'object' && body !== null && !Array.isArray(body) && '_evalMockError' in body