refactor(core): Tidy agent-eval runner and rating-persistence internals (no-changelog) (#35424)

packages/@n8n/db/src/repositories/__tests__/agent-eval-rating.repository.test.ts

@@ -1,4 +1,5 @@
 import { Container } from '@n8n/di';
+import { In } from '@n8n/typeorm';
 import type { Mock } from 'vitest';
 
 import { AgentEvalRating } from '../../entities/agent-eval-rating.ee';
@@ -101,7 +102,7 @@ describe('AgentEvalRatingRepository', () => {
 			expect(qb.where).toHaveBeenCalledWith('result.runId = :runId', { runId: 'run-1' });
 			// The superseded r-1 is never fetched, only the two winners.
 			expect(entityManager.find.mock.calls[0]?.[1]).toEqual({
-				where: { id: expect.objectContaining({ _value: ['r-2', 'r-3'] }) },
+				where: { id: In(['r-2', 'r-3']) },
 			});
 			expect(latest.map((rating) => rating.id)).toEqual(['r-2', 'r-3']);
 		});

packages/@n8n/db/src/repositories/agent-eval-rating.repository.ee.ts

@@ -41,6 +41,10 @@ export class AgentEvalRatingRepository extends Repository<AgentEvalRating> {
 	/**
 	 * Newest rating per result in a run. Reduced here, not in SQL: `DISTINCT ON` is
 	 * Postgres-only and `MAX(createdAt)` returns both rows on a millisecond tie.
+	 *
+	 * The trailing `rating.id` tiebreak buys reproducibility, not recency: ids are
+	 * random nanoids, so on a same-millisecond tie the winner is arbitrary but at
+	 * least stable across calls. Picking the later one needs a monotonic column.
 	 */
 	async findLatestByRunId(runId: string): Promise<AgentEvalRating[]> {
 		// Ids only — whole rows would load every superseded `correction` just to drop it.

packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts

@@ -384,6 +384,29 @@ describe('AgentEvalRunnerService', () => {
 				'empty_input',
 				expect.anything(),
 			);
+			// Screened out before the queue: an unusable case must never hold a slot.
+			expect(concurrencyControl.throttle).not.toHaveBeenCalled();
+			expect(resultRepository.markAsRunning).not.toHaveBeenCalled();
+		});
+
+		it('still completes the run when recording an empty-input case fails', async () => {
+			seedFor(
+				[
+					{ id: 'row-1', question: '   ' },
+					{ id: 'row-2', question: 'Q2' },
+				],
+				{ success: 1, error: 1 },
+			);
+			// This write sits outside `runCase`'s catch, so left unguarded the rejection
+			// would settle as a dispatch failure and error an otherwise-successful run.
+			resultRepository.markAsError.mockRejectedValueOnce(new Error('db unavailable'));
+			evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never);
+
+			const { finished } = await service.startRun('ds-1', 'proj-1', user);
+			await finished;
+
+			expect(runRepository.markAsCompleted).toHaveBeenCalled();
+			expect(runRepository.markAsError).not.toHaveBeenCalled();
 		});
 
 		it('pages through every row when the table exceeds one page', async () => {
@@ -682,8 +705,12 @@ describe('AgentEvalRunnerService', () => {
 			await finished;
 
 			expect(concurrencyControl.throttle).toHaveBeenCalledTimes(2);
+			// Prefixed so a throttled id is attributable to agent evals in log streaming.
 			expect(concurrencyControl.throttle).toHaveBeenCalledWith(
-				expect.objectContaining({ mode: 'evaluation' }),
+				expect.objectContaining({
+					mode: 'evaluation',
+					executionId: expect.stringMatching(/^agent-eval:/),
+				}),
 			);
 			expect(concurrencyControl.release).toHaveBeenCalledTimes(2);
 			expect(concurrencyControl.release).toHaveBeenCalledWith({ mode: 'evaluation' });

packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts

@@ -263,6 +263,22 @@ export class AgentEvalRunnerService {
 				await this.resultRepository.markAsCancelled(resultRow.id);
 			};
 
+			// An empty input can't produce an execution, so its verdict is recorded
+			// without taking a queue slot. Self-contained like `runCase`: one unusable
+			// case must not fail the run just because recording it failed.
+			const markEmptyInput = async (resultRow: AgentEvalResult) => {
+				try {
+					await this.resultRepository.markAsError(resultRow.id, 'empty_input', {
+						message: 'Case has no value in the mapped input column.',
+					});
+				} catch (error) {
+					this.logger.error(
+						`[AgentEvalRunner] Could not record empty input for case ${resultRow.id}`,
+						{ error: error instanceof Error ? error.message : String(error) },
+					);
+				}
+			};
+
 			const deadline = this.startRunDeadline(abort);
 
 			// The stop reads/writes around `runCase` sit outside its safety net, so
@@ -278,9 +294,16 @@ export class AgentEvalRunnerService {
 								return;
 							}
 
+							// Screened before the slot, after the stop: a cancel still outranks it.
+							if (resolvedCase.input.trim().length === 0) {
+								await markEmptyInput(resultRow);
+								return;
+							}
+
 							// The helper owns release/remove when the run stops while queued;
 							// the finally owns release once a slot is held.
-							const executionId = `${runId}-case-${index}`;
+							// Prefixed so the id is attributable in `n8n.execution.throttled`.
+							const executionId = `agent-eval:${runId}-case-${index}`;
 							if (!(await this.acquireEvaluationSlot(executionId, abort.signal))) {
 								await stopCase(resultRow);
 								return;
@@ -486,7 +509,8 @@ export class AgentEvalRunnerService {
 	 * Execute one case and persist its result. Returns the case's token usage.
 	 * Fully self-contained: a thrown execution or DB error is converted into a
 	 * per-case error so one case can never abort the batch or leave its result
-	 * stuck `running`.
+	 * stuck `running`. Assumes a non-empty input — the caller screens those out
+	 * before taking a queue slot.
 	 */
 	private async runCase(
 		resultRow: AgentEvalResult,
@@ -494,13 +518,6 @@ export class AgentEvalRunnerService {
 		ctx: { agentId: string; projectId: string; user: User; timeoutMs?: number },
 	): Promise<CaseUsage | undefined> {
 		try {
-			if (resolvedCase.input.trim().length === 0) {
-				await this.resultRepository.markAsError(resultRow.id, 'empty_input', {
-					message: 'Case has no value in the mapped input column.',
-				});
-				return undefined;
-			}
-
 			await this.resultRepository.markAsRunning(resultRow.id);
 
 			const execResult = await this.evalAgentExecutionService.executeWithLlmMock(
@@ -568,7 +585,7 @@ export class AgentEvalRunnerService {
 
 		// Validate the mapping against the live columns so a renamed/deleted column
 		// fails loudly here instead of silently turning every case into an
-		// "empty input" error at execution time.
+		// "empty input" error once the run starts.
 		const columnNames = new Set(
 			(await this.dataTableService.getColumns(dataTableId, tableProjectId)).map((c) => c.name),
 		);

packages/cli/vitest.config.integration.ts

@@ -6,5 +6,9 @@ export default mergeConfig(baseConfig, {
 	test: {
 		include: ['test/integration/**/*.test.ts', 'src/**/*.integration.test.ts'],
 		testTimeout: 10_000,
+		// Loading modules and building the entity schema in `beforeAll` routinely
+		// runs past Vitest's 10s hook default, which reads as broken coverage
+		// (whole suite skipped) rather than a slow setup.
+		hookTimeout: 30_000,
 	},
 });