Skip to content

Conversation

@ZeroAurora
Copy link

@ZeroAurora ZeroAurora commented Nov 24, 2025

Closes #3446 . Related to #3515.
#3533 is doing the same.

  • Introduced openai_chat_custom_reasoning_field and openai_chat_include_reasoning_in_request to OpenAIModelProfile.
  • Updated deepseek_model_profile and moonshotai_model_profile to utilize the new fields.
  • Enhanced OpenAIChatModel to prefer custom reasoning fields and provide fallback options.
  • Added relevant documentations.

@ZeroAurora

This comment was marked as resolved.

@ZeroAurora

This comment was marked as resolved.

@ZeroAurora
Copy link
Author

ZeroAurora commented Nov 24, 2025

图片 This test is interesting: we are getting different line breaks by spilting thinks and texts and recombining them again (which the code does originally). I don't think it's good for context caching, but let's keep it for now.

@ZeroAurora ZeroAurora force-pushed the feat/custom-reasoning-field branch from ac74841 to 31b3042 Compare November 24, 2025 20:12
@ZeroAurora
Copy link
Author

ZeroAurora commented Nov 24, 2025

all pass full cov let's goooo
review appreciated @DouweM

@ZeroAurora
Copy link
Author

ZeroAurora commented Nov 26, 2025

Let me start work on this
my dev machine died so following ups might delay

@ZeroAurora

This comment was marked as resolved.

@ZeroAurora ZeroAurora force-pushed the feat/custom-reasoning-field branch from 2a249d5 to c37a657 Compare November 27, 2025 03:42
@ZeroAurora ZeroAurora requested a review from DouweM November 27, 2025 03:50
@ZeroAurora ZeroAurora force-pushed the feat/custom-reasoning-field branch 2 times, most recently from 1807d54 to c93aa15 Compare November 27, 2025 12:15
@ZeroAurora
Copy link
Author

ZeroAurora commented Dec 1, 2025

DeepSeek is releasing their stable v3.2 model today, recommending to send back the thinking content when performing agentic operations. Reference

They still recommend to drop thinking content when user sends a new prompt, mainly for bandwidth saving purposes. I will not implement this due to complexity.

- Introduced `openai_chat_custom_reasoning_field` and `openai_chat_include_reasoning_in_request` to `OpenAIModelProfile`.
- Updated `deepseek_model_profile` and `moonshotai_model_profile` to utilize the new fields.
- Enhanced `OpenAIChatModel` to prefer custom reasoning fields and provide fallback options.
- Added relevant documentations.
@ZeroAurora ZeroAurora force-pushed the feat/custom-reasoning-field branch from c93aa15 to 6b28ffa Compare December 1, 2025 12:19
@ZeroAurora ZeroAurora force-pushed the feat/custom-reasoning-field branch from 6b28ffa to a5e57a2 Compare December 1, 2025 12:20
@ZeroAurora
Copy link
Author

ZeroAurora commented Dec 1, 2025

I tested DeepSeek v3.2 in my codebase and discovered deepseek-reasoner does not support tool_choice=required. Shall I include the change in this PR or open another one after this is closed?

Edit: I've created a ticket with DeepSeek on this

This method may be overridden by subclasses of `OpenAIChatModel` to apply custom mappings.
"""
profile = OpenAIModelProfile.from_profile(self.profile)
custom_field = profile.openai_chat_custom_reasoning_field or ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not have this default to an empty string, that's just going to lead to confusion. I'd rather have it be None, and then deal with that in the for... below

"""
# The `reasoning_content` field is only present in DeepSeek models.
profile = OpenAIModelProfile.from_profile(self._model_profile)
custom_field = profile.openai_chat_custom_reasoning_field or ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as up

"""Whether the model includes reasoning content in requests.
This can be:
* `'thinking_tags'` (default): The reasoning content is included in the main `content` field, enclosed within thinking tags.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's refer to the thinking_tags profile setting, as we refer to openai_chat_custom_reasoning_field below

* `'custom_field'`: The reasoning content is included in a separate field specified by `openai_chat_custom_reasoning_field`.
* `False`: No reasoning content is sent in the request.
Defaults to `'thinking_tags'` for compatibility reasons."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Defaults to `'thinking_tags'` for compatibility reasons."""
Defaults to `'thinking_tags'` for backward compatibility reasons."""

ALL FIELDS MUST BE `openai_` PREFIXED SO YOU CAN MERGE THEM WITH OTHER MODELS.
"""

openai_chat_custom_reasoning_field: str | None = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's call it thinking as that's the word we use everywhere. "Custom" also seems unnecessary as all these profile settings are about non-standard/custom things.

Suggested change
openai_chat_custom_reasoning_field: str | None = None
openai_chat_thinking_field: str | None = None

If `openai_chat_send_back_thinking_parts` is set to `'custom_field'`, this field must be set to a non-None value."""

openai_chat_send_back_thinking_parts: Literal['thinking_tags', 'custom_field', False] = 'thinking_tags'
Copy link
Collaborator

Choose a reason for hiding this comment

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

With the openai_chat_thinking_field rename, maybe just tags and field and False?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Send back thinking parts in a field of request message

2 participants