feat: [ENG-3645] AI Gateway Reasoning Support - Anthropic #5303
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @connortbot's task —— View job PR Review: AI Gateway Reasoning Support - AnthropicTodo List:
Review AnalysisCritical Issues Found1. Silent Temperature Override (Critical)Location: // temperature 1 is required for Anthropic models to enable reasoning
antBody.temperature = 1;Issue: This unconditionally overwrites any user-provided temperature when reasoning is enabled. If a user sets Suggested Fix: if (antBody.temperature !== undefined && antBody.temperature !== 1) {
console.warn(`Temperature changed from ${antBody.temperature} to 1 for reasoning compatibility`);
}
antBody.temperature = 1;2. Data Loss in Stream Consolidation (Critical)Location: reasoning_details:
c.delta.reasoning_details && cur.choices[i].delta.reasoning_details
? [...c.delta.reasoning_details, ...cur.choices[i].delta.reasoning_details]
: c.delta.reasoning_details || cur.choices[i].delta.reasoning_details,Issue: The logic is inconsistent with Suggested Fix: reasoning_details: cur.choices[i].delta.reasoning_details
? [...(c.delta.reasoning_details || []), ...cur.choices[i].delta.reasoning_details]
: c.delta.reasoning_details,Code Quality Issues3. Type Safety ViolationsLocation: throw new Error(`Unsupported content type: ${(part as any).type}`);Issue: Using 4. Inconsistent Error HandlingLocation: private requiresReasoningOptions(providerModelId: string): boolean {
return providerModelId.includes("claude");
}Issue: String-based provider detection is fragile. Consider using enum or more robust pattern matching. Positive Aspects✅ Comprehensive Type Definitions: Good addition of reasoning types across the codebase Security & Performance🔒 Security: No security issues identified - proper input validation and sanitization Minor Issues
Score: 6/10 - Minor issues with critical data handling bugs Suggestions Summary:
The implementation is solid overall with good architecture, but the two critical data handling issues need to be addressed before merging. |
Greptile OverviewGreptile SummaryThis PR implements reasoning support for Anthropic's thinking feature on the AI Gateway. It adds bidirectional mapping between OpenAI's reasoning format and Anthropic's thinking blocks, enabling users to leverage Anthropic's extended thinking capabilities through a unified interface. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client
participant Gateway as AI Gateway
participant Mapper as LLM Mapper
participant Anthropic
Client->>Gateway: POST /chat/completions<br/>{reasoning_effort, reasoning_options}
Gateway->>Gateway: Validate request
Gateway->>Mapper: toAnthropic()
Mapper->>Mapper: Map reasoning_options.budget_tokens<br/>to thinking.budget_tokens
Mapper->>Mapper: Set temperature = 1<br/>(required for thinking)
Mapper->>Mapper: Convert messages<br/>(thinking blocks first)
Mapper->>Anthropic: POST /messages<br/>{thinking: {budget_tokens}}
alt Streaming Response
Anthropic-->>Mapper: thinking_delta events
Mapper->>Mapper: Accumulate thinking + signature
Mapper-->>Gateway: Stream reasoning deltas
Gateway-->>Client: Stream with reasoning field
Anthropic-->>Mapper: message_delta (final)
Mapper->>Mapper: Collect reasoning_details<br/>from accumulated state
Mapper-->>Gateway: Final chunk with reasoning_details
Gateway-->>Client: Complete stream
else Non-Streaming Response
Anthropic-->>Mapper: Response with thinking blocks
Mapper->>Mapper: Extract thinking blocks
Mapper->>Mapper: Map to reasoning + reasoning_details
Mapper-->>Gateway: OpenAI format response
Gateway-->>Client: Response with reasoning fields
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
packages/llm-mapper/transform/providers/openai/request/toAnthropic.ts
Outdated
Show resolved
Hide resolved
| (this.requestWrapper.heliconeHeaders.gatewayConfig.bodyMapping === "OPENAI" | ||
| || this.requestWrapper.heliconeHeaders.gatewayConfig.bodyMapping === "RESPONSES" | ||
| ) |
There was a problem hiding this comment.
it was wrong to begin with, responses API automatically includes usage.
the logic here was moved when we map responses input to chat completions input.
H2Shami
left a comment
H2Shami
left a comment
There was a problem hiding this comment.
lgtm, but i think it would be smart to have @chitalian take a look
Other notable fixes
streamParserto consolidate reasoning deltas