Skip to content
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

Handle azure deepseek reasoning response #8288

Merged

Conversation

vibhavbhat
Copy link
Contributor

@vibhavbhat vibhavbhat commented Feb 5, 2025

Title

Relevant issues

Fixes #8267

Type

πŸ†• New Feature
πŸ› Bug Fix
βœ… Test

Changes

Some deepseek model hosts (specifically Azure) returns the reasoning content within the content itself as <think>Thinking Content<think>User facing response. This PR properly handles that case and puts the reasoning content on the api response as intended. This fix should work in general for all hosts that set the reasoning content like this.

[REQUIRED] Testing - Attach a screenshot of any new tests passing locally

If UI changes, send a screenshot/GIF of working UI fixes

Added test w/ reasoning content and it passed
image
image

Unit test for helper also passes
image

Copy link

vercel bot commented Feb 5, 2025

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
litellm βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Feb 6, 2025 7:32am

@vibhavbhat
Copy link
Contributor Author

@krrishdholakia

@@ -415,8 +416,17 @@ def convert_to_model_response_object( # noqa: PLR0915
for field in choice["message"].keys():
if field not in message_keys:
provider_specific_fields[field] = choice["message"][field]

# Handle reasoning models that display `reasoning_content` within `content``
content = choice["message"].get("content", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have this be in a separate helper function - it'll be easier to add a unit test for this as well

@krrishdholakia krrishdholakia self-assigned this Feb 5, 2025
@krrishdholakia
Copy link
Contributor

Thank you for the work on this @vibhavbhat - if you can just refactor the check to be in a separate helper function and add a direct test for that (prevents any future regression), i think we should be good!

@vibhavbhat
Copy link
Contributor Author

@krrishdholakia Added helper + unit tests

@vibhavbhat
Copy link
Contributor Author

@krrishdholakia is this good to merge?

@krrishdholakia krrishdholakia changed the base branch from main to litellm_dev_02_07_2025_p1 February 7, 2025 17:10
@krrishdholakia krrishdholakia merged commit dfb0266 into BerriAI:litellm_dev_02_07_2025_p1 Feb 7, 2025
2 checks passed
krrishdholakia added a commit that referenced this pull request Feb 8, 2025
* Handle azure deepseek reasoning response (#8288)

* Handle deepseek reasoning response

* Add helper method + unit test

* Fix: Follow infinity api url format (#8346)

* Follow infinity api url format

* Update test_infinity.py

* fix(infinity/transformation.py): fix linting error

---------

Co-authored-by: vibhavbhat <[email protected]>
Co-authored-by: Hao Shan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Missing the [include_reasoning] content, but I only get the <think> in content(not stream)
2 participants