-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix #2155 DeepSeek reasoning content handling for LiteLLM #2156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix #2155 DeepSeek reasoning content handling for LiteLLM #2156
Conversation
|
Thanks for sending this PR! One thing I want to check is that if this customization could be done on the LiteLLM side, that'd be the most appropriate situation for us. Have you considered contributing to the LiteLLM project? |
|
This PR doesn’t cover the case where DeepSeek is accessed directly through its OpenAI-compatible endpoint. For example: custom_client = AsyncOpenAI(
base_url="https://api.deepseek.com",
api_key=os.getenv("DEEPSEEK_API_KEY")
)
model = OpenAIChatCompletionsModel(
model="deepseek-reasoner",
openai_client=custom_client,
)
agent = Agent(
name="Assistant",
model= model,
...
)Since the SDK’s internal conversion flow here through Also, I’m currently updating Gemini 3 Pro support in PR 2158, so there might be some overlapping changes or conflicts to be aware of. |
Yes—looked into it. LiteLLM already returns reasoning_content on DeepSeek responses, but it doesn’t (and can’t, without keeping state) auto-inject that into subsequent assistant tool-call messages. To make it automatic upstream, they’d need an opt-in flag like “auto_include_reasoning_content” that replays the last reasoning text, or at least a warning/helper for DeepSeek + thinking + tools. I’ve drafted a LiteLLM docs update showing how to echo reasoning_content and linking to DeepSeek’s official guide; and already turned that into a PR. |
I picked this up and extended the fix to cover DeepSeek when it’s accessed via an OpenAI-compatible client (not just LiteLLM). Changes: Add reasoning_content forwarding for DeepSeek in OpenAIChatCompletionsModel so tool-call messages include reasoning_content when model name/base_url indicates DeepSeek or thinking is enabled. This should cover both LiteLLM and direct DeepSeek/OpenAI-compatible paths. |
| ) | ||
| return response, ret | ||
|
|
||
| def _should_include_reasoning_content(self, model_settings: ModelSettings) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic here should be strictly restricted to DeepSeek models only to avoid potential regressions for other providers.
According to the DeepSeek Thinking Mode docs, we should only return True in these two cases:
- The model_name contains
deepseek-reasoner.
or - The model_name contains
deepseekand thethinkingparameter is explicitly set.
Right now your logic will enable this whenever a thinking parameter is passed for non-DeepSeek models, and also for deepseek-chat even when thinking is not set.
Propagate
reasoning_contentonto assistant tool-call messages when using DeepSeek/thinking mode via LiteLLM. Extend the chat completion converter to optionally include reasoning content and enable it for DeepSeek. Added regression test covering reasoning content attachment.Fix LiteLLM DeepSeek thinking mode fails with missing
reasoning_contenon tool calls #2155make formatmake lintmake mypymake tests