Repository navigation
Conversation
…empt id usageHistory.ts's trackPendingRequest() deliberately reuses one pending id across every combo fallback target of a single client request (so a dashboard tab's live poll survives a fallback), and persistAttemptLogs() calls saveCallLog() once per attempt with that same id -- but saveCallLog() still did a plain INSERT. Every attempt after the first died with "UNIQUE constraint failed: call_logs.id" and was silently dropped: the row permanently kept the FIRST (often a failed) attempt's data, and when the dropped attempt was the eventual success, its response_id/artifact were lost too -- breaking resolvePreviousResponseState's continuation lookup for that response id (surfaces to the client as previous_response_not_found on its very next turn). Converts the INSERT to an upsert (ON CONFLICT(id) DO UPDATE) so the latest attempt's data -- reflecting this request's current/final state -- replaces the row instead of being discarded by it. Regression test proves a first-attempt failure followed by a second-attempt success on the same id ends up with exactly one row carrying the SUCCESSFUL attempt's status/model/response_id, and fails on the prior plain-INSERT code.
… of failing closed on a reused attempt id) into dev/omniroute-dev-combined
|
Thanks for the thorough root-cause writeup and the regression test — this is solid work. In Triage note: this is the review recommendation — the close itself happens only after the maintainer's per-PR sign-off (and, where a superseding PR is named, after it has landed). Nothing is being closed by this comment. |
Problem
[callLogs] Failed to save call log: SqliteError: UNIQUE constraint failed: call_logs.idfires constantly in production logs -- dozens of times per hour under ordinary combo fallback traffic.Root cause
trackPendingRequest()(usageHistory.ts) deliberately reuses one pending id across every combo fallback target of a single client request:persistAttemptLogs()is called once per attempt with that same id, andsaveCallLog()still did a plainINSERT. Every attempt after the first fails closed with the UNIQUE constraint violation and is silently dropped (caught and logged, nothing else) -- the row permanently keeps the first attempt's data (often a failure), and when the dropped attempt was the eventual success, itsresponse_idand artifact are lost with it.That last part has a real downstream effect:
resolvePreviousResponseStatelooks up a stored continuation state byresponse_id. If the successful attempt's row never got written, a later turn'sprevious_response_idcontinuation lookup finds nothing and fails closed withprevious_response_not_found-- a clean, different-looking error, but ultimately caused by this same silent write failure.Fix
Converted the
INSERTtoINSERT ... ON CONFLICT(id) DO UPDATE SET <every non-id column> = excluded.<column>. The latest attempt reflects the request's current/final state, so it replaces the row instead of being discarded by it -- matching the intent already documented at thetrackPendingRequest()call site.No schema change:
call_logs.idis alreadyTEXT PRIMARY KEY.Known follow-up
Each attempt still writes its own on-disk artifact file (
writeCallArtifactAsync); an overwritten row's earlier artifact is now orphaned on disk rather than referenced. Not fixed here -- existing artifact-rotation/cleanup (separately covered by #12992) already reclaims disk space over time, and cleaning up the specific orphan immediately would need its own careful review ofcallLogArtifactWriter.ts.Testing
save-call-log-persistence.test.ts: a failed first attempt followed by a successful second attempt on the same id ends up as exactly one row carrying the successful attempt'sstatus/model/response_id. Confirmed it fails on the prior plain-INSERTcode (asserts200, gets the stale429).call-log*/call-logs-*/responses-continuation-storesuite (183 tests) passes.tsc -p tsconfig.typecheck-core.jsonclean.🤖 Generated with Claude Code