From 97ae10179caf8eca27a68c0d50ea4cd188c84467 Mon Sep 17 00:00:00 2001 From: Koosha Paridehpour <42529354+KooshaPari@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:27:15 -0700 Subject: [PATCH] fix(combo): use traceId for call log id to prevent UNIQUE constraint failure (#13546) Combo attempt call logs are keyed on the per-attempt `traceId` instead of the shared `pendingRequestId` (#13481). Every attempt of a failed-over combo reused the same id, so the successful member hit the `call_logs` UNIQUE constraint and was silently dropped: the dashboard showed only the failed steps. Maintainer additions: carried your regression test from #13526 (`combo attempt uses traceId as the log id, not pendingRequestId`, two attempts sharing one request id both persist) into this cleaner branch. Removed the now-unused `pendingRequestId` destructure (`no-unused-vars`) and added a short comment at the call site. Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green. Thanks @KooshaPari! --- open-sse/handlers/chatCore/attemptLogging.ts | 6 ++- tests/unit/chatcore-attempt-logging.test.ts | 41 ++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/open-sse/handlers/chatCore/attemptLogging.ts b/open-sse/handlers/chatCore/attemptLogging.ts index 3192ddbd9b..f1ddcc1678 100644 --- a/open-sse/handlers/chatCore/attemptLogging.ts +++ b/open-sse/handlers/chatCore/attemptLogging.ts @@ -359,7 +359,6 @@ export function persistAttemptLogs(args: PersistAttemptLogsArgs, ctx: PersistAtt skillRequestId, detailedLoggingEnabled, reqLogger, - pendingRequestId, clientRawRequest, requestedModel, credentials, @@ -458,8 +457,11 @@ export function persistAttemptLogs(args: PersistAttemptLogsArgs, ctx: PersistAtt } } + // #13481: each combo attempt needs its own row. Attempts share pendingRequestId, so + // keying the log on it made the successful member's insert hit the UNIQUE constraint + // and vanish from the dashboard; traceId is per attempt and pairs with request.started. saveCallLog({ - id: pendingRequestId, + id: traceId, method: "POST", path: clientRawRequest?.endpoint || "/v1/chat/completions", status, diff --git a/tests/unit/chatcore-attempt-logging.test.ts b/tests/unit/chatcore-attempt-logging.test.ts index 2af6bbd41c..785409c93d 100644 --- a/tests/unit/chatcore-attempt-logging.test.ts +++ b/tests/unit/chatcore-attempt-logging.test.ts @@ -28,7 +28,11 @@ type CodexRotationEnvelope = { }; function baseCtx(overrides: Record = {}) { + // #13481: traceId defaults to pendingRequestId so existing tests (which poll + // by pendingRequestId) continue to work. Combo tests set both explicitly. + const pendingRequestId = (overrides.pendingRequestId as string) ?? "REPLACE"; return { + traceId: overrides.traceId ?? pendingRequestId, provider: "openai", connectionId: "conn-1", model: "gpt-x", @@ -195,3 +199,40 @@ test("unique tool_calls do not write provider.spec_violation audit", () => { }); assert.equal(rows.length, 0); }); + +// #13481: Combo attempts must use traceId as the log id, not pendingRequestId. +// When a combo fails over, each attempt has a unique traceId but shares the +// same pendingRequestId. Using pendingRequestId as the log id caused a UNIQUE +// constraint violation — only the first (failed) attempt was logged. +test("combo attempt uses traceId as the log id, not pendingRequestId", async () => { + const traceId = "combo-trace-attempt-2"; + const pendingRequestId = "combo-shared-request-id"; + persistAttemptLogs( + { status: 200, tokens: { input: 10, output: 20 } }, + baseCtx({ + traceId, + pendingRequestId, + comboName: "my-combo", + comboStepId: "my-combo-model-2", + }) + ); + const row = await pollForCallLog(traceId); + assert.ok(row, "call log row should be persisted with traceId as id"); + assert.equal(row.status, 200); + assert.equal(row.comboStepId, "my-combo-model-2"); + + // A second attempt with the same pendingRequestId but different traceId + const traceId2 = "combo-trace-attempt-3"; + persistAttemptLogs( + { status: 200, tokens: { input: 30, output: 40 } }, + baseCtx({ + traceId: traceId2, + pendingRequestId, + comboName: "my-combo", + comboStepId: "my-combo-model-3", + }) + ); + const row2 = await pollForCallLog(traceId2); + assert.ok(row2, "second combo attempt should also be persisted"); + assert.equal(row2.comboStepId, "my-combo-model-3"); +});