Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions apps/web/src/routers/admin-code-reviews-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,93 @@ describe('adminCodeReviewsRouter', () => {
expect(categoryNames).not.toContain('Upstream Server Error');
});

// The status-code branches used bare substring matches ('%429%', '%500%'),
// so any digits embedded in an id, byte count or duration were bucketed as an
// HTTP failure. These rows carry no terminal_reason, so they exercise the
// message fallback directly.
it('does not treat embedded digits as HTTP status codes', async () => {
const owner = { type: 'user', id: adminUser.id } satisfies ReviewOwner;

await db.insert(cloud_agent_code_reviews).values([
reviewValues({
owner,
status: 'failed',
createdAt: timestamp(800),
errorMessage: 'Upload finished after 4290 ms',
}),
reviewValues({
owner,
status: 'failed',
createdAt: timestamp(805),
errorMessage: 'Agent processed 5000 items',
}),
reviewValues({
owner,
status: 'failed',
createdAt: timestamp(810),
errorMessage: 'Reference req_4045 was rejected',
}),
]);

const caller = await createCallerForUser(adminUser.id);
const categoryNames = (
await caller.admin.codeReviews.getErrorAnalysis(filterInput())
).categories.map(category => category.category);

expect(categoryNames).not.toContain('Rate Limited');
expect(categoryNames).not.toContain('Upstream Server Error');
expect(categoryNames).not.toContain('Not Found');
});

it('still buckets genuine HTTP status codes', async () => {
const owner = { type: 'user', id: adminUser.id } satisfies ReviewOwner;

await db.insert(cloud_agent_code_reviews).values([
reviewValues({
owner,
status: 'failed',
createdAt: timestamp(820),
errorMessage: 'Provider returned HTTP 429',
}),
reviewValues({
owner,
status: 'failed',
createdAt: timestamp(825),
errorMessage: 'Upstream responded with (503)',
}),
]);

const caller = await createCallerForUser(adminUser.id);
const errors = await caller.admin.codeReviews.getErrorAnalysis(filterInput());

expect(errors.categories).toEqual(
expect.arrayContaining([
expect.objectContaining({ category: 'Rate Limited', count: 1 }),
expect.objectContaining({ category: 'Upstream Server Error', count: 1 }),
])
);
});

it('matches rate limit text regardless of case', async () => {
const owner = { type: 'user', id: adminUser.id } satisfies ReviewOwner;

await db.insert(cloud_agent_code_reviews).values([
reviewValues({
owner,
status: 'failed',
createdAt: timestamp(830),
errorMessage: 'RATE LIMIT exceeded for this key',
}),
]);

const caller = await createCallerForUser(adminUser.id);
const errors = await caller.admin.codeReviews.getErrorAnalysis(filterInput());

expect(errors.categories).toEqual(
expect.arrayContaining([expect.objectContaining({ category: 'Rate Limited', count: 1 })])
);
});

it('classifies final model-not-found outcomes as cancellations instead of failures', async () => {
const owner = { type: 'user', id: adminUser.id } satisfies ReviewOwner;

Expand Down
24 changes: 20 additions & 4 deletions apps/web/src/routers/admin-code-reviews-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,32 @@ function buildErrorCategoryExpr(terminalReasonColumn: PgColumn, errorMessageColu
sql`WHEN ${terminalReasonColumn} = ${literal(reason)} THEN ${literal(label)}`
);

// Bare HTTP status codes need word boundaries. `LIKE '%429%'` matches the
// digits anywhere, so a session id, byte count, duration or timestamp
// containing 429 was bucketed as Rate Limited, and '%500%' caught things like
// "5000 tokens". `\y` is the Postgres regex word boundary, so 429 matches in
// "HTTP 429" but not in "4290", "14290" or "req_429abc".
// Inlined for the same reason as the reason labels above: this expression is
// used in both SELECT and GROUP BY, and bound parameters are renumbered by
// position, so Postgres would stop treating the two as the same expression.
// The pattern is built from a number literal, so there is nothing to escape.
const httpStatus = (code: number) => sql`${errorMessageColumn} ~ ${sql.raw(`'\\y${code}\\y'`)}`;
const anyHttpStatus = (...codes: number[]) =>
sql.join(
codes.map(code => httpStatus(code)),
sql.raw(' OR ')
);

return sql<string>`CASE
${sql.join(reasonCases, sql.raw(' '))}
WHEN ${errorMessageColumn} LIKE '%sandbox storage full%' OR ${errorMessageColumn} LIKE '%admission rejected%' OR ${errorMessageColumn} LIKE '%storage full%' THEN 'Sandbox Capacity'
WHEN ${errorMessageColumn} LIKE '%connect to the sandbox%' OR ${errorMessageColumn} LIKE '%Sandbox connection failed%' OR ${errorMessageColumn} LIKE '%container shut down%' THEN 'Sandbox Connection'
WHEN ${errorMessageColumn} LIKE '%rate limit%' OR ${errorMessageColumn} LIKE '%Rate limit%' OR ${errorMessageColumn} LIKE '%429%' THEN 'Rate Limited'
WHEN ${errorMessageColumn} ILIKE '%rate limit%' OR ${anyHttpStatus(429)} THEN 'Rate Limited'
WHEN ${errorMessageColumn} LIKE '%timeout%' OR ${errorMessageColumn} LIKE '%Timeout%' OR ${errorMessageColumn} LIKE '%ETIMEDOUT%' OR ${errorMessageColumn} LIKE '%timed out%' THEN 'Timeout'
WHEN ${errorMessageColumn} LIKE '%context window%' OR ${errorMessageColumn} LIKE '%token limit%' OR ${errorMessageColumn} LIKE '%too large%' OR ${errorMessageColumn} LIKE '%maximum context length%' THEN 'Context Window Exceeded'
WHEN ${errorMessageColumn} LIKE '%authentication%' OR ${errorMessageColumn} LIKE '%401%' OR ${errorMessageColumn} LIKE '%403%' OR ${errorMessageColumn} LIKE '%permission%' THEN 'Auth / Permission Error'
WHEN ${errorMessageColumn} LIKE '%not found%' OR ${errorMessageColumn} LIKE '%404%' THEN 'Not Found'
WHEN ${errorMessageColumn} LIKE '%500%' OR ${errorMessageColumn} LIKE '%502%' OR ${errorMessageColumn} LIKE '%503%' OR ${errorMessageColumn} LIKE '%internal server%' OR ${errorMessageColumn} LIKE '%Internal Server%' THEN 'Upstream Server Error'
WHEN ${errorMessageColumn} ILIKE '%authentication%' OR ${errorMessageColumn} ILIKE '%permission%' OR ${anyHttpStatus(401, 403)} THEN 'Auth / Permission Error'
WHEN ${errorMessageColumn} ILIKE '%not found%' OR ${anyHttpStatus(404)} THEN 'Not Found'
WHEN ${errorMessageColumn} ILIKE '%internal server%' OR ${anyHttpStatus(500, 502, 503)} THEN 'Upstream Server Error'
WHEN ${errorMessageColumn} LIKE '%ECONNREFUSED%' OR ${errorMessageColumn} LIKE '%ECONNRESET%' OR ${errorMessageColumn} LIKE '%socket hang up%' OR ${errorMessageColumn} LIKE '%network%' THEN 'Network Error'
WHEN ${errorMessageColumn} LIKE '%parse%' OR ${errorMessageColumn} LIKE '%JSON%' OR ${errorMessageColumn} LIKE '%unexpected token%' THEN 'Parse Error'
WHEN ${errorMessageColumn} LIKE '%could not be delivered%' THEN 'Delivery Failure'
Expand Down