Repository navigation
fix(logs): keep the provider's closed error class and finishReason on failure rows - #8489
Conversation
… failure rows A Vertex/Gemini rejection reached the request log as a bare request_rejected: Google puts its class in error.status (not error.type), the Vertex and Antigravity transports rewrite the body for display before dispatch reads it, and a mid-stream error frame or a truncated turn kept only English text on the context. Rows now carry the closed class (Anthropic envelope types plus the canonical google.rpc.Code names) and the Gemini FinishReason member on failureDetail, never the provider message, and a later successful replay retracts the detail. Refs #8446
|
Maintainer judgment
|
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
Maintainer judgment — merge record
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7a7a9654d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const observeEvent = (event: AdapterEvent, response: object): void => { | ||
| delivery.noteAdapterEvent(); | ||
| observeUsage(event, response); | ||
| observeProviderFailureEvent(logCtx, event); |
There was a problem hiding this comment.
Forward late provider failure details to the combo parent
For a streaming combo request that emits text before a Google error frame or Vertex truncation arrives, this observer records the new detail only on the combo's childLog. In core-combo.ts, the parent copies that context once when the child commits; subsequent callbacks forward usage and terminal metadata, but not failureDetail. Consequently, the final request row loses the provider class or finish reason for failures occurring after commitment, even though the same failure is recorded on a direct route. Forward these updates through the committed-child callbacks, including removal when a replay recovers, and cover a combo stream whose failure arrives after its first output.
Useful? React with 👍 / 👎.
Summary
A provider that rejects a request through a Google route left almost nothing in
ocx logs getorusage.jsonl(#8446). A Vertex 400 for Gemini 4 Argon, reached through a combo, recorded onlyinvalid_request_error/request_rejected. Three things lost the provider's own facts:error.status(INVALID_ARGUMENT,FAILED_PRECONDITION, ...). The row capture only knew the Anthropicerror.typeclasses, and only kept them alongside schema-validation pairs.normalizeUpstreamHttpErrorResponse) before dispatch reads it, so even a correct parser saw no JSON.errorframe and a fail-closed Vertex truncation (MALFORMED_FUNCTION_CALL,MAX_TOKENSmid tool call) kept only English text on the request context.After this change, a failed row carries
failureDetail: { code: "provider_error", type: "INVALID_ARGUMENT" }and, for a truncated turn,finishReason: "MALFORMED_FUNCTION_CALL". The dashboard shows "The provider returned an error (INVALID_ARGUMENT)" and appends[finishReason: …]as a machine token, the same way validation pairs are shown.The privacy contract in
structure/data-planes/error-detail.mdis unchanged. Rows still hold only closed vocabularies: the Anthropic envelope types plus the canonicalgoogle.rpc.Codenames, and the GeminiFinishReasonenum. The provider's message never reaches a row,usage.jsonl,/api/logsor the service log. Facts travel from the adapter to the log through aWeakMapkeyed by the event or response object, so nothing new is serialized and no client-visible bytes change.Guards against mislabeling:
consumeComboFailurewhen dispatch passes it the raw upstream response. Proxy-marked responses and every otherconsumeComboFailurecaller keep the validation-only capture.doneon the same request retracts the recorded detail, so a retry or empty-completion replay that succeeds leaves no provider failure on a successful row.Out of scope: the provider's free-text message. The log contract forbids upstream text, and the lane brief kept this to codes and
finishReason.reasoningOutputTokensis already recorded whenever Gemini or Vertex reportthoughtsTokenCount(usageFromGemini, streaming and buffered). Whether Argon on Vertex streaming omits it needs a captured frame; no code change is made on a guess.Closes #8446
Verification
bun test tests/usage/provider-failure-detail.test.ts: 12 pass. The new file covers the closed contract, the object and streamed-array Google bodies, and the Google adapter events. It also driveshandleResponseson the Vertex route withfetchintercepted (nothing leaves the process) for an HTTP 400, a combo child, a mid-stream error frame, a truncated turn, and a replay that recovers.devand the new exports kept, the six behavioral cases fail (both adapter-event cases and four row cases); they pass with the fix.bun run typecheck: no type errors in 2,379 files.tests/usage/request-log-error-detail.test.ts,upstream-failure-diagnostics,tests/server/upstream-error-detail-boundaries.test.ts,tests/adapters/google/*,chat-error-body-normalization,server-chat-error-body-evidence,request-log-normalization,upstream-http-error. All pass when each file runs on its own. Run together in one process, 31 Antigravity discovery cases fail from shared global state; those three files pass alone, 56/56.bun run test:changed(which selected 1,596 files becauserequest-transport.tsis widely imported): 37,059 pass, 59 skip, 17 fail. Every failing file was rerun alone and passes, excepttests/codex-integration/catalog-omit-disabled-native.test.ts("retained sync and serve-options omit the same rows"), which fails identically on cleandev. The others were 5-second timeouts under 4x parallel load, plus theocx-childe2e that needs cargo.bun run privacy:scan,bun run structure:check,bun run lint:gui: pass. Layout guards, the file-size ratchet, and the Lab core boundary: 75 pass after mergingdevwith fix(ci): bring layout.json and the Anthropic pool doc back under their caps #8485.bun run testas one invocation (test:changedalready selected most of the suite); macOS and Windows CI (manualdevbatch only, per policy).Checklist