Skip to content

fix(logs): keep the provider's closed error class and finishReason on failure rows - #8489

Merged
lidge-jun merged 2 commits into
devfrom
codex/issues-account-pool-1111
Nov 10, 2026
Merged

lidge-jun merged 2 commits into
devfrom
codex/issues-account-pool-1111

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

A provider that rejects a request through a Google route left almost nothing in ocx logs get or usage.jsonl (#8446). A Vertex 400 for Gemini 4 Argon, reached through a combo, recorded only invalid_request_error / request_rejected. Three things lost the provider's own facts:

  • Gemini, Vertex and Antigravity put their error class in error.status (INVALID_ARGUMENT, FAILED_PRECONDITION, ...). The row capture only knew the Anthropic error.type classes, and only kept them alongside schema-validation pairs.
  • The Vertex and Antigravity transports rewrite the HTTP error body into display text (normalizeUpstreamHttpErrorResponse) before dispatch reads it, so even a correct parser saw no JSON.
  • A mid-stream error frame and a fail-closed Vertex truncation (MALFORMED_FUNCTION_CALL, MAX_TOKENS mid 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.md is unchanged. Rows still hold only closed vocabularies: the Anthropic envelope types plus the canonical google.rpc.Code names, and the Gemini FinishReason enum. The provider's message never reaches a row, usage.jsonl, /api/logs or the service log. Facts travel from the adapter to the log through a WeakMap keyed by the event or response object, so nothing new is serialized and no client-visible bytes change.

Guards against mislabeling:

  • The broader capture runs only where the provider's own body is in hand: native Chat, adapter dispatch, and consumeComboFailure when dispatch passes it the raw upstream response. Proxy-marked responses and every other consumeComboFailure caller keep the validation-only capture.
  • A later done on 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. reasoningOutputTokens is already recorded whenever Gemini or Vertex report thoughtsTokenCount (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

Logs failure detail with the provider finishReason

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 drives handleResponses on the Vertex route with fetch intercepted (nothing leaves the process) for an HTTP 400, a combo child, a mid-stream error frame, a truncated turn, and a replay that recovers.
  • Fails before the fix: with the six capture sites restored to dev and 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.
  • Related suites, run per file: 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 because request-transport.ts is widely imported): 37,059 pass, 59 skip, 17 fail. Every failing file was rerun alone and passes, except tests/codex-integration/catalog-omit-disabled-native.test.ts ("retained sync and serve-options omit the same rows"), which fails identically on clean dev. The others were 5-second timeouts under 4x parallel load, plus the ocx-child e2e 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 merging dev with fix(ci): bring layout.json and the Anthropic pool doc back under their caps #8485.
  • Independent reviews on the final code: correctness APPROVE, privacy/security SECURITY_APPROVE.
  • NOT RUN: full bun run test as one invocation (test:changed already selected most of the suite); macOS and Windows CI (manual dev batch only, per policy).

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

cpa added 2 commits November 11, 2026 03:17
… 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
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner November 10, 2026 18:23
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer judgment

  • Value: makes a Google/Vertex/Antigravity rejection diagnosable from ocx logs get without replaying a paid call, which is the gap Keep provider error code/message, finishReason and thoughtsTokenCount in failure logs (no content) #8446 reported twice on v2.99.0.
  • UX change scope: the Logs "Failure detail" line now names the provider's published error class (for example INVALID_ARGUMENT) and, for a truncated Vertex turn, the finishReason. No new column, no locale strings, no client-visible response bytes change.
  • Regression evidence for existing users: rows that already had a validation detail keep it unchanged; the broader capture is confined to call sites that hold the provider's own body and skips proxy-marked responses; a recovered replay retracts the detail (tested). Older usage.jsonl rows read as before, since boundedFailureDetail only adds two optional closed fields.
  • Windows impact: none expected; no filesystem, process or path code changed.
  • Validation scope: focused file (12 cases, 6 fail without the capture sites), typecheck, related suites run per file, test:changed across 1,596 files with only pre-existing or load-timing failures, privacy/structure/layout/ratchet/Lab guards, lint:gui, dashboard screenshot from a seeded row.
  • NOT RUN: full bun run test as one invocation; macOS/Windows CI (manual batch on dev only).

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T18:25:39.044308Z d7a7a96 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Nov 10, 2026
@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer judgment — merge record

  • Reviewed head: 8ffd532 (independent correctness review: APPROVE; independent privacy/security review: SECURITY_APPROVE; reviewer model codex / hyds/deepseek-v4.1-flash-1m).
  • Merge head: d7a7a96 = reviewed head + merge of dev (fix(ci): bring layout.json and the Anthropic pool doc back under their caps #8485 only). The PR diff is byte-identical before and after that merge (same git patch-id), so no re-review was needed.
  • bun run test:changed (1,596 files): 37,059 pass, 59 skip, 17 fail. 16 of the 17 pass when their files run alone (codex-refresh, link-relay-bound-transport, status-install-method, ocx-launcher-desktop-handoff, native-codex-toggle, and the ocx-child e2e cases that need cargo): 5-second timeouts under 4x parallel load or a missing local toolchain. The remaining one, tests/codex-integration/catalog-omit-disabled-native.test.ts ("retained sync and serve-options omit the same rows without changing routed bytes"), fails the same way on clean dev.
  • PR CI on d7a7a96 at merge time: no failed check. changes, structure gate, storage policy, docs site build, validate-source, enforce-target, hygiene, react-doctor and label passed; the test shards, gates, docker smoke, api usage, npm-global and CodeRabbit were still pending and were not waited on, per the lane policy.
  • Merged with --admin because GitHub blocks on REVIEW_REQUIRED only.
  • NOT RUN: full bun run test as one invocation; macOS/Windows CI (manual dev batch only).

@lidge-jun
lidge-jun merged commit 61151e3 into dev Nov 10, 2026
28 of 31 checks passed
@lidge-jun
lidge-jun deleted the codex/issues-account-pool-1111 branch November 10, 2026 18:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep provider error code/message, finishReason and thoughtsTokenCount in failure logs (no content)

1 participant