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!
This commit is contained in:
Koosha Paridehpour
2026-09-14 19:27:15 -07:00
committed by GitHub
parent cce3a958b5
commit 97ae10179c
2 changed files with 45 additions and 2 deletions

View File

@@ -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,

View File

@@ -28,7 +28,11 @@ type CodexRotationEnvelope = {
};
function baseCtx(overrides: Record<string, unknown> = {}) {
// #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");
});