fix(messages): stop forwarding Anthropic-only fields to OpenAI-compatible upstreams - #710
Conversation
…ible upstreams The /v1/messages cross-provider path preserved every unrecognized Anthropic top-level field in ChatFormat.extra, and the OpenAI bridge flattens extras into the upstream request body. Anthropic-only fields (context_management, thinking, top_k, mcp_servers, ...) reached OpenAI/Azure upstreams verbatim, which reject them: 400 "Unknown parameter: 'context_management'". Replace the tools/tool_choice-only fixup with a whitelist translation (translate_extras_to_openai_shape): tools/tool_choice keep their existing translation, stop_sequences -> stop, metadata.user_id -> user, thinking -> reasoning_effort (LiteLLM-aligned budget buckets); every other Anthropic-only field is dropped, so future Anthropic SDK fields can't re-trigger the 400. Mirrors the whitelist-and-drop policy the /v1/responses bridge (#825) already applies in the opposite direction. Fixes api7/AISIX-Cloud#953
📝 WalkthroughWalkthroughThis PR adds a ChangesAnthropic-to-OpenAI extras translation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Dispatch as cross_provider_dispatch
participant Translator as translate_extras_to_openai_shape
participant Upstream as OpenAI-compatible upstream
Client->>Dispatch: POST /v1/messages (Anthropic body with extras)
Dispatch->>Translator: translate_extras_to_openai_shape(chat.extra)
Translator->>Translator: map tools/tool_choice, stop_sequences, metadata.user_id, thinking
Translator->>Translator: drop unrecognized Anthropic-only fields
Translator-->>Dispatch: translated extras
Dispatch->>Upstream: POST /v1/chat/completions (translated body)
Upstream-->>Dispatch: response
Dispatch-->>Client: response
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/src/cases/messages-cross-provider-extras-e2e.test.ts (1)
27-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
thinkingout of the drop-only set. It maps toreasoning_effort, so this case should assert the translated field instead of treating it as Anthropic-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/src/cases/messages-cross-provider-extras-e2e.test.ts` around lines 27 - 40, Move thinking out of ANTHROPIC_ONLY_FIELDS in messages-cross-provider-extras-e2e.test.ts, because it translates to reasoning_effort rather than being Anthropic-only. Update the test case that uses ANTHROPIC_ONLY_FIELDS to assert the translated reasoning_effort field instead, and keep only true drop-only extras in the Anthropic-only set. Use the ANTHROPIC_ONLY_FIELDS constant and the related cross-provider extras assertions to locate the test logic.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-provider-anthropic/src/wire.rs`:
- Around line 487-491: The current `thinking` handling in `wire.rs` always maps
to `reasoning_effort`, which can break non-reasoning upstreams like `gpt-4o`.
Update the `thinking` branch in the request translation logic to only emit
`reasoning_effort` when the target model/provider supports it, using the
existing `reasoning_effort_from_thinking` helper plus a capability/model gate.
Keep the `extra.insert("reasoning_effort", ...)` path behind that check so
Anthropic `thinking` is ignored or dropped for unsupported OpenAI models rather
than forwarded as an invalid field.
---
Nitpick comments:
In `@tests/e2e/src/cases/messages-cross-provider-extras-e2e.test.ts`:
- Around line 27-40: Move thinking out of ANTHROPIC_ONLY_FIELDS in
messages-cross-provider-extras-e2e.test.ts, because it translates to
reasoning_effort rather than being Anthropic-only. Update the test case that
uses ANTHROPIC_ONLY_FIELDS to assert the translated reasoning_effort field
instead, and keep only true drop-only extras in the Anthropic-only set. Use the
ANTHROPIC_ONLY_FIELDS constant and the related cross-provider extras assertions
to locate the test logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 488f4ddf-0133-42d9-96e8-92bbea4eaf46
📒 Files selected for processing (4)
crates/aisix-provider-anthropic/src/lib.rscrates/aisix-provider-anthropic/src/wire.rscrates/aisix-proxy/src/messages.rstests/e2e/src/cases/messages-cross-provider-extras-e2e.test.ts
Problem
When an Anthropic SDK / Claude Code client calls
/v1/messagestargeting a model whose upstream is OpenAI/Azure OpenAI, Anthropic-only top-level request fields leak into the upstream request body and the upstream rejects them:The chain:
parse_inbound_request()copies every non-whitelisted top-level key intoChatFormat.extra;cross_provider_dispatch()only translatestools/tool_choice;OpenAiRequest.extrais#[serde(flatten)], socontext_management(andthinking,top_k,mcp_servers, …) land verbatim on the OpenAI wire.Fix
Replace the tools/tool_choice-only fixup in
cross_provider_dispatchwith a whitelist translation,translate_extras_to_openai_shape()in the anthropic provider crate:tools/tool_choice→ existing OpenAI-shape translation (bug: /v1/messages silently drops tools and tool_choice when upstream is OpenAI-compatible #236 behavior unchanged)stop_sequences→stopmetadata.user_id→user(LiteLLM parity)thinking→reasoning_effort(LiteLLM-aligned buckets: budget_tokens ≥4096 high / ≥2048 medium / ≥1024 low / below minimal;adaptive→ medium;disabled→ omitted)context_management,top_k,mcp_servers,container,service_tier,betas, …) is dropped (debug-logged), so future Anthropic SDK fields cannot re-trigger the 400This mirrors the whitelist-and-drop policy the
/v1/responsesbridge (#825) already applies in the opposite direction. Both streaming and non-streaming go through the sameChatFormatbuild, so one fix covers both.Behavior changes / compatibility
/v1/messagespassthrough (Anthropic upstream) is untouched — bodies still forward verbatim,context_managementkeeps working there.tools/tool_choicefrom extras, so dropping the rest changes nothing for them.88e03e5): its Anthropic→OpenAI adapter is also whitelist-based and mapsmetadata.user_id→userandthinking→reasoning_effort; it translatescontext_managementinto OpenAI Responses-API compaction (or polyfills it in-gateway on the chat path) rather than dropping — that fuller treatment is deferred to a follow-up issue.stop_sequences→stopgoes beyond LiteLLM (which silently drops it) since chat/completions supportsstopnatively.Tests
translate_extras_to_openai_shape(drop set, mappings, thinking buckets).messages-cross-provider-extras-e2e.test.ts(non-streaming + streaming): asserts Anthropic-only fields never reach the mock OpenAI upstream while translated fields survive. Fails before this fix (context_managementleaked), passes after.Fixes api7/AISIX-Cloud#953
Summary by CodeRabbit
New Features
Bug Fixes
Tests