Feature/deepseek thinking mode #4190
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Author
|
这个pr需要 多多测试，好像支持思考中调用工具的不止deepseek |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey，我这边发现了 6 个问题，并给出了一些整体性的反馈：
- 在
_query_deepseek_thinking和_query_stream_deepseek_thinking里，base_payload只保留了model/messages/tools，因此来自payloads的其他标准参数（例如 temperature、top_p 等）会被悄悄丢弃；建议在构建call_payload时，以原始的payloads（减去非默认参数）为基础，以保持现有行为。 DeepSeekThinkingManager.get_api_messages的定义中需要传入initial_messages参数，但在openai_source.py中是以get_api_messages(session_id)的形式调用，这会导致运行时报TypeError；可以考虑让initial_messages变为可选并提供默认值，或者修改调用处以显式传入初始消息列表。- 在流式思考路径中，循环内部出现错误时既会 yield 一个
is_error=True的LLMResponse，然后又会重新抛出异常；建议只选择一种机制（要么通过结构化错误响应，要么直接抛异常），以避免下游调用方既收到错误片段又遇到未预期异常这种令人困惑的情况。
给 AI Agent 的提示词
请根据下面这次代码评审中的评论逐条进行修改：
## 整体性评论
- 在 `_query_deepseek_thinking` 和 `_query_stream_deepseek_thinking` 里，`base_payload` 只保留了 `model/messages/tools`，因此来自 `payloads` 的其他标准参数（例如 temperature、top_p 等）会被悄悄丢弃；建议在构建 `call_payload` 时，以原始的 `payloads`（减去非默认参数）为基础，以保持现有行为。
- `DeepSeekThinkingManager.get_api_messages` 的定义中需要传入 `initial_messages` 参数，但在 `openai_source.py` 中是以 `get_api_messages(session_id)` 的形式调用，这会导致运行时报 `TypeError`；可以考虑让 `initial_messages` 变为可选并提供默认值，或者修改调用处以显式传入初始消息列表。
- 在流式思考路径中，循环内部出现错误时既会 yield 一个 `is_error=True` 的 `LLMResponse`，然后又会重新抛出异常；建议只选择一种机制（要么通过结构化错误响应，要么直接抛异常），以避免下游调用方既收到错误片段又遇到未预期异常这种令人困惑的情况。
## 逐条评论
### Comment 1
<location> `astrbot/core/provider/sources/openai_source.py:219` </location>
<code_context>
+ # 思考循环
+ while session.should_continue():
+ # 获取当前上下文消息
+ current_messages = self.thinking_manager.get_api_messages(session_id)
+ if not current_messages:
+ # 第一次调用，使用原始消息
</code_context>
<issue_to_address>
**issue (bug_risk):** DeepSeekThinkingManager.get_api_messages 在调用时缺少必需参数，运行时会抛异常。
`DeepSeekThinkingManager.get_api_messages` 的定义如下：
```python
def get_api_messages(self, session_id: str, initial_messages: list[dict]) -> list[dict]:
```
在 `_query_deepseek_thinking` 和 `_query_stream_deepseek_thinking` 中调用时缺少 `initial_messages`：
```python
current_messages = self.thinking_manager.get_api_messages(session_id)
```
这里应该传入原始的 `messages`：
```python
current_messages = self.thinking_manager.get_api_messages(session_id, messages)
```
两个调用点都需要修正。
</issue_to_address>
### Comment 2
<location> `astrbot/core/provider/sources/openai_source.py:189-190` </location>
<code_context>
+ omit_empty_parameter_field=omit_empty_param_field,
+ )
+
+ # 准备基础payload
+ base_payload = {
+ "model": model,
+ "messages": messages,
</code_context>
<issue_to_address>
**issue (bug_risk):** 在思考模式下，默认的模型参数（例如 temperature、max_tokens）没有进入 call_payload，导致被丢弃。
在 `_query_deepseek_thinking` 中，`base_payload` 构造方式如下：
```python
base_payload = {
"model": model,
"messages": messages,
"tools": tool_list if tool_list else None,
}
```
之后你从 `payloads` 中把非默认的 key 移到 `extra_body`，但 `call_payload` 始终是 `base_payload.copy()`，从未再与 `payloads` 进行合并。结果就是，出现在 `payloads` 中且属于 `self.default_params` 的标准参数（例如 `temperature`、`max_tokens`、`top_p`）在思考请求中会被忽略。
为了避免和非思考路径 `_query`（直接传入 `**payloads`）相比出现行为回退，可以：
- 在移动非默认参数之后，从 `payloads` 重建 `base_payload`，或者
- 在思考循环之前，把 `payloads` 中的默认参数显式复制进 `base_payload`。
`_query_stream_deepseek_thinking` 中也需要做同样的修正，以保持流式 / 非流式行为一致。
</issue_to_address>
### Comment 3
<location> `astrbot/core/provider/sources/openai_source.py:586-598` </location>
<code_context>
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
+ # 针对 deepseek 模型的特殊处理：思考中调用工具
+ if "deepseek" in model and self.provider_config.get(
+ "ds_thinking_tool_call", False
+ ):
+ # 为 DeepSeek 模型启用思考模式
+ if "thinking" not in extra_body:
+ extra_body["thinking"] = {"type": "enabled"}
+ logger.debug(
+ "DeepSeek model detected in stream with thinking tool call enabled"
+ )
</code_context>
<issue_to_address>
**suggestion:** 在通用的 stream 路径中，这段额外的 DeepSeek 思考逻辑看起来是死代码，因为前面已经有基于 thinking-manager 的分流。
由于 `_query_stream` 已经会把所有 `is_thinking_enabled(model, self.provider_config)` 的情况路由到 `_query_stream_deepseek_thinking` 并直接返回，对于任何 `ds_thinking_tool_call=True` 的 DeepSeek 模型，都不会再走到后面这个 `if "deepseek" in model and self.provider_config.get("ds_thinking_tool_call", False)` 分支。如果你确实想保留一条不同的路径（例如不走 manager 的 DeepSeek 思考，或者由不同的开关控制），就需要调整条件；否则可以移除这块逻辑，以简化控制流。
```suggestion
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
```
</issue_to_address>
### Comment 4
<location> `astrbot/core/provider/sources/openai_source.py:267-276` </location>
<code_context>
+
+ return final_response
+
+ except Exception as e:
+ logger.error(f"Error in thinking session {session_id}: {e}")
+ self.thinking_manager.cleanup_session(session_id)
+ raise
+
+ # 如果循环结束但没有得到答案，返回错误
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 在流式思考模式的错误处理里，既会 yield 一个错误的 LLMResponse，又会重新抛出异常，这可能会让调用方感到困惑。
在 `_query_stream_deepseek_thinking` 中，先 yield 一个 `is_error=True` 的 `LLMResponse`，然后再重新抛出异常，意味着调用方既会收到一个错误 chunk，又会遇到异步生成器抛出的异常。如果它们把 `is_error` 当作终止信号，这种行为可能导致重复处理或未处理的错误。
更清晰的做法是二选一：（a）只 yield 错误响应而不再抛异常，或者（b）只抛异常并让调用方负责格式化错误。建议和非流式错误行为保持一致，使 API 行为更可预测。
</issue_to_address>
### Comment 5
<location> `thinking_assistant_debug.py:12-17` </location>
<code_context>
+from openai import AsyncOpenAI
+
+# 配置
+API_KEY = "输入你的key" # 填入你的 DeepSeek API Key
+MODEL = "deepseek-chat"
+BASE_URL = "https://api.deepseek.com"
</code_context>
<issue_to_address>
**🚨 suggestion (security):** 即便是占位用的 API key，也不应写死在代码中；从环境变量 / 配置读取既更安全，也更方便本地使用。
即使是调试脚本，在代码中写死 key 也容易诱导他人提交真实秘钥，并且不利于在多环境之间切换使用。
更推荐从环境 / 配置中读取，例如：
```python
API_KEY = os.getenv("DEEPSEEK_API_KEY")
if not API_KEY:
print("❌ 请先设置环境变量 DEEPSEEK_API_KEY")
return
```
这样可以避免把秘钥放进仓库里，也更符合常见的部署实践。
```suggestion
from openai import AsyncOpenAI
# 配置
API_KEY = os.getenv("DEEPSEEK_API_KEY")
if not API_KEY:
raise RuntimeError("❌ 请先设置环境变量 DEEPSEEK_API_KEY 用于访问 DeepSeek 接口")
MODEL = "deepseek-chat"
BASE_URL = "https://api.deepseek.com"
```
</issue_to_address>
### Comment 6
<location> `astrbot/core/provider/sources/openai_source.py:112` </location>
<code_context>
raise Exception(f"获取模型列表失败：{e}")
async def _query(self, payloads: dict, tools: ToolSet | None) -> LLMResponse:
+ model = payloads.get("model", "").lower()
+
</code_context>
<issue_to_address>
**issue (complexity):** 建议抽取工具 / extra_body 构造、DeepSeek 会话 ID 生成、以及思考模式初始化的公共逻辑，减少重复，让 DeepSeek 相关路径更易理解。
在保持现有 DeepSeek 功能的前提下，可以通过抽出几个小的辅助函数，让思考和非思考路径都复用，从而移除大量重复代码和嵌套。
### 1. 去重工具列表和 extra_body 处理逻辑
以下逻辑在 `_query`、`_query_stream`、`_query_deepseek_thinking`、`_query_stream_deepseek_thinking` 中重复出现：
- 计算 `model = payloads["model"].lower()`
- 基于 `ToolSet` 构造工具 payload，对 Gemini 使用 `omit_empty_param_field`
- 使用 `self.default_params` 将 `payloads` 拆分为 `payloads` 和 `extra_body`
- 合并来自 `provider_config` 的 `custom_extra_body`
- DeepSeek 思考时设置 `extra_body["thinking"]`
可以将这些逻辑集中到两个辅助函数中，并在各处复用：
```python
def _build_tools_payload(
self, model: str, tools: ToolSet | None
) -> list | None:
if not tools:
return None
omit_empty_param_field = "gemini" in model.lower()
return tools.get_func_desc_openai_style(
omit_empty_parameter_field=omit_empty_param_field,
) or None
def _split_payload_and_extra_body(
self, payloads: dict, *, enable_thinking: bool = False
) -> tuple[dict, dict]:
base_payload = dict(payloads) # shallow copy
extra_body: dict = {}
# move non-default params into extra_body
for key in list(base_payload.keys()):
if key not in self.default_params:
extra_body[key] = base_payload.pop(key)
# merge custom_extra_body
custom_extra_body = self.provider_config.get("custom_extra_body", {})
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
if enable_thinking:
extra_body.setdefault("thinking", {"type": "enabled"})
return base_payload, extra_body
```
在 `_query` 中使用之后，代码会精简很多，并且 DeepSeek 的差异点会更加明显：
```python
async def _query(self, payloads: dict, tools: ToolSet | None) -> LLMResponse:
model = payloads.get("model", "").lower()
if self.thinking_manager.is_thinking_enabled(model, self.provider_config):
return await self._query_deepseek_thinking(payloads, tools)
tool_list = self._build_tools_payload(model, tools)
if tool_list:
payloads["tools"] = tool_list
payloads, extra_body = self._split_payload_and_extra_body(payloads)
completion = await self.client.chat.completions.create(
**payloads, stream=False, extra_body=extra_body
)
...
```
在 `_query_stream_deepseek_thinking` / `_query_deepseek_thinking` 中，则可以这样复用：
```python
tool_list = self._build_tools_payload(model, tools)
base_payload = {
"model": model,
"messages": messages,
}
if tool_list:
base_payload["tools"] = tool_list
base_payload, extra_body = self._split_payload_and_extra_body(
{**base_payload, **payloads}, enable_thinking=True
)
```
这样就可以去掉四个位置上重复的 `to_del` 循环和 custom_extra_body 合并代码。
### 2. 去重 session id 生成逻辑
两个思考方法都重复了相同的“取首条 user 消息 → SHA‑256”的逻辑，并且在函数内部 import hashlib。可以抽取成一个小的私有辅助方法，让主流程更聚焦于“编排”：
```python
import hashlib # 顶部统一导入
def _build_thinking_session_id(self, messages: list[dict]) -> str:
user_content = ""
for msg in messages:
if msg.get("role") == "user":
user_content = msg.get("content", "") or ""
break
return hashlib.sha256(user_content.encode("utf-8")).hexdigest()
```
这样两个方法就可以简化为：
```python
model = payloads.get("model", "")
messages = payloads.get("messages", []) or []
session_id = self._build_thinking_session_id(messages)
session = self.thinking_manager.start_new_session(session_id, messages)
```
（如果 `DeepSeekThinkingManager` 更希望拿到 `user_content` 而不是消息列表，可以同时传入或调整其签名。）
### 3. 抽取思考模式专用的初始化辅助函数
`_query_deepseek_thinking` 和 `_query_stream_deepseek_thinking` 共享的初始化逻辑包括：
- 计算 `model`、`messages`
- 生成 `session_id` 并启动 session
- 构建 `tool_list`
- 构建 `base_payload`
- 在启用 thinking 的前提下拆分 payload / extra_body
可以把这些样板步骤封装到一个辅助函数中，让两个主方法的主体更聚焦在循环以及流式 vs 非流式的差异上：
```python
def _prepare_deepseek_thinking(
self, payloads: dict, tools: ToolSet | None
) -> tuple[str, Any, dict, dict, list | None]:
model = payloads.get("model", "")
messages = payloads.get("messages", []) or []
session_id = self._build_thinking_session_id(messages)
session = self.thinking_manager.start_new_session(session_id, messages)
tool_list = self._build_tools_payload(model, tools)
base_payload = {
"model": model,
"messages": messages,
}
if tool_list:
base_payload["tools"] = tool_list
# 与原始 payloads 合并，保证调用方仍然可以覆盖默认值
merged = {**payloads, **base_payload}
merged, extra_body = self._split_payload_and_extra_body(
merged, enable_thinking=True
)
return session_id, session, merged, extra_body, tool_list
```
这样 `_query_deepseek_thinking` 就可以大幅缩减为：
```python
async def _query_deepseek_thinking(
self, payloads: dict, tools: ToolSet | None
) -> LLMResponse:
session_id, session, base_payload, extra_body, tool_list = (
self._prepare_deepseek_thinking(payloads, tools)
)
while session.should_continue():
current_messages = self.thinking_manager.get_api_messages(session_id) or base_payload["messages"]
call_payload = dict(base_payload, messages=current_messages)
try:
completion = await self.client.chat.completions.create(
**call_payload, stream=False, extra_body=extra_body
)
response_dict = completion.model_dump()
should_continue, tool_calls, final_answer = (
self.thinking_manager.process_response(session_id, response_dict)
)
...
except Exception:
...
```
`_query_stream_deepseek_thinking` 也可以复用同样的 `session_id, session, base_payload, extra_body, tool_list`，而无需再次展开所有初始化逻辑。
通过这三个辅助函数，可以在保持现有行为不变的情况下，让主方法更加简洁，并更明显地体现“思考编排”的主线逻辑，有助于后续维护者理解和演进 DeepSeek 的流程。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据反馈改进后续评审。
Original comment in English
Hey - I've found 6 issues, and left some high level feedback:
- In both
_query_deepseek_thinkingand_query_stream_deepseek_thinking,base_payloadonly carriesmodel/messages/tools, so any other standard parameters frompayloads(e.g. temperature, top_p, etc.) are silently dropped; consider buildingcall_payloadstarting from the originalpayloads(minus non-defaults) to preserve existing behavior. DeepSeekThinkingManager.get_api_messagesis defined to take aninitial_messagesparameter but is called asget_api_messages(session_id)inopenai_source.py, which will raise aTypeError; either makeinitial_messagesoptional with a default or update the call sites to pass the initial message list.- In the streaming thinking path, errors inside the loop both yield an
LLMResponsewithis_error=Trueand then re-raise the exception; consider choosing one mechanism (either structured error responses or exceptions) to avoid surprising consumers that may receive both an error chunk and an unexpected exception.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both `_query_deepseek_thinking` and `_query_stream_deepseek_thinking`, `base_payload` only carries `model/messages/tools`, so any other standard parameters from `payloads` (e.g. temperature, top_p, etc.) are silently dropped; consider building `call_payload` starting from the original `payloads` (minus non-defaults) to preserve existing behavior.
- `DeepSeekThinkingManager.get_api_messages` is defined to take an `initial_messages` parameter but is called as `get_api_messages(session_id)` in `openai_source.py`, which will raise a `TypeError`; either make `initial_messages` optional with a default or update the call sites to pass the initial message list.
- In the streaming thinking path, errors inside the loop both yield an `LLMResponse` with `is_error=True` and then re-raise the exception; consider choosing one mechanism (either structured error responses or exceptions) to avoid surprising consumers that may receive both an error chunk and an unexpected exception.
## Individual Comments
### Comment 1
<location> `astrbot/core/provider/sources/openai_source.py:219` </location>
<code_context>
+ # 思考循环
+ while session.should_continue():
+ # 获取当前上下文消息
+ current_messages = self.thinking_manager.get_api_messages(session_id)
+ if not current_messages:
+ # 第一次调用，使用原始消息
</code_context>
<issue_to_address>
**issue (bug_risk):** DeepSeekThinkingManager.get_api_messages is called with missing required argument, which will raise at runtime.
`DeepSeekThinkingManager.get_api_messages` is defined as:
```python
def get_api_messages(self, session_id: str, initial_messages: list[dict]) -> list[dict]:
```
In `_query_deepseek_thinking` and `_query_stream_deepseek_thinking` it’s called without `initial_messages`:
```python
current_messages = self.thinking_manager.get_api_messages(session_id)
```
You should pass the original `messages` instead:
```python
current_messages = self.thinking_manager.get_api_messages(session_id, messages)
```
in both call sites.
</issue_to_address>
### Comment 2
<location> `astrbot/core/provider/sources/openai_source.py:189-190` </location>
<code_context>
+ omit_empty_parameter_field=omit_empty_param_field,
+ )
+
+ # 准备基础payload
+ base_payload = {
+ "model": model,
+ "messages": messages,
</code_context>
<issue_to_address>
**issue (bug_risk):** Default model parameters (e.g. temperature, max_tokens) are dropped in thinking mode because they never make it into call_payload.
In `_query_deepseek_thinking`, `base_payload` is built as:
```python
base_payload = {
"model": model,
"messages": messages,
"tools": tool_list if tool_list else None,
}
```
Then you strip non-default keys from `payloads` into `extra_body`, but `call_payload` is always `base_payload.copy()` and never re-merged with `payloads`. As a result, standard params present in `payloads` and `self.default_params` (e.g. `temperature`, `max_tokens`, `top_p`) are ignored for thinking requests.
To avoid this regression vs. the non-thinking `_query` path (which passes `**payloads`), either:
- Rebuild `base_payload` from `payloads` after moving non-default keys, or
- Explicitly copy default params from `payloads` into `base_payload` before the thinking loop.
The same fix is needed in `_query_stream_deepseek_thinking` to keep streaming/non-streaming behavior consistent.
</issue_to_address>
### Comment 3
<location> `astrbot/core/provider/sources/openai_source.py:586-598` </location>
<code_context>
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
+ # 针对 deepseek 模型的特殊处理：思考中调用工具
+ if "deepseek" in model and self.provider_config.get(
+ "ds_thinking_tool_call", False
+ ):
+ # 为 DeepSeek 模型启用思考模式
+ if "thinking" not in extra_body:
+ extra_body["thinking"] = {"type": "enabled"}
+ logger.debug(
+ "DeepSeek model detected in stream with thinking tool call enabled"
+ )
</code_context>
<issue_to_address>
**suggestion:** The extra DeepSeek-thinking block in the generic stream path appears to be dead code given the earlier thinking-manager routing.
Because `_query_stream` already routes all `is_thinking_enabled(model, self.provider_config)` cases through `_query_stream_deepseek_thinking` and returns immediately, any DeepSeek model with `ds_thinking_tool_call=True` will never reach this later `if "deepseek" in model and self.provider_config.get("ds_thinking_tool_call", False)` block. If you do want a distinct path (e.g., DeepSeek thinking without the manager or under a different flag), the conditions need to differ; otherwise this block can be removed to simplify control flow.
```suggestion
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
```
</issue_to_address>
### Comment 4
<location> `astrbot/core/provider/sources/openai_source.py:267-276` </location>
<code_context>
+
+ return final_response
+
+ except Exception as e:
+ logger.error(f"Error in thinking session {session_id}: {e}")
+ self.thinking_manager.cleanup_session(session_id)
+ raise
+
+ # 如果循环结束但没有得到答案，返回错误
</code_context>
<issue_to_address>
**suggestion (bug_risk):** In streaming-thinking error handling you both yield an error LLMResponse and then re-raise, which can surprise consumers.
In `_query_stream_deepseek_thinking`, the pattern of yielding an error `LLMResponse` and then re-raising means callers may see both an error chunk and an exception from the async generator. If they treat `is_error` as terminal, this can cause duplicated or unhandled error handling.
It would be clearer to either (a) yield the error response and not re-raise, or (b) just raise and let callers format errors. Align this with the non-streaming error behavior so the API is predictable.
</issue_to_address>
### Comment 5
<location> `thinking_assistant_debug.py:12-17` </location>
<code_context>
+from openai import AsyncOpenAI
+
+# 配置
+API_KEY = "输入你的key" # 填入你的 DeepSeek API Key
+MODEL = "deepseek-chat"
+BASE_URL = "https://api.deepseek.com"
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Hard-coded API key placeholder is better sourced from environment/config to reduce risk and ease local use.
Even for debug helpers, defining the key in code encourages committing real secrets and makes multi-environment use harder.
Prefer reading from env/config instead, e.g.:
```python
API_KEY = os.getenv("DEEPSEEK_API_KEY")
if not API_KEY:
print("❌ 请先设置环境变量 DEEPSEEK_API_KEY")
return
```
This keeps secrets out of the repo and matches typical deployment practices.
```suggestion
from openai import AsyncOpenAI
# 配置
API_KEY = os.getenv("DEEPSEEK_API_KEY")
if not API_KEY:
raise RuntimeError("❌ 请先设置环境变量 DEEPSEEK_API_KEY 用于访问 DeepSeek 接口")
MODEL = "deepseek-chat"
BASE_URL = "https://api.deepseek.com"
```
</issue_to_address>
### Comment 6
<location> `astrbot/core/provider/sources/openai_source.py:112` </location>
<code_context>
raise Exception(f"获取模型列表失败：{e}")
async def _query(self, payloads: dict, tools: ToolSet | None) -> LLMResponse:
+ model = payloads.get("model", "").lower()
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared helpers for tools/extra_body setup, DeepSeek session id creation, and thinking-specific initialization to remove duplication and make the DeepSeek paths easier to follow.
You can keep all the new DeepSeek functionality while cutting a lot of duplication and nesting by extracting a few small helpers that both thinking and non‑thinking paths use.
### 1. Deduplicate tool list & extra_body handling
The following logic appears in `_query`, `_query_stream`, `_query_deepseek_thinking`, `_query_stream_deepseek_thinking`:
- Compute `model = payloads["model"].lower()`
- Build tools payload from `ToolSet` with `omit_empty_param_field` for Gemini
- Split `payloads` into `payloads` vs `extra_body` using `self.default_params`
- Merge `custom_extra_body` from `provider_config`
- For DeepSeek thinking: set `extra_body["thinking"]`
You can centralize this into two helpers and reuse everywhere:
```python
def _build_tools_payload(
self, model: str, tools: ToolSet | None
) -> list | None:
if not tools:
return None
omit_empty_param_field = "gemini" in model.lower()
return tools.get_func_desc_openai_style(
omit_empty_parameter_field=omit_empty_param_field,
) or None
def _split_payload_and_extra_body(
self, payloads: dict, *, enable_thinking: bool = False
) -> tuple[dict, dict]:
base_payload = dict(payloads) # shallow copy
extra_body: dict = {}
# move non-default params into extra_body
for key in list(base_payload.keys()):
if key not in self.default_params:
extra_body[key] = base_payload.pop(key)
# merge custom_extra_body
custom_extra_body = self.provider_config.get("custom_extra_body", {})
if isinstance(custom_extra_body, dict):
extra_body.update(custom_extra_body)
if enable_thinking:
extra_body.setdefault("thinking", {"type": "enabled"})
return base_payload, extra_body
```
Usage in `_query` becomes very small and makes the DeepSeek difference obvious:
```python
async def _query(self, payloads: dict, tools: ToolSet | None) -> LLMResponse:
model = payloads.get("model", "").lower()
if self.thinking_manager.is_thinking_enabled(model, self.provider_config):
return await self._query_deepseek_thinking(payloads, tools)
tool_list = self._build_tools_payload(model, tools)
if tool_list:
payloads["tools"] = tool_list
payloads, extra_body = self._split_payload_and_extra_body(payloads)
completion = await self.client.chat.completions.create(
**payloads, stream=False, extra_body=extra_body
)
...
```
And in `_query_stream_deepseek_thinking` / `_query_deepseek_thinking` you can do:
```python
tool_list = self._build_tools_payload(model, tools)
base_payload = {
"model": model,
"messages": messages,
}
if tool_list:
base_payload["tools"] = tool_list
base_payload, extra_body = self._split_payload_and_extra_body(
{**base_payload, **payloads}, enable_thinking=True
)
```
That removes the repeated `to_del` loops and custom_extra_body merging from four places.
### 2. Deduplicate session id generation
Both thinking methods repeat the same “first user message → SHA‑256” logic and import hashlib inside the function. A tiny private helper keeps them focused on orchestration:
```python
import hashlib # top of file
def _build_thinking_session_id(self, messages: list[dict]) -> str:
user_content = ""
for msg in messages:
if msg.get("role") == "user":
user_content = msg.get("content", "") or ""
break
return hashlib.sha256(user_content.encode("utf-8")).hexdigest()
```
Then both methods reduce to:
```python
model = payloads.get("model", "")
messages = payloads.get("messages", []) or []
session_id = self._build_thinking_session_id(messages)
session = self.thinking_manager.start_new_session(session_id, messages)
```
(If `DeepSeekThinkingManager` prefers `user_content`, you can still pass both or adjust its signature.)
### 3. Extract thinking‑specific setup to a shared helper
`_query_deepseek_thinking` and `_query_stream_deepseek_thinking` share the same setup:
- compute `model`, `messages`
- build `session_id`, start session
- build `tool_list`
- build `base_payload`
- split payloads / extra_body with thinking enabled
You can hide that boilerplate in one helper and make both main methods mostly about the loop and streaming vs non‑stream behavior:
```python
def _prepare_deepseek_thinking(
self, payloads: dict, tools: ToolSet | None
) -> tuple[str, Any, dict, dict, list | None]:
model = payloads.get("model", "")
messages = payloads.get("messages", []) or []
session_id = self._build_thinking_session_id(messages)
session = self.thinking_manager.start_new_session(session_id, messages)
tool_list = self._build_tools_payload(model, tools)
base_payload = {
"model": model,
"messages": messages,
}
if tool_list:
base_payload["tools"] = tool_list
# merge with original payloads so caller can override defaults if needed
merged = {**payloads, **base_payload}
merged, extra_body = self._split_payload_and_extra_body(
merged, enable_thinking=True
)
return session_id, session, merged, extra_body, tool_list
```
Then `_query_deepseek_thinking` shrinks down:
```python
async def _query_deepseek_thinking(
self, payloads: dict, tools: ToolSet | None
) -> LLMResponse:
session_id, session, base_payload, extra_body, tool_list = (
self._prepare_deepseek_thinking(payloads, tools)
)
while session.should_continue():
current_messages = self.thinking_manager.get_api_messages(session_id) or base_payload["messages"]
call_payload = dict(base_payload, messages=current_messages)
try:
completion = await self.client.chat.completions.create(
**call_payload, stream=False, extra_body=extra_body
)
response_dict = completion.model_dump()
should_continue, tool_calls, final_answer = (
self.thinking_manager.process_response(session_id, response_dict)
)
...
except Exception:
...
```
And `_query_stream_deepseek_thinking` can reuse the same `session_id, session, base_payload, extra_body, tool_list` without re‑expanding all that logic.
These three helpers keep behavior identical but make the main methods much shorter and more obviously about “thinking orchestration” instead of request munging, which should help the next person reason about and evolve the DeepSeek flow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
未来打算补全deepseek的特殊格式功能 |
kawayiYokami
force-pushed
the
feature/deepseek-thinking-mode
branch
from
1de3644 to
3faec93
Compare
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些整体性的反馈：
- 在
_query和_query_stream中，用于准备消息并启用 thinking 模式的逻辑几乎完全相同；建议抽取成一个共享的辅助函数，以减少重复，并把未来可能的 DeepSeek 协议变更集中到一个地方。 - 在 DeepSeek 适配器中，当最后一条消息是
user时，之前所有 assistant 的reasoning_content字段都会被清空；如果历史推理内容在某些流程中仍然有用，建议只清理最后一条 assistant 消息，或者通过配置开关控制这类清理逻辑。
提供给 AI Agents 的提示词
Please address the comments from this code review:
## Overall Comments
- The logic to prepare messages and enable thinking mode in `_query` and `_query_stream` is nearly identical; consider extracting it into a shared helper to reduce duplication and keep future DeepSeek protocol changes in one place.
- In the DeepSeek adapter, when the last message is a `user`, all previous assistant `reasoning_content` fields are cleared; if older reasoning is useful for some flows, consider limiting this to only the last assistant message or gating the cleanup behind a config flag.
## Individual Comments
### Comment 1
<location> `astrbot/core/agent/runners/tool_loop_agent_runner.py:217-221` </location>
<code_context>
tool_calls_info=AssistantMessageSegment(
tool_calls=llm_resp.to_openai_to_calls_model(),
content=llm_resp.completion_text,
+ reasoning_content=llm_resp.reasoning_content or "",
),
tool_calls_result=tool_call_result_blocks,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider preserving `None` for missing reasoning content instead of coercing to an empty string.
`AssistantMessageSegment.reasoning_content` is `str | None`, but this coerces `None` to `""`, losing the distinction between "no reasoning" and "empty reasoning". Consider passing `llm_resp.reasoning_content` through unchanged (or leaving it as `None`) to preserve that semantic difference for downstream consumers.
```suggestion
tool_calls_info=AssistantMessageSegment(
tool_calls=llm_resp.to_openai_to_calls_model(),
content=llm_resp.completion_text,
reasoning_content=llm_resp.reasoning_content,
),
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/provider/sources/deepseek_source.py:49-58` </location>
<code_context>
+ async def _query_stream(
</code_context>
<issue_to_address>
**suggestion:** The reasoning setup logic is duplicated between `_query_stream` and `_query` and could be factored out.
Both `_query_stream` and `_query` repeat the same logic for enabling thinking mode, normalizing `messages`, and setting `payloads["thinking"]`. This duplication risks divergence if DeepSeek’s behavior changes. Consider extracting a small helper (e.g., `_prepare_thinking_payload(payloads: dict) -> dict`) to centralize this logic and simplify future maintenance.
Suggested implementation:
```python
# 其他模型根据配置决定
return self.provider_config.get("ds_thinking_tool_call", False)
def _prepare_thinking_payload(self, payloads: dict) -> dict:
"""
Centralize DeepSeek thinking-mode setup.
This helper:
- determines whether thinking mode should be enabled
- normalizes the messages list when thinking is enabled
"""
if self._should_enable_thinking(payloads):
# Ensure messages is always a list when thinking mode is enabled;
# any further normalization for reasoning should be added here.
payloads.setdefault("messages", [])
return payloads
async def _query_stream(
self,
payloads: dict,
tools: ToolSet | None,
):
payloads = self._prepare_thinking_payload(payloads)
```
To fully remove the duplication between `_query_stream` and `_query`:
1. In the `_query` method, replace the duplicated reasoning-setup logic (the call to `_should_enable_thinking`, the `messages = payloads.get("messages", [])` normalization, and any logic that sets `payloads["thinking"]`) with a single call:
- `payloads = self._prepare_thinking_payload(payloads)`
2. Move any additional reasoning-related payload normalization that is currently implemented directly in `_query` or `_query_stream` into `_prepare_thinking_payload` so that both methods share exactly the same behavior for:
- deciding whether thinking mode is enabled,
- normalizing `messages`,
- and initializing or updating `payloads["thinking"]`.
3. After these adjustments, `_query_stream` and `_query` should treat `payloads` as already prepared for DeepSeek thinking mode, avoiding further per-method reasoning-specific branches.帮我变得更有用！请对每条评论点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The logic to prepare messages and enable thinking mode in
_queryand_query_streamis nearly identical; consider extracting it into a shared helper to reduce duplication and keep future DeepSeek protocol changes in one place. - In the DeepSeek adapter, when the last message is a
user, all previous assistantreasoning_contentfields are cleared; if older reasoning is useful for some flows, consider limiting this to only the last assistant message or gating the cleanup behind a config flag.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic to prepare messages and enable thinking mode in `_query` and `_query_stream` is nearly identical; consider extracting it into a shared helper to reduce duplication and keep future DeepSeek protocol changes in one place.
- In the DeepSeek adapter, when the last message is a `user`, all previous assistant `reasoning_content` fields are cleared; if older reasoning is useful for some flows, consider limiting this to only the last assistant message or gating the cleanup behind a config flag.
## Individual Comments
### Comment 1
<location> `astrbot/core/agent/runners/tool_loop_agent_runner.py:217-221` </location>
<code_context>
tool_calls_info=AssistantMessageSegment(
tool_calls=llm_resp.to_openai_to_calls_model(),
content=llm_resp.completion_text,
+ reasoning_content=llm_resp.reasoning_content or "",
),
tool_calls_result=tool_call_result_blocks,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider preserving `None` for missing reasoning content instead of coercing to an empty string.
`AssistantMessageSegment.reasoning_content` is `str | None`, but this coerces `None` to `""`, losing the distinction between "no reasoning" and "empty reasoning". Consider passing `llm_resp.reasoning_content` through unchanged (or leaving it as `None`) to preserve that semantic difference for downstream consumers.
```suggestion
tool_calls_info=AssistantMessageSegment(
tool_calls=llm_resp.to_openai_to_calls_model(),
content=llm_resp.completion_text,
reasoning_content=llm_resp.reasoning_content,
),
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/provider/sources/deepseek_source.py:49-58` </location>
<code_context>
+ async def _query_stream(
</code_context>
<issue_to_address>
**suggestion:** The reasoning setup logic is duplicated between `_query_stream` and `_query` and could be factored out.
Both `_query_stream` and `_query` repeat the same logic for enabling thinking mode, normalizing `messages`, and setting `payloads["thinking"]`. This duplication risks divergence if DeepSeek’s behavior changes. Consider extracting a small helper (e.g., `_prepare_thinking_payload(payloads: dict) -> dict`) to centralize this logic and simplify future maintenance.
Suggested implementation:
```python
# 其他模型根据配置决定
return self.provider_config.get("ds_thinking_tool_call", False)
def _prepare_thinking_payload(self, payloads: dict) -> dict:
"""
Centralize DeepSeek thinking-mode setup.
This helper:
- determines whether thinking mode should be enabled
- normalizes the messages list when thinking is enabled
"""
if self._should_enable_thinking(payloads):
# Ensure messages is always a list when thinking mode is enabled;
# any further normalization for reasoning should be added here.
payloads.setdefault("messages", [])
return payloads
async def _query_stream(
self,
payloads: dict,
tools: ToolSet | None,
):
payloads = self._prepare_thinking_payload(payloads)
```
To fully remove the duplication between `_query_stream` and `_query`:
1. In the `_query` method, replace the duplicated reasoning-setup logic (the call to `_should_enable_thinking`, the `messages = payloads.get("messages", [])` normalization, and any logic that sets `payloads["thinking"]`) with a single call:
- `payloads = self._prepare_thinking_payload(payloads)`
2. Move any additional reasoning-related payload normalization that is currently implemented directly in `_query` or `_query_stream` into `_prepare_thinking_payload` so that both methods share exactly the same behavior for:
- deciding whether thinking mode is enabled,
- normalizing `messages`,
- and initializing or updating `payloads["thinking"]`.
3. After these adjustments, `_query_stream` and `_query` should treat `payloads` as already prepared for DeepSeek thinking mode, avoiding further per-method reasoning-specific branches.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Soulter
reviewed
Dec 26, 2025
astrbot/core/config/default.py
Outdated
| "provider_settings.agent_runner_type": "local", | ||
| }, | ||
| }, | ||
| "provider_settings.ds_thinking_tool_call": { |
Soulter
reviewed
Dec 26, 2025
astrbot/core/config/default.py
Outdated
| "gm_resp_image_modal": False, | ||
| "gm_native_search": False, | ||
| "gm_native_coderunner": False, | ||
| "ds_thinking_tool_call": False, |
dosubot
bot
added
the
size:L
Contributor
Author
|
@Soulter 其实压根不需要开关，就可以兼容chat 和reason，但是感觉不加个开关很多人不知道必须要用DEEPSEEK供应商跑DEEPSEEK才可以用deepseek的功能。 因为不清楚会不会有其他供应商有字段过敏所以是deepseek专用 你的看法是？ |
Member
据我所知，目前需要传递 reasoning_content 字段的模型有：
|
Member
|
暂时不需要做其他修改，思考了一下，可能得引入一个单独的 ThinkPart 来解决 thinking 的问题，这样也能解决 #4119 的问题。 |
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes: #4174
Modifications / 改动点
本次提交为 DeepSeek 提供商添加了专用适配器，支持思考模式（Thinking Mode）并实现智能化的思维链控制。
核心文件修改：
新增 DeepSeek 专用适配器 (
astrbot/core/provider/sources/deepseek_source.py)ProviderDeepSeek类，继承自ProviderOpenAIOfficial_should_enable_thinking()消息模型扩展 (
astrbot/core/agent/message.py)AssistantMessageSegment添加reasoning_content字段工具调用增强 (
astrbot/core/agent/runners/tool_loop_agent_runner.py)reasoning_content，确保多轮对话中推理内容正确传递配置扩展 (
astrbot/core/config/default.py)ds_thinking_tool_call配置项，控制思考中调用工具功能提供商注册 (
astrbot/core/provider/manager.py)deepseek_chat_completion提供商类型核心功能：
智能思维链控制：
deepseek-chat模型：强制关闭思维链deepseek-reasoner模型：强制开启思维链ds_thinking_tool_call配置决定完整的思考模式支持：
reasoning_content字段This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
（请在此处添加测试截图或日志）
Checklist / 检查清单
Summary by Sourcery
添加一个针对 DeepSeek 的特定聊天补全提供方，具备智能思维模式控制，并在消息和工具调用流程中传递推理内容。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Add a DeepSeek-specific chat completion provider with intelligent thinking-mode control and propagate reasoning content through messages and tool-calling flows.
New Features:
Enhancements: