mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 21:49:36 +02:00
feat(core): Paginate agent-eval list endpoints and retype response DTOs (no-changelog) (#35425)
This commit is contained in:
Arvin A
GitHub
parent
67200352df
commit
916ff3946a
|
|
@ -145,5 +145,21 @@ describe('PaginationDto', () => {
|
|||
expect(result.data).toBe(customMaxItems);
|
||||
}
|
||||
});
|
||||
|
||||
test('should fall back to 10 when no default is given', () => {
|
||||
expect(createTakeValidator(MAX_ITEMS_PER_PAGE).parse(undefined)).toBe(10);
|
||||
});
|
||||
|
||||
test('should apply a custom default when take is omitted', () => {
|
||||
expect(createTakeValidator(MAX_ITEMS_PER_PAGE, false, 50).parse(undefined)).toBe(50);
|
||||
});
|
||||
|
||||
// The default is a fallback, not a floor or ceiling.
|
||||
test('should let an explicit take override a custom default', () => {
|
||||
const validator = createTakeValidator(MAX_ITEMS_PER_PAGE, false, 50);
|
||||
|
||||
expect(validator.parse('5')).toBe(5);
|
||||
expect(validator.parse('9999')).toBe(MAX_ITEMS_PER_PAGE);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -15,11 +15,16 @@ const skipValidator = z
|
|||
message: 'Param `skip` must be a non-negative integer',
|
||||
});
|
||||
|
||||
export const createTakeValidator = (maxItems: number, allowInfinity: boolean = false) =>
|
||||
/** `defaultTake` applies when a request omits `take`. */
|
||||
export const createTakeValidator = (
|
||||
maxItems: number,
|
||||
allowInfinity: boolean = false,
|
||||
defaultTake: number = 10,
|
||||
) =>
|
||||
z
|
||||
.string()
|
||||
.optional()
|
||||
.transform((val) => (val ? parseInt(val, 10) : 10))
|
||||
.transform((val) => (val ? parseInt(val, 10) : defaultTake))
|
||||
.refine((val) => !isNaN(val) && Number.isInteger(val), {
|
||||
message: 'Param `take` must be a valid integer',
|
||||
})
|
||||
|
|
|
|||
|
|
@ -585,6 +585,8 @@ export {
|
|||
UpdateAgentEvalDatasetDto,
|
||||
createAgentEvalRunSchema,
|
||||
CreateAgentEvalRunDto,
|
||||
AGENT_EVAL_RESULTS_DEFAULT_TAKE,
|
||||
AgentEvalRunDetailQueryDto,
|
||||
agentEvalCorrectionSchema,
|
||||
createAgentEvalRatingSchema,
|
||||
CreateAgentEvalRatingDto,
|
||||
|
|
@ -602,10 +604,12 @@ export type {
|
|||
UpdateAgentEvalDatasetPayload,
|
||||
CreateAgentEvalRunPayload,
|
||||
CreateAgentEvalRatingPayload,
|
||||
AgentEvalPage,
|
||||
AgentEvalDatasetRecord,
|
||||
AgentEvalRunRecord,
|
||||
AgentEvalResultRecord,
|
||||
AgentEvalRatingRecord,
|
||||
AgentEvalRunList,
|
||||
AgentEvalRunDetail,
|
||||
AgentEvalRunSummary,
|
||||
AgentEvalDraftCase,
|
||||
|
|
|
|||
|
|
@ -1,7 +1,12 @@
|
|||
import type { JsonValue } from 'n8n-workflow';
|
||||
import type { IDataObject, JsonObject, JsonValue } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import { datasetRefSchema, type DatasetRef } from '../dto/evaluations/evaluation-config.dto';
|
||||
import {
|
||||
MAX_ITEMS_PER_PAGE,
|
||||
createTakeValidator,
|
||||
paginationSchema,
|
||||
} from '../dto/pagination/pagination.dto';
|
||||
import { Z } from '../zod-class';
|
||||
|
||||
// A JSON blob that flows from a request into persistence must infer to the
|
||||
|
|
@ -107,6 +112,14 @@ export const createAgentEvalRunSchema = z.object(createAgentEvalRunShape);
|
|||
export type CreateAgentEvalRunPayload = z.infer<typeof createAgentEvalRunSchema>;
|
||||
export class CreateAgentEvalRunDto extends Z.class(createAgentEvalRunShape) {}
|
||||
|
||||
// Wider default than the shared `PaginationDto`: opening a run reads its cases,
|
||||
// of which there are many, so 10 would page immediately for no reason.
|
||||
export const AGENT_EVAL_RESULTS_DEFAULT_TAKE = 50;
|
||||
export class AgentEvalRunDetailQueryDto extends Z.class({
|
||||
...paginationSchema,
|
||||
take: createTakeValidator(MAX_ITEMS_PER_PAGE, false, AGENT_EVAL_RESULTS_DEFAULT_TAKE),
|
||||
}) {}
|
||||
|
||||
// A correction carries the edited answer under `finalText`, mirroring the key the
|
||||
// runner writes into a result's `output` so calibration can diff the two directly.
|
||||
// Required when a correction is sent: a correction without it persists a rating
|
||||
|
|
@ -134,8 +147,17 @@ export class CreateAgentEvalRatingDto extends Z.class(createAgentEvalRatingShape
|
|||
// own output through validation. Dates are serialized as ISO strings; internal
|
||||
// coordination columns (`runningInstanceId`, `cancelRequested`) are omitted
|
||||
// from the contract.
|
||||
//
|
||||
// JSON blobs mirror the type their entity column stores, so consumers needn't
|
||||
// re-narrow `unknown` on every read.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** One page of a list route. `count` is the total matching rows, not the page length. */
|
||||
export type AgentEvalPage<T> = {
|
||||
count: number;
|
||||
data: T[];
|
||||
};
|
||||
|
||||
export type AgentEvalDatasetRecord = {
|
||||
id: string;
|
||||
name: string;
|
||||
|
|
@ -154,9 +176,9 @@ export type AgentEvalRunRecord = {
|
|||
status: AgentEvalRunStatus;
|
||||
runAt: string | null;
|
||||
completedAt: string | null;
|
||||
metrics: Record<string, unknown> | null;
|
||||
metrics: IDataObject | null;
|
||||
errorCode: string | null;
|
||||
errorDetails: Record<string, unknown> | null;
|
||||
errorDetails: IDataObject | null;
|
||||
createdById: string | null;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
|
|
@ -168,14 +190,14 @@ export type AgentEvalResultRecord = {
|
|||
sourceRowId: string | null;
|
||||
runIndex: number | null;
|
||||
status: AgentEvalResultStatus;
|
||||
input: Record<string, unknown> | null;
|
||||
output: Record<string, unknown> | null;
|
||||
toolCalls: Record<string, unknown> | null;
|
||||
metrics: Record<string, unknown> | null;
|
||||
input: JsonObject | null;
|
||||
output: JsonObject | null;
|
||||
toolCalls: JsonObject | null;
|
||||
metrics: IDataObject | null;
|
||||
runAt: string | null;
|
||||
completedAt: string | null;
|
||||
errorCode: string | null;
|
||||
errorDetails: Record<string, unknown> | null;
|
||||
errorDetails: IDataObject | null;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
};
|
||||
|
|
@ -185,15 +207,20 @@ export type AgentEvalRatingRecord = {
|
|||
resultId: string;
|
||||
vote: AgentEvalVote;
|
||||
comment: string | null;
|
||||
correction: Record<string, unknown> | null;
|
||||
correction: JsonObject | null;
|
||||
ratedById: string | null;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
};
|
||||
|
||||
// A run with its per-case results — the "open a run" view.
|
||||
// Runs of a dataset, newest first. Paginated because nothing caps how many a
|
||||
// dataset accumulates — every "Run all" adds one.
|
||||
export type AgentEvalRunList = AgentEvalPage<AgentEvalRunRecord>;
|
||||
|
||||
// A run with a page of its per-case results — the "open a run" view. Paginated
|
||||
// because each row carries its full input/output JSON.
|
||||
export type AgentEvalRunDetail = AgentEvalRunRecord & {
|
||||
results: AgentEvalResultRecord[];
|
||||
results: AgentEvalPage<AgentEvalResultRecord>;
|
||||
};
|
||||
|
||||
// The progress-polling shape: status plus tallies, no per-case rows, so polling
|
||||
|
|
|
|||
|
|
@ -115,14 +115,39 @@ describe('AgentEvalResultRepository', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('findByRunId', () => {
|
||||
describe('findAndCountByRunId', () => {
|
||||
it('scopes to runId ordered by runIndex ascending', async () => {
|
||||
entityManager.find.mockResolvedValueOnce([]);
|
||||
entityManager.findAndCount.mockResolvedValueOnce([[], 0]);
|
||||
|
||||
await repo.findByRunId('run-1');
|
||||
await repo.findAndCountByRunId('run-1');
|
||||
|
||||
const callArgs = entityManager.find.mock.calls[0];
|
||||
expect(callArgs?.[1]).toEqual({ where: { runId: 'run-1' }, order: { runIndex: 'ASC' } });
|
||||
expect(entityManager.findAndCount.mock.calls[0]?.[1]).toMatchObject({
|
||||
where: { runId: 'run-1' },
|
||||
order: { runIndex: 'ASC' },
|
||||
});
|
||||
});
|
||||
|
||||
// Must reach the query: slicing in memory would load the full
|
||||
// input/output/toolCalls JSON that pagination exists to avoid.
|
||||
it('pushes the page window into the query rather than slicing in memory', async () => {
|
||||
entityManager.findAndCount.mockResolvedValueOnce([[], 0]);
|
||||
|
||||
await repo.findAndCountByRunId('run-1', { take: 25, skip: 50 });
|
||||
|
||||
expect(entityManager.findAndCount.mock.calls[0]?.[1]).toMatchObject({
|
||||
take: 25,
|
||||
skip: 50,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns the unpaginated total alongside the page', async () => {
|
||||
const page = [{ id: 'res-1' }] as AgentEvalResult[];
|
||||
entityManager.findAndCount.mockResolvedValueOnce([page, 500]);
|
||||
|
||||
const [results, count] = await repo.findAndCountByRunId('run-1', { take: 1, skip: 0 });
|
||||
|
||||
expect(results).toEqual(page);
|
||||
expect(count).toBe(500);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -185,17 +185,53 @@ describe('AgentEvalRunRepository', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('findByDatasetIdAndAgentId', () => {
|
||||
describe('findAndCountByDatasetIdAndAgentId', () => {
|
||||
it('constrains the dataset’s runs by that agent, newest first', async () => {
|
||||
entityManager.find.mockResolvedValueOnce([]);
|
||||
entityManager.findAndCount.mockResolvedValueOnce([[], 0]);
|
||||
|
||||
await repo.findByDatasetIdAndAgentId('ds-1', 'agent-1');
|
||||
await repo.findAndCountByDatasetIdAndAgentId('ds-1', 'agent-1');
|
||||
|
||||
expect(entityManager.find.mock.calls[0]?.[1]).toEqual({
|
||||
expect(entityManager.findAndCount.mock.calls[0]?.[1]).toMatchObject({
|
||||
where: { datasetId: 'ds-1', dataset: { agentId: 'agent-1' } },
|
||||
order: { createdAt: 'DESC' },
|
||||
});
|
||||
});
|
||||
|
||||
// `createdAt` alone is not a total order, so without the tiebreak a run
|
||||
// sharing a timestamp could show up on two pages, or on none.
|
||||
it('breaks createdAt ties deterministically so paging cannot skip a run', async () => {
|
||||
entityManager.findAndCount.mockResolvedValueOnce([[], 0]);
|
||||
|
||||
await repo.findAndCountByDatasetIdAndAgentId('ds-1', 'agent-1');
|
||||
|
||||
expect(entityManager.findAndCount.mock.calls[0]?.[1]).toMatchObject({
|
||||
order: { createdAt: 'DESC', id: 'DESC' },
|
||||
});
|
||||
});
|
||||
|
||||
// The point of the method: one page from the database, not every run.
|
||||
it('pushes the page window into the query', async () => {
|
||||
entityManager.findAndCount.mockResolvedValueOnce([[], 0]);
|
||||
|
||||
await repo.findAndCountByDatasetIdAndAgentId('ds-1', 'agent-1', { take: 20, skip: 40 });
|
||||
|
||||
expect(entityManager.findAndCount.mock.calls[0]?.[1]).toMatchObject({
|
||||
take: 20,
|
||||
skip: 40,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns the unpaginated total alongside the page', async () => {
|
||||
const page = [{ id: 'run-1' }, { id: 'run-2' }] as AgentEvalRun[];
|
||||
entityManager.findAndCount.mockResolvedValueOnce([page, 137]);
|
||||
|
||||
const [runs, count] = await repo.findAndCountByDatasetIdAndAgentId('ds-1', 'agent-1', {
|
||||
take: 2,
|
||||
skip: 0,
|
||||
});
|
||||
|
||||
expect(runs).toEqual(page);
|
||||
expect(count).toBe(137);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isCancellationRequested', () => {
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ export class AgentEvalResultRepository extends Repository<AgentEvalResult> {
|
|||
status: 'new',
|
||||
runId: c.runId,
|
||||
sourceRowId: c.sourceRowId ?? null,
|
||||
// Fall back to the seed position so `findByRunId` (orders by
|
||||
// Fall back to the seed position so `findAndCountByRunId` (orders by
|
||||
// runIndex ASC) returns a stable order on every database. A null
|
||||
// runIndex would sort first on SQLite but last on Postgres.
|
||||
runIndex: c.runIndex ?? index,
|
||||
|
|
@ -103,8 +103,21 @@ export class AgentEvalResultRepository extends Repository<AgentEvalResult> {
|
|||
});
|
||||
}
|
||||
|
||||
async findByRunId(runId: string): Promise<AgentEvalResult[]> {
|
||||
return await this.find({ where: { runId }, order: { runIndex: 'ASC' } });
|
||||
/**
|
||||
* Paged in SQL, not by slicing a full read: every row carries its `input`,
|
||||
* `output` and `toolCalls` JSON. `runIndex` is distinct per run, so it is
|
||||
* already a total order and needs no tiebreak.
|
||||
*/
|
||||
async findAndCountByRunId(
|
||||
runId: string,
|
||||
options: { skip?: number; take?: number } = {},
|
||||
): Promise<[AgentEvalResult[], number]> {
|
||||
return await this.findAndCount({
|
||||
where: { runId },
|
||||
order: { runIndex: 'ASC' },
|
||||
skip: options.skip,
|
||||
take: options.take,
|
||||
});
|
||||
}
|
||||
|
||||
async findById(id: string): Promise<AgentEvalResult | null> {
|
||||
|
|
|
|||
|
|
@ -96,11 +96,21 @@ export class AgentEvalRunRepository extends Repository<AgentEvalRun> {
|
|||
return await this.findOne({ where: { id, dataset: { agentId } } });
|
||||
}
|
||||
|
||||
/** Runs of a dataset, scoped to the agent that dataset must belong to. */
|
||||
async findByDatasetIdAndAgentId(datasetId: string, agentId: string): Promise<AgentEvalRun[]> {
|
||||
return await this.find({
|
||||
/**
|
||||
* One page of a dataset's runs, newest first, scoped to its agent. The `id`
|
||||
* tiebreak is what keeps equal-`createdAt` rows from landing on two pages or
|
||||
* none — ids are random nanoids, so it orders nothing, it just stays stable.
|
||||
*/
|
||||
async findAndCountByDatasetIdAndAgentId(
|
||||
datasetId: string,
|
||||
agentId: string,
|
||||
options: { skip?: number; take?: number } = {},
|
||||
): Promise<[AgentEvalRun[], number]> {
|
||||
return await this.findAndCount({
|
||||
where: { datasetId, dataset: { agentId } },
|
||||
order: { createdAt: 'DESC' },
|
||||
order: { createdAt: 'DESC', id: 'DESC' },
|
||||
skip: options.skip,
|
||||
take: options.take,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -158,7 +158,7 @@ describe('AgentEvalRunnerService (integration)', () => {
|
|||
const inputsSent = evalAgentExecutionService.executeWithLlmMock.mock.calls.map((c) => c[3]);
|
||||
expect(inputsSent).toEqual(expect.arrayContaining(['What is 2+2?', 'Capital of France?']));
|
||||
|
||||
const results = await Container.get(AgentEvalResultRepository).findByRunId(runId);
|
||||
const [results] = await Container.get(AgentEvalResultRepository).findAndCountByRunId(runId);
|
||||
expect(results).toHaveLength(2);
|
||||
for (const result of results) {
|
||||
expect(result.status).toBe('success');
|
||||
|
|
@ -217,7 +217,7 @@ describe('AgentEvalRunnerService (integration)', () => {
|
|||
expect(summary.status).toBe('completed');
|
||||
expect(summary.counts).toMatchObject({ total: 1, success: 0, error: 1 });
|
||||
|
||||
const [result] = await Container.get(AgentEvalResultRepository).findByRunId(runId);
|
||||
const [[result]] = await Container.get(AgentEvalResultRepository).findAndCountByRunId(runId);
|
||||
expect(result.status).toBe('error');
|
||||
expect(result.errorCode).toBe('execution_failed');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -102,17 +102,78 @@ describe('agent-eval scoped repository reads (integration)', () => {
|
|||
await expect(runRepository.findByIdAndAgentId(run.id, foreign.id)).resolves.toBeNull();
|
||||
});
|
||||
|
||||
// TypeORM pages a relation-filtered query with a distinct sub-query, so only
|
||||
// a real database proves it. Every run shares one `createdAt`, which is the
|
||||
// case the `id` tiebreak exists for.
|
||||
it('walks a dataset’s runs page by page without repeating or dropping one', async () => {
|
||||
const { own, dataset } = await seedTwoAgents();
|
||||
// One run exists from the seed; five more leave a partial second page.
|
||||
for (let i = 0; i < 5; i++) await runRepository.createRun({ datasetId: dataset.id });
|
||||
// `@CreateDateColumn` only auto-fills on insert, so pin them explicitly.
|
||||
const sharedCreatedAt = new Date('2026-01-01T00:00:00.000Z');
|
||||
await runRepository.update({ datasetId: dataset.id }, { createdAt: sharedCreatedAt });
|
||||
|
||||
const [firstPage, total] = await runRepository.findAndCountByDatasetIdAndAgentId(
|
||||
dataset.id,
|
||||
own.id,
|
||||
{ take: 4, skip: 0 },
|
||||
);
|
||||
const [secondPage] = await runRepository.findAndCountByDatasetIdAndAgentId(
|
||||
dataset.id,
|
||||
own.id,
|
||||
{ take: 4, skip: 4 },
|
||||
);
|
||||
// The same rows read in one go, as the yardstick for the paged walk.
|
||||
const [whole] = await runRepository.findAndCountByDatasetIdAndAgentId(dataset.id, own.id);
|
||||
|
||||
// The count is the whole set, not the page.
|
||||
expect(total).toBe(6);
|
||||
expect(firstPage).toHaveLength(4);
|
||||
expect(secondPage).toHaveLength(2);
|
||||
|
||||
const walked = [...firstPage, ...secondPage].map((r) => r.id);
|
||||
// No run repeated across pages, and none lost between them.
|
||||
expect(new Set(walked).size).toBe(6);
|
||||
// Paging agrees with reading everything at once — compared against the
|
||||
// database's own ordering, since its collation is not JS's sort order.
|
||||
expect(walked).toEqual(whole.map((r) => r.id));
|
||||
});
|
||||
|
||||
it('pages a run’s cases in seed order and counts them all', async () => {
|
||||
const { dataset } = await seedTwoAgents();
|
||||
const run = await runRepository.createRun({ datasetId: dataset.id });
|
||||
await resultRepository.seedResults(
|
||||
Array.from({ length: 5 }, (_, i) => ({
|
||||
runId: run.id,
|
||||
sourceRowId: `row-${i}`,
|
||||
runIndex: i,
|
||||
input: { input: `q${i}` },
|
||||
})),
|
||||
);
|
||||
|
||||
const [page, total] = await resultRepository.findAndCountByRunId(run.id, {
|
||||
take: 2,
|
||||
skip: 2,
|
||||
});
|
||||
|
||||
// `seedTwoAgents` seeded a case under another run, so 5 rather than 6
|
||||
// is what shows the count is scoped to this one.
|
||||
expect(total).toBe(5);
|
||||
expect(page.map((r) => r.sourceRowId)).toEqual(['row-2', 'row-3']);
|
||||
});
|
||||
|
||||
it('lists a dataset’s runs only for that dataset’s agent', async () => {
|
||||
const { own, foreign, dataset, run } = await seedTwoAgents();
|
||||
|
||||
await expect(
|
||||
runRepository.findByDatasetIdAndAgentId(dataset.id, own.id),
|
||||
).resolves.toMatchObject([{ id: run.id }]);
|
||||
runRepository.findAndCountByDatasetIdAndAgentId(dataset.id, own.id),
|
||||
).resolves.toMatchObject([[{ id: run.id }], 1]);
|
||||
// Right dataset id, wrong agent: the pairing has to be checked, or a foreign
|
||||
// caller reads another agent's run history.
|
||||
// caller reads another agent's run history — and a bare count would leak
|
||||
// how many runs that agent has.
|
||||
await expect(
|
||||
runRepository.findByDatasetIdAndAgentId(dataset.id, foreign.id),
|
||||
).resolves.toEqual([]);
|
||||
runRepository.findAndCountByDatasetIdAndAgentId(dataset.id, foreign.id),
|
||||
).resolves.toEqual([[], 0]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -154,7 +215,7 @@ describe('agent-eval scoped repository reads (integration)', () => {
|
|||
|
||||
await expect(datasetRepository.findById(dataset.id)).resolves.toBeNull();
|
||||
await expect(runRepository.findById(run.id)).resolves.toBeNull();
|
||||
await expect(resultRepository.findByRunId(run.id)).resolves.toEqual([]);
|
||||
await expect(resultRepository.findAndCountByRunId(run.id)).resolves.toEqual([[], 0]);
|
||||
expect(result).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -33,6 +33,8 @@ vi.mock('../agent-eval-case-generation.service', () => ({
|
|||
|
||||
const AGENT_ID = 'agent-1';
|
||||
const PROJECT_ID = 'proj-1';
|
||||
// What `PaginationDto` hands the service once the route has defaulted it.
|
||||
const PAGE = { take: 10, skip: 0 };
|
||||
|
||||
describe('AgentEvalService', () => {
|
||||
const user = mock<User>({ id: 'user-1' });
|
||||
|
|
@ -93,6 +95,8 @@ describe('AgentEvalService', () => {
|
|||
agentRepository.findByIdAndProjectId.mockResolvedValue(mock<Agent>({ id: AGENT_ID }));
|
||||
datasetRepository.findByIdAndAgentId.mockResolvedValue(makeDataset());
|
||||
runRepository.findByIdAndAgentId.mockResolvedValue(makeRun());
|
||||
runRepository.findAndCountByDatasetIdAndAgentId.mockResolvedValue([[], 0]);
|
||||
resultRepository.findAndCountByRunId.mockResolvedValue([[], 0]);
|
||||
|
||||
service = new AgentEvalService(
|
||||
moduleRegistry,
|
||||
|
|
@ -132,8 +136,8 @@ describe('AgentEvalService', () => {
|
|||
async () => await service.generateDraftCases(user, AGENT_ID, PROJECT_ID, {}),
|
||||
],
|
||||
['startRun', async () => await service.startRun(user, AGENT_ID, PROJECT_ID, 'ds-1', {})],
|
||||
['listRuns', async () => await service.listRuns(AGENT_ID, PROJECT_ID, 'ds-1')],
|
||||
['getRunDetail', async () => await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1')],
|
||||
['listRuns', async () => await service.listRuns(AGENT_ID, PROJECT_ID, 'ds-1', PAGE)],
|
||||
['getRunDetail', async () => await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', PAGE)],
|
||||
['getRunSummary', async () => await service.getRunSummary(AGENT_ID, PROJECT_ID, 'run-1')],
|
||||
['cancelRun', async () => await service.cancelRun(AGENT_ID, PROJECT_ID, 'run-1')],
|
||||
];
|
||||
|
|
@ -184,10 +188,10 @@ describe('AgentEvalService', () => {
|
|||
it('404s a run belonging to another agent, and reads no results for it', async () => {
|
||||
runRepository.findByIdAndAgentId.mockResolvedValue(null);
|
||||
|
||||
await expect(service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-other')).rejects.toThrow(
|
||||
await expect(service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-other', PAGE)).rejects.toThrow(
|
||||
NotFoundError,
|
||||
);
|
||||
expect(resultRepository.findByRunId).not.toHaveBeenCalled();
|
||||
expect(resultRepository.findAndCountByRunId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses to start a run on another agent’s dataset, without touching the runner', async () => {
|
||||
|
|
@ -329,9 +333,7 @@ describe('AgentEvalService', () => {
|
|||
|
||||
describe('response mapping', () => {
|
||||
it('serializes dates as ISO strings and keeps run coordination columns off the wire', async () => {
|
||||
resultRepository.findByRunId.mockResolvedValue([]);
|
||||
|
||||
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1');
|
||||
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', PAGE);
|
||||
|
||||
expect(detail.runAt).toBe('2026-01-03T00:00:00.000Z');
|
||||
expect(detail.createdAt).toBe('2026-01-03T00:00:00.000Z');
|
||||
|
|
@ -382,23 +384,26 @@ describe('AgentEvalService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('includes every per-case result in a run detail', async () => {
|
||||
resultRepository.findByRunId.mockResolvedValue([
|
||||
mock<AgentEvalResult>({
|
||||
id: 'res-1',
|
||||
runId: 'run-1',
|
||||
status: 'success',
|
||||
runAt: new Date('2026-01-03T00:00:01.000Z'),
|
||||
completedAt: new Date('2026-01-03T00:00:02.000Z'),
|
||||
createdAt: new Date('2026-01-03T00:00:00.000Z'),
|
||||
updatedAt: new Date('2026-01-03T00:00:02.000Z'),
|
||||
}),
|
||||
it('maps every per-case result on the requested page of a run detail', async () => {
|
||||
resultRepository.findAndCountByRunId.mockResolvedValue([
|
||||
[
|
||||
mock<AgentEvalResult>({
|
||||
id: 'res-1',
|
||||
runId: 'run-1',
|
||||
status: 'success',
|
||||
runAt: new Date('2026-01-03T00:00:01.000Z'),
|
||||
completedAt: new Date('2026-01-03T00:00:02.000Z'),
|
||||
createdAt: new Date('2026-01-03T00:00:00.000Z'),
|
||||
updatedAt: new Date('2026-01-03T00:00:02.000Z'),
|
||||
}),
|
||||
],
|
||||
1,
|
||||
]);
|
||||
|
||||
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1');
|
||||
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', PAGE);
|
||||
|
||||
expect(detail.results).toHaveLength(1);
|
||||
expect(detail.results[0]).toMatchObject({
|
||||
expect(detail.results.data).toHaveLength(1);
|
||||
expect(detail.results.data[0]).toMatchObject({
|
||||
id: 'res-1',
|
||||
status: 'success',
|
||||
runAt: '2026-01-03T00:00:01.000Z',
|
||||
|
|
@ -407,6 +412,48 @@ describe('AgentEvalService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// The window has to reach the repositories: slicing in memory would still
|
||||
// grow without bound as a dataset ages.
|
||||
describe('pagination', () => {
|
||||
it('passes the run-list window to the repository and reports the total', async () => {
|
||||
runRepository.findAndCountByDatasetIdAndAgentId.mockResolvedValue([[makeRun()], 342]);
|
||||
|
||||
const page = await service.listRuns(AGENT_ID, PROJECT_ID, 'ds-1', { take: 25, skip: 50 });
|
||||
|
||||
expect(runRepository.findAndCountByDatasetIdAndAgentId).toHaveBeenCalledWith(
|
||||
'ds-1',
|
||||
AGENT_ID,
|
||||
{ take: 25, skip: 50 },
|
||||
);
|
||||
expect(page).toMatchObject({ count: 342 });
|
||||
expect(page.data).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('passes the run-detail window to the repository and reports the case total', async () => {
|
||||
resultRepository.findAndCountByRunId.mockResolvedValue([[], 500]);
|
||||
|
||||
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', {
|
||||
take: 25,
|
||||
skip: 50,
|
||||
});
|
||||
|
||||
expect(resultRepository.findAndCountByRunId).toHaveBeenCalledWith('run-1', {
|
||||
take: 25,
|
||||
skip: 50,
|
||||
});
|
||||
expect(detail.results).toEqual({ count: 500, data: [] });
|
||||
});
|
||||
|
||||
// A client sizing its pager off `data.length` would misread the total.
|
||||
it('reports a total larger than the page it returned', async () => {
|
||||
runRepository.findAndCountByDatasetIdAndAgentId.mockResolvedValue([[makeRun()], 342]);
|
||||
|
||||
const page = await service.listRuns(AGENT_ID, PROJECT_ID, 'ds-1', { take: 1, skip: 0 });
|
||||
|
||||
expect(page.count).toBeGreaterThan(page.data.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('generateDraftCases', () => {
|
||||
it('delegates with the project resolved from the URL', async () => {
|
||||
caseGenerationService.generateDraftCases.mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -1,3 +1,9 @@
|
|||
import {
|
||||
AGENT_EVAL_RESULTS_DEFAULT_TAKE,
|
||||
AgentEvalRunDetailQueryDto,
|
||||
MAX_ITEMS_PER_PAGE,
|
||||
PaginationDto,
|
||||
} from '@n8n/api-types';
|
||||
import type { AuthenticatedRequest, User } from '@n8n/db';
|
||||
import { ControllerRegistryMetadata } from '@n8n/decorators';
|
||||
import { Container } from '@n8n/di';
|
||||
|
|
@ -19,6 +25,10 @@ vi.mock('../agent-evals-flag-gate', () => ({ AgentEvalsFlagGate: class AgentEval
|
|||
|
||||
const PROJECT_ID = 'proj-1';
|
||||
const AGENT_ID = 'agent-1';
|
||||
// What each route binds when the client sends no window. The two DTOs differ:
|
||||
// run detail pages cases, so it defaults wider.
|
||||
const PAGE = PaginationDto.parse({});
|
||||
const RUN_PAGE = AgentEvalRunDetailQueryDto.parse({});
|
||||
|
||||
describe('AgentEvalsController', () => {
|
||||
const user = mock<User>({ id: 'user-1' });
|
||||
|
|
@ -72,10 +82,9 @@ describe('AgentEvalsController', () => {
|
|||
expect(route.accessScope?.scope.startsWith('agent:')).toBe(true);
|
||||
});
|
||||
|
||||
// Reads stay on agent:read; anything that writes eval config — including
|
||||
// generation, which spends the builder's model credits, and rating, whose
|
||||
// corrections seed judge calibration — needs agent:update;
|
||||
// starting/cancelling a run is agent:execute.
|
||||
// Reads on agent:read; eval-config writes — generation, which spends the
|
||||
// builder's model credits, rating, and cancellation, which acts on a run
|
||||
// someone else started — on agent:update. Only starting is agent:execute.
|
||||
const expectedScopes = {
|
||||
listDatasets: 'agent:read',
|
||||
getDataset: 'agent:read',
|
||||
|
|
@ -89,8 +98,8 @@ describe('AgentEvalsController', () => {
|
|||
deleteDataset: 'agent:update',
|
||||
generateDraftCases: 'agent:update',
|
||||
rateResult: 'agent:update',
|
||||
cancelRun: 'agent:update',
|
||||
startRun: 'agent:execute',
|
||||
cancelRun: 'agent:execute',
|
||||
} as const;
|
||||
|
||||
it.each(Object.entries(expectedScopes))('%s uses %s', (handlerName, scope) => {
|
||||
|
|
@ -104,6 +113,13 @@ describe('AgentEvalsController', () => {
|
|||
Object.keys(expectedScopes).sort(),
|
||||
);
|
||||
});
|
||||
|
||||
// `PROJECT_CHAT_USER_SCOPES` is only `agent:execute` + `workflow:execute-chat`,
|
||||
// so cancel on `agent:execute` would let a chat-only user stop any run.
|
||||
it('keeps cancel off the scope a chat-only user holds', () => {
|
||||
expect(metadata.routes.get('cancelRun')?.accessScope?.scope).not.toBe('agent:execute');
|
||||
expect(metadata.routes.get('startRun')?.accessScope?.scope).toBe('agent:execute');
|
||||
});
|
||||
});
|
||||
|
||||
// Flag-off must look like an unknown endpoint on every route, and must not
|
||||
|
|
@ -137,8 +153,8 @@ describe('AgentEvalsController', () => {
|
|||
async () => await controller.generateDraftCases(agentReq(), undefined, {}),
|
||||
],
|
||||
['startRun', async () => await controller.startRun(datasetReq(), undefined, {})],
|
||||
['listRuns', async () => await controller.listRuns(datasetReq())],
|
||||
['getRun', async () => await controller.getRun(runReq())],
|
||||
['listRuns', async () => await controller.listRuns(datasetReq(), undefined, PAGE)],
|
||||
['getRun', async () => await controller.getRun(runReq(), undefined, RUN_PAGE)],
|
||||
['getRunSummary', async () => await controller.getRunSummary(runReq())],
|
||||
['cancelRun', async () => await controller.cancelRun(runReq())],
|
||||
[
|
||||
|
|
@ -276,4 +292,61 @@ describe('AgentEvalsController', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
// A controller that dropped `query` would silently serve the default page.
|
||||
describe('pagination', () => {
|
||||
it('forwards the run-list window to the service', async () => {
|
||||
const query = PaginationDto.parse({ take: '25', skip: '50' });
|
||||
|
||||
await controller.listRuns(datasetReq(), undefined, query);
|
||||
|
||||
expect(service.listRuns).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID, 'ds-1', {
|
||||
take: 25,
|
||||
skip: 50,
|
||||
});
|
||||
});
|
||||
|
||||
it('forwards the run-detail window to the service', async () => {
|
||||
const query = AgentEvalRunDetailQueryDto.parse({ take: '25', skip: '50' });
|
||||
|
||||
await controller.getRun(runReq(), undefined, query);
|
||||
|
||||
expect(service.getRunDetail).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID, 'run-1', {
|
||||
take: 25,
|
||||
skip: 50,
|
||||
});
|
||||
});
|
||||
|
||||
// The DTO always supplies a window, so neither route defaults to unbounded.
|
||||
it('bounds both list reads even when the client sends no window', async () => {
|
||||
await controller.listRuns(datasetReq(), undefined, PaginationDto.parse({}));
|
||||
await controller.getRun(runReq(), undefined, AgentEvalRunDetailQueryDto.parse({}));
|
||||
|
||||
for (const call of [service.listRuns, service.getRunDetail]) {
|
||||
const page = call.mock.calls[0]?.at(-1) as { take: number; skip: number };
|
||||
expect(page.take).toBeGreaterThan(0);
|
||||
expect(page.skip).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
// Opening a run reads its cases, so this route defaults wider.
|
||||
it('defaults the run-detail window wider than the shared list default', async () => {
|
||||
await controller.getRun(runReq(), undefined, AgentEvalRunDetailQueryDto.parse({}));
|
||||
|
||||
expect(service.getRunDetail).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID, 'run-1', {
|
||||
take: AGENT_EVAL_RESULTS_DEFAULT_TAKE,
|
||||
skip: 0,
|
||||
});
|
||||
expect(AGENT_EVAL_RESULTS_DEFAULT_TAKE).toBeGreaterThan(PaginationDto.parse({}).take);
|
||||
});
|
||||
|
||||
// A bigger ask is still held to the shared cap.
|
||||
it('clamps an oversized run-detail window to the shared maximum', async () => {
|
||||
const query = AgentEvalRunDetailQueryDto.parse({ take: '9999' });
|
||||
|
||||
await controller.getRun(runReq(), undefined, query);
|
||||
|
||||
expect(query.take).toBe(MAX_ITEMS_PER_PAGE);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import type {
|
||||
AgentEvalDatasetRecord,
|
||||
AgentEvalRunDetail,
|
||||
AgentEvalRunList,
|
||||
AgentEvalRunRecord,
|
||||
AgentEvalRunSummary,
|
||||
CreateAgentEvalDatasetDto,
|
||||
|
|
@ -30,6 +31,12 @@ import { assertRequiredModulesActive } from './agent-evals-required-modules';
|
|||
/** Statuses a run can still be asked to stop from. */
|
||||
const CANCELLABLE_STATUSES = new Set(['new', 'running']);
|
||||
|
||||
/**
|
||||
* Required, not optional, so a caller can't forget to pass a window. Note a
|
||||
* `take` of 0 is n8n's "no limit" idiom, so this bounds the default path only.
|
||||
*/
|
||||
type PageParams = { take: number; skip: number };
|
||||
|
||||
/**
|
||||
* Dataset CRUD, run reads and cancellation behind the agent-eval REST routes.
|
||||
*
|
||||
|
|
@ -158,23 +165,29 @@ export class AgentEvalService {
|
|||
agentId: string,
|
||||
projectId: string,
|
||||
datasetId: string,
|
||||
): Promise<AgentEvalRunRecord[]> {
|
||||
page: PageParams,
|
||||
): Promise<AgentEvalRunList> {
|
||||
await this.assertAgentInProject(agentId, projectId);
|
||||
await this.resolveDataset(agentId, datasetId);
|
||||
const runs = await this.runRepository.findByDatasetIdAndAgentId(datasetId, agentId);
|
||||
return runs.map(toRunRecord);
|
||||
const [runs, count] = await this.runRepository.findAndCountByDatasetIdAndAgentId(
|
||||
datasetId,
|
||||
agentId,
|
||||
page,
|
||||
);
|
||||
return { count, data: runs.map(toRunRecord) };
|
||||
}
|
||||
|
||||
/** A run with every per-case result — the "open a run" view. */
|
||||
/** A run with a page of its per-case results — the "open a run" view. */
|
||||
async getRunDetail(
|
||||
agentId: string,
|
||||
projectId: string,
|
||||
runId: string,
|
||||
page: PageParams,
|
||||
): Promise<AgentEvalRunDetail> {
|
||||
await this.assertAgentInProject(agentId, projectId);
|
||||
const run = await this.resolveRun(agentId, runId);
|
||||
const results = await this.resultRepository.findByRunId(runId);
|
||||
return { ...toRunRecord(run), results: results.map(toResultRecord) };
|
||||
const [results, count] = await this.resultRepository.findAndCountByRunId(runId, page);
|
||||
return { ...toRunRecord(run), results: { count, data: results.map(toResultRecord) } };
|
||||
}
|
||||
|
||||
// Per-case status counts for progress polling, ownership-resolved first so this
|
||||
|
|
|
|||
|
|
@ -1,18 +1,30 @@
|
|||
import {
|
||||
AgentEvalRunDetailQueryDto,
|
||||
CreateAgentEvalRatingDto,
|
||||
CreateAgentEvalRunDto,
|
||||
GenerateDraftCasesOptionsDto,
|
||||
PaginationDto,
|
||||
UpdateAgentEvalDatasetDto,
|
||||
createAgentEvalDatasetSchema,
|
||||
type AgentEvalDatasetRecord,
|
||||
type AgentEvalRatingRecord,
|
||||
type AgentEvalRunDetail,
|
||||
type AgentEvalRunList,
|
||||
type AgentEvalRunRecord,
|
||||
type AgentEvalRunSummary,
|
||||
type GenerateDraftCasesResult,
|
||||
} from '@n8n/api-types';
|
||||
import type { AuthenticatedRequest } from '@n8n/db';
|
||||
import { Body, Delete, Get, Patch, Post, ProjectScope, RestController } from '@n8n/decorators';
|
||||
import {
|
||||
Body,
|
||||
Delete,
|
||||
Get,
|
||||
Patch,
|
||||
Post,
|
||||
ProjectScope,
|
||||
Query,
|
||||
RestController,
|
||||
} from '@n8n/decorators';
|
||||
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
|
||||
|
|
@ -34,9 +46,12 @@ type ResultParam = AgentParam & { resultId: string };
|
|||
* `101_agent_evals` rollout — the services behind it check neither, so a route
|
||||
* added here without a scope decorator and a `flagGate` call is an open one.
|
||||
*
|
||||
* `agent:read` for reads, `agent:execute` for running a run, `agent:update` for
|
||||
* `agent:read` for reads, `agent:execute` for starting a run, `agent:update` for
|
||||
* eval-config writes — including generation, which spends the builder's model
|
||||
* credits and so must stay closed to viewers (they hold `agent:execute`).
|
||||
* credits, and cancellation, which acts on a run someone else started. Both must
|
||||
* stay closed to viewers, who hold `agent:execute`.
|
||||
*
|
||||
* Paged reads answer with `{ count, data }`, where `count` is the total.
|
||||
*/
|
||||
@RestController('/projects/:projectId/agents/v2')
|
||||
export class AgentEvalsController {
|
||||
|
|
@ -130,20 +145,30 @@ export class AgentEvalsController {
|
|||
return await this.service.startRun(req.user, agentId, projectId, datasetId, payload);
|
||||
}
|
||||
|
||||
/** Newest first. Unbounded over a dataset's life, so the window is required. */
|
||||
@Get('/:agentId/evals/datasets/:datasetId/runs')
|
||||
@ProjectScope('agent:read')
|
||||
async listRuns(req: AuthenticatedRequest<DatasetParam>): Promise<AgentEvalRunRecord[]> {
|
||||
async listRuns(
|
||||
req: AuthenticatedRequest<DatasetParam>,
|
||||
_res: unknown,
|
||||
@Query query: PaginationDto,
|
||||
): Promise<AgentEvalRunList> {
|
||||
await this.flagGate.assertEnabled(req.user);
|
||||
const { agentId, projectId, datasetId } = req.params;
|
||||
return await this.service.listRuns(agentId, projectId, datasetId);
|
||||
return await this.service.listRuns(agentId, projectId, datasetId, query);
|
||||
}
|
||||
|
||||
/** `take`/`skip` page the run's cases, not the run itself. */
|
||||
@Get('/:agentId/evals/runs/:runId')
|
||||
@ProjectScope('agent:read')
|
||||
async getRun(req: AuthenticatedRequest<RunParam>): Promise<AgentEvalRunDetail> {
|
||||
async getRun(
|
||||
req: AuthenticatedRequest<RunParam>,
|
||||
_res: unknown,
|
||||
@Query query: AgentEvalRunDetailQueryDto,
|
||||
): Promise<AgentEvalRunDetail> {
|
||||
await this.flagGate.assertEnabled(req.user);
|
||||
const { agentId, projectId, runId } = req.params;
|
||||
return await this.service.getRunDetail(agentId, projectId, runId);
|
||||
return await this.service.getRunDetail(agentId, projectId, runId, query);
|
||||
}
|
||||
|
||||
/** Counts only — cheap enough for the UI to poll while a run is in flight. */
|
||||
|
|
@ -155,8 +180,10 @@ export class AgentEvalsController {
|
|||
return await this.service.getRunSummary(agentId, projectId, runId);
|
||||
}
|
||||
|
||||
// Cancelling stops work someone else started, so it's a write, not an
|
||||
// execution — and `agent:execute` is all a chat-only user holds.
|
||||
@Post('/:agentId/evals/runs/:runId/cancel')
|
||||
@ProjectScope('agent:execute')
|
||||
@ProjectScope('agent:update')
|
||||
async cancelRun(req: AuthenticatedRequest<RunParam>): Promise<AgentEvalRunRecord> {
|
||||
await this.flagGate.assertEnabled(req.user);
|
||||
const { agentId, projectId, runId } = req.params;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user