From 36e489c338541312b89b9a432a198cee0a4cc3d1 Mon Sep 17 00:00:00 2001 From: St0rmz1 Date: Mon, 27 Jul 2026 08:33:25 -0700 Subject: [PATCH] fix(code reviewer) error pattern precision. Correctly match error codes in the error code area in the events/logs --- .../routers/admin-code-reviews-router.test.ts | 87 +++++++++++++++++++ .../src/routers/admin-code-reviews-router.ts | 24 ++++- 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/apps/web/src/routers/admin-code-reviews-router.test.ts b/apps/web/src/routers/admin-code-reviews-router.test.ts index c138c8ee96..01a264aedb 100644 --- a/apps/web/src/routers/admin-code-reviews-router.test.ts +++ b/apps/web/src/routers/admin-code-reviews-router.test.ts @@ -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; diff --git a/apps/web/src/routers/admin-code-reviews-router.ts b/apps/web/src/routers/admin-code-reviews-router.ts index 228b8a7266..3b6c279fa8 100644 --- a/apps/web/src/routers/admin-code-reviews-router.ts +++ b/apps/web/src/routers/admin-code-reviews-router.ts @@ -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`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'