mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-01 15:47:41 +08:00
fix(ai-builder): Route eval report artifacts through --output-dir (no-changelog) (#35083)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,186 @@
|
||||
import { existsSync, mkdtempSync, readdirSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import path from 'path';
|
||||
|
||||
import type { EvalLogger } from '../harness/logger';
|
||||
import { writeScenarioVerificationSnapshot } from '../harness/scenario-execution';
|
||||
import { writeRunDebugReport } from '../report/run-debug-report';
|
||||
import { writeWorkflowReport } from '../report/workflow-report';
|
||||
import type { ChecklistResult, WorkflowTestCase, WorkflowTestCaseResult } from '../types';
|
||||
|
||||
// Pins artifact PLACEMENT for the lang-tracer dispatcher (lang-tracer
|
||||
// `packages/dispatcher/src/lib/runner.ts`): it runs several concurrent eval
|
||||
// children against ONE n8n checkout in one container — each child gets its own
|
||||
// `--output-dir`, and relies on that flag covering EVERY artifact the run
|
||||
// writes, not just `eval-results.json`. All three writers that used to hardcode
|
||||
// the package-level `.data` directory are covered here. The HTML reports are
|
||||
// the sharpest case: their filenames are stable
|
||||
// (`workflow-eval-report.html`, `workflow-eval-llm-debug.html`), so ignoring
|
||||
// `--output-dir` silently lets concurrent runs clobber each other's reports.
|
||||
// The no-arg default must stay `.data` for local dev and for n8n's own eval CI,
|
||||
// which uploads that path as a build artifact without passing `--output-dir`.
|
||||
|
||||
const DEFAULT_REPORT_DIR = path.join(__dirname, '..', '..', '.data');
|
||||
|
||||
const TEST_CASE: WorkflowTestCase = {
|
||||
conversation: [{ role: 'user', text: 'Build a Slack notifier' }],
|
||||
complexity: 'simple',
|
||||
tags: [],
|
||||
executionScenarios: [{ name: 's', description: 'd', dataSetup: '', successCriteria: 'ok' }],
|
||||
datasets: ['full'],
|
||||
};
|
||||
|
||||
const RESULTS: WorkflowTestCaseResult[] = [
|
||||
{
|
||||
testCase: TEST_CASE,
|
||||
workflowBuildSuccess: true,
|
||||
executionScenarioResults: [],
|
||||
fileSlug: 'slack-notifier',
|
||||
},
|
||||
];
|
||||
|
||||
function freshOutputDir(): string {
|
||||
return mkdtempSync(path.join(tmpdir(), 'eval-artifact-output-dir-'));
|
||||
}
|
||||
|
||||
describe('eval report artifacts — --output-dir contract', () => {
|
||||
describe('writeWorkflowReport', () => {
|
||||
it('writes the timestamped and stable reports into the given outputDir', () => {
|
||||
const outputDir = freshOutputDir();
|
||||
|
||||
const reportPath = writeWorkflowReport(RESULTS, outputDir);
|
||||
|
||||
expect(path.dirname(reportPath)).toBe(outputDir);
|
||||
expect(existsSync(reportPath)).toBe(true);
|
||||
expect(existsSync(path.join(outputDir, 'workflow-eval-report.html'))).toBe(true);
|
||||
});
|
||||
|
||||
it('creates the outputDir when it does not exist yet', () => {
|
||||
const outputDir = path.join(freshOutputDir(), 'nested', 'run-1');
|
||||
|
||||
const reportPath = writeWorkflowReport(RESULTS, outputDir);
|
||||
|
||||
expect(path.dirname(reportPath)).toBe(outputDir);
|
||||
expect(readdirSync(outputDir)).toContain('workflow-eval-report.html');
|
||||
});
|
||||
|
||||
it('falls back to the package .data directory when no outputDir is given', () => {
|
||||
const reportPath = writeWorkflowReport(RESULTS);
|
||||
|
||||
expect(path.dirname(reportPath)).toBe(DEFAULT_REPORT_DIR);
|
||||
expect(existsSync(path.join(DEFAULT_REPORT_DIR, 'workflow-eval-report.html'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('writeRunDebugReport', () => {
|
||||
it('writes the timestamped and stable reports into the given outputDir', () => {
|
||||
const outputDir = freshOutputDir();
|
||||
|
||||
const reportPath = writeRunDebugReport(RESULTS, outputDir);
|
||||
|
||||
expect(path.dirname(reportPath)).toBe(outputDir);
|
||||
expect(existsSync(reportPath)).toBe(true);
|
||||
expect(existsSync(path.join(outputDir, 'workflow-eval-llm-debug.html'))).toBe(true);
|
||||
});
|
||||
|
||||
it('creates the outputDir when it does not exist yet', () => {
|
||||
const outputDir = path.join(freshOutputDir(), 'nested', 'run-2');
|
||||
|
||||
const reportPath = writeRunDebugReport(RESULTS, outputDir);
|
||||
|
||||
expect(path.dirname(reportPath)).toBe(outputDir);
|
||||
expect(readdirSync(outputDir)).toContain('workflow-eval-llm-debug.html');
|
||||
});
|
||||
|
||||
it('falls back to the package .data directory when no outputDir is given', () => {
|
||||
const reportPath = writeRunDebugReport(RESULTS);
|
||||
|
||||
expect(path.dirname(reportPath)).toBe(DEFAULT_REPORT_DIR);
|
||||
expect(existsSync(path.join(DEFAULT_REPORT_DIR, 'workflow-eval-llm-debug.html'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('writeScenarioVerificationSnapshot', () => {
|
||||
const CHECKLIST_RESULT: ChecklistResult = {
|
||||
id: 1,
|
||||
pass: true,
|
||||
reasoning: 'digest arrived',
|
||||
strategy: 'llm',
|
||||
};
|
||||
|
||||
/** Collects warnings so a swallowed write error reads differently from a
|
||||
* snapshot correctly written somewhere else. */
|
||||
function collectingLogger(): { logger: EvalLogger; warnings: string[] } {
|
||||
const warnings: string[] = [];
|
||||
const logger: EvalLogger = {
|
||||
info: () => {},
|
||||
verbose: () => {},
|
||||
success: () => {},
|
||||
warn: (msg: string) => warnings.push(msg),
|
||||
error: (msg: string) => warnings.push(msg),
|
||||
isVerbose: false,
|
||||
};
|
||||
return { logger, warnings };
|
||||
}
|
||||
|
||||
async function writeSnapshot(testCaseName: string, outputDir?: string): Promise<string[]> {
|
||||
const { logger, warnings } = collectingLogger();
|
||||
await writeScenarioVerificationSnapshot({
|
||||
testCaseName,
|
||||
scenarioName: 'happy path',
|
||||
workflowId: 'wf-1',
|
||||
passed: true,
|
||||
result: CHECKLIST_RESULT,
|
||||
verificationResults: [CHECKLIST_RESULT],
|
||||
verifierAttempts: [],
|
||||
logger,
|
||||
outputDir,
|
||||
});
|
||||
expect(warnings).toEqual([]);
|
||||
return warnings;
|
||||
}
|
||||
|
||||
it('writes the snapshot into the given outputDir', async () => {
|
||||
const outputDir = freshOutputDir();
|
||||
|
||||
await writeSnapshot('daily digest', outputDir);
|
||||
|
||||
const names = readdirSync(outputDir);
|
||||
expect(names).toHaveLength(1);
|
||||
expect(names[0]).toMatch(/^daily-digest_happy-path_.*\.json$/);
|
||||
});
|
||||
|
||||
it('creates the outputDir when it does not exist yet', async () => {
|
||||
const outputDir = path.join(freshOutputDir(), 'nested', 'run-3');
|
||||
|
||||
await writeSnapshot('daily digest', outputDir);
|
||||
|
||||
expect(readdirSync(outputDir)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('falls back to the package .data directory when no outputDir is given', async () => {
|
||||
const caseName = `fallback case ${String(Date.now())}`;
|
||||
|
||||
await writeSnapshot(caseName);
|
||||
|
||||
const slug = caseName.replace(/[^a-z0-9]+/gi, '-').toLowerCase();
|
||||
expect(readdirSync(DEFAULT_REPORT_DIR).some((n) => n.startsWith(`${slug}_`))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('gives concurrent runs their own copy of the stable-named reports', () => {
|
||||
const dirA = freshOutputDir();
|
||||
const dirB = freshOutputDir();
|
||||
|
||||
writeWorkflowReport(RESULTS, dirA);
|
||||
writeRunDebugReport(RESULTS, dirA);
|
||||
writeWorkflowReport(RESULTS, dirB);
|
||||
writeRunDebugReport(RESULTS, dirB);
|
||||
|
||||
for (const dir of [dirA, dirB]) {
|
||||
const names = readdirSync(dir);
|
||||
expect(names).toContain('workflow-eval-report.html');
|
||||
expect(names).toContain('workflow-eval-llm-debug.html');
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -124,6 +124,7 @@ async function main(): Promise<void> {
|
||||
gate,
|
||||
slugByTestCase,
|
||||
commitSha,
|
||||
outputDir: args.outputDir,
|
||||
jsonPath,
|
||||
prCommentPath,
|
||||
experimentName: args.experimentName,
|
||||
|
||||
@@ -68,6 +68,7 @@ export async function executeAgentScenario(
|
||||
timeoutMs?: number,
|
||||
testCaseName?: string,
|
||||
buildTrace?: BuildTrace,
|
||||
outputDir?: string,
|
||||
): Promise<ExecutionScenarioResult> {
|
||||
const execStart = Date.now();
|
||||
const projectId = await client.getPersonalProjectId();
|
||||
@@ -130,6 +131,7 @@ export async function executeAgentScenario(
|
||||
verifierAttempts: verification.attempts,
|
||||
buildTrace,
|
||||
logger,
|
||||
outputDir,
|
||||
});
|
||||
const incomplete = verificationResults.length === 0;
|
||||
const attemptErrors = verification.attempts
|
||||
|
||||
@@ -54,13 +54,17 @@ export async function writeScenarioVerificationSnapshot(input: {
|
||||
verifierAttempts: VerifierAttemptDebug[];
|
||||
buildTrace?: BuildTrace;
|
||||
logger: EvalLogger;
|
||||
/** --output-dir; falls back to EVAL_DATA_DIR. Concurrent eval children share
|
||||
* one checkout, so each needs its snapshots under its own directory. */
|
||||
outputDir?: string;
|
||||
}): Promise<void> {
|
||||
const snapshotDir = input.outputDir ?? EVAL_DATA_DIR;
|
||||
const timestamp = makeArtifactTimestamp();
|
||||
const fileName = `${slugifyArtifactSegment(input.testCaseName, 'workflow')}_${slugifyArtifactSegment(input.scenarioName, 'scenario')}_${timestamp}.json`;
|
||||
const filePath = path.join(EVAL_DATA_DIR, fileName);
|
||||
const filePath = path.join(snapshotDir, fileName);
|
||||
|
||||
try {
|
||||
await mkdir(EVAL_DATA_DIR, { recursive: true });
|
||||
await mkdir(snapshotDir, { recursive: true });
|
||||
await writeFile(
|
||||
filePath,
|
||||
JSON.stringify(
|
||||
@@ -90,6 +94,9 @@ export async function writeScenarioVerificationSnapshot(input: {
|
||||
|
||||
/**
|
||||
* Execute a single scenario against a pre-built workflow and verify the result.
|
||||
*
|
||||
* `outputDir` (--output-dir) is where the verification snapshot lands; omitting
|
||||
* it keeps the historical EVAL_DATA_DIR location.
|
||||
*/
|
||||
export async function executeScenario(
|
||||
client: N8nClient,
|
||||
@@ -102,6 +109,7 @@ export async function executeScenario(
|
||||
buildTrace?: BuildTrace,
|
||||
pinAiRoots?: string[],
|
||||
seedContext?: ScenarioSeedContext,
|
||||
outputDir?: string,
|
||||
): Promise<ExecutionScenarioResult> {
|
||||
return await runScenario(
|
||||
client,
|
||||
@@ -114,6 +122,7 @@ export async function executeScenario(
|
||||
buildTrace,
|
||||
pinAiRoots,
|
||||
seedContext,
|
||||
outputDir,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -248,6 +257,7 @@ async function runScenario(
|
||||
buildTrace?: BuildTrace,
|
||||
pinAiRoots?: string[],
|
||||
seedContext?: ScenarioSeedContext,
|
||||
outputDir?: string,
|
||||
): Promise<ExecutionScenarioResult> {
|
||||
const pinNodes = pinAiRoots && pinAiRoots.length > 0 ? pinAiRoots : undefined;
|
||||
const targetWorkflowId = selectScenarioWorkflowId(scenario, workflowId, workflowJsons, logger);
|
||||
@@ -336,6 +346,7 @@ async function runScenario(
|
||||
verifierAttempts: verification.attempts,
|
||||
buildTrace,
|
||||
logger,
|
||||
outputDir,
|
||||
});
|
||||
// Empty verification = the verifier itself failed after all attempts. The run
|
||||
// is excluded from scoring (mirrors incomplete build expectations) but stays
|
||||
|
||||
@@ -415,8 +415,12 @@ export function generateRunDebugReport(results: WorkflowTestCaseResult[]): strin
|
||||
</html>`;
|
||||
}
|
||||
|
||||
export function writeRunDebugReport(results: WorkflowTestCaseResult[]): string {
|
||||
const reportDir = path.join(__dirname, '..', '..', '.data');
|
||||
/**
|
||||
* Write the LLM debug report into `outputDir` (--output-dir), falling back to
|
||||
* the package-level `.data` directory — same contract as writeWorkflowReport.
|
||||
*/
|
||||
export function writeRunDebugReport(results: WorkflowTestCaseResult[], outputDir?: string): string {
|
||||
const reportDir = outputDir ?? path.join(__dirname, '..', '..', '.data');
|
||||
if (!fs.existsSync(reportDir)) {
|
||||
fs.mkdirSync(reportDir, { recursive: true });
|
||||
}
|
||||
|
||||
@@ -1806,8 +1806,14 @@ ${results.map((r, i) => renderTestCase(r, i)).join('')}
|
||||
// Write report to disk
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export function writeWorkflowReport(results: WorkflowTestCaseResult[]): string {
|
||||
const reportDir = path.join(__dirname, '..', '..', '.data');
|
||||
/**
|
||||
* Write the HTML report into `outputDir` (--output-dir), falling back to the
|
||||
* package-level `.data` directory. The stable filename makes the fallback
|
||||
* unsafe for concurrent runs against one checkout, so callers that have a
|
||||
* per-run directory must pass it (see run/reporters.ts).
|
||||
*/
|
||||
export function writeWorkflowReport(results: WorkflowTestCaseResult[], outputDir?: string): string {
|
||||
const reportDir = outputDir ?? path.join(__dirname, '..', '..', '.data');
|
||||
if (!fs.existsSync(reportDir)) {
|
||||
fs.mkdirSync(reportDir, { recursive: true });
|
||||
}
|
||||
|
||||
@@ -175,6 +175,7 @@ export function createEvalSession(config: EvalSessionConfig): EvalSession {
|
||||
execArgs.buildTrace,
|
||||
args.pinAiRoots,
|
||||
execArgs.seedContext,
|
||||
args.outputDir,
|
||||
),
|
||||
),
|
||||
tracedExecuteAgent: wrap(
|
||||
@@ -197,6 +198,7 @@ export function createEvalSession(config: EvalSessionConfig): EvalSession {
|
||||
execArgs.timeoutMs,
|
||||
execArgs.testCaseName,
|
||||
execArgs.buildTrace,
|
||||
args.outputDir,
|
||||
),
|
||||
),
|
||||
};
|
||||
|
||||
@@ -84,18 +84,30 @@ export function emitRunReports(config: {
|
||||
gate: GateResult | undefined;
|
||||
slugByTestCase: Map<WorkflowTestCase, string> | undefined;
|
||||
commitSha: string | undefined;
|
||||
/** --output-dir; the HTML reports land here alongside the data artifacts.
|
||||
* Undefined leaves each writer on its `.data` default. */
|
||||
outputDir: string | undefined;
|
||||
jsonPath: string;
|
||||
prCommentPath: string;
|
||||
/** --experiment-name; baseline-prefixed names trigger the noise advisory. */
|
||||
experimentName?: string;
|
||||
}): void {
|
||||
const { evaluation, outcome, gate, slugByTestCase, commitSha, jsonPath, prCommentPath } = config;
|
||||
const {
|
||||
evaluation,
|
||||
outcome,
|
||||
gate,
|
||||
slugByTestCase,
|
||||
commitSha,
|
||||
outputDir,
|
||||
jsonPath,
|
||||
prCommentPath,
|
||||
} = config;
|
||||
console.log(`Results: ${jsonPath}`);
|
||||
console.log(`PR comment: ${prCommentPath}`);
|
||||
const reportResults = flattenRunsForReport(evaluation);
|
||||
const htmlPath = writeWorkflowReport(reportResults);
|
||||
const htmlPath = writeWorkflowReport(reportResults, outputDir);
|
||||
console.log(`Report: ${htmlPath}`);
|
||||
const debugHtmlPath = writeRunDebugReport(reportResults);
|
||||
const debugHtmlPath = writeRunDebugReport(reportResults, outputDir);
|
||||
console.log(`LLM debug: ${debugHtmlPath}`);
|
||||
console.log(
|
||||
'\n' + formatComparisonTerminal(evaluation, outcome, { commitSha, slugByTestCase, gate }),
|
||||
|
||||
Reference in New Issue
Block a user