Fix/8267 mimo reasoning content#8327
Conversation
- 消息过滤时增加reasoning_content判断,保留仅含思考内容的assistant消息 - 自动为MiMo推理模型的assistant历史消息注入空reasoning_content,满足API要求 - 通过模型名称集合和xiaomimimo.com端点双重判断是否为MiMo推理模型 - 添加单元测试覆盖不同模型识别、字段注入、端点检测和已有内容保留等场景
- 回退通过xiaomimimo.com主机名自动识别MiMo推理模型的逻辑 - 仅保留基于模型名称集合的判断方式,避免误判非MiMo模型 - 删除对应主机名检测的单元测试用例
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
mimo_reasoning_modelsset is duplicated between the implementation and tests; consider defining a single shared constant (or using a pattern-based check) to avoid future drift when models are added or renamed. - The PR description mentions using both model-name and
xiaomimimo.comendpoint checks to detect MiMo reasoning models, but the current code only checks model names; either add the host-based check (similar tois_deepseek_reasoning) or update the description for consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `mimo_reasoning_models` set is duplicated between the implementation and tests; consider defining a single shared constant (or using a pattern-based check) to avoid future drift when models are added or renamed.
- The PR description mentions using both model-name and `xiaomimimo.com` endpoint checks to detect MiMo reasoning models, but the current code only checks model names; either add the host-based check (similar to `is_deepseek_reasoning`) or update the description for consistency.
## Individual Comments
### Comment 1
<location path="tests/test_openai_source.py" line_range="1808-1810" />
<code_context>
+]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("model", MIMO_REASONING_MODELS)
+async def test_mimo_reasoning_model_adds_empty_reasoning_content(model: str):
+ """MiMo 推理模型:assistant 消息缺少 reasoning_content 时自动补空字符串"""
+ provider = _make_provider()
</code_context>
<issue_to_address>
**suggestion (testing):** Extend tests to cover interaction with the assistant-message filtering logic when only reasoning_content is present
MiMo tests currently cover injection/preservation of `reasoning_content`, but not its interaction with the "empty assistant message" filter. Please add a test where an assistant message has `reasoning_content` set (possibly injected) and empty/omitted `content`, run it through the filtering flow, and assert that the message is not dropped. This will guard against regressions where MiMo-related messages are filtered out.
Suggested implementation:
```python
payloads = {
"model": model,
"messages": [
{"role": "user", "content": "hello"},
{
"role": "assistant",
"content": "I will help with that.",
},
# assistant 消息只包含 reasoning_content,content 为空,用于验证过滤逻辑不会将其丢弃
{
"role": "assistant",
"content": "",
"reasoning_content": "Thinking about how to help with that.",
},
],
```
To fully implement the requested coverage (asserting the message is not dropped by the "empty assistant message" filter), you should:
1. Locate where this test currently asserts against the serialized / filtered messages (for example, inspecting the outgoing request body captured by an HTTP mock, or the result of an internal normalization/filtering helper).
2. Extend those assertions to verify that the assistant message with `content == ""` and non-empty `reasoning_content` is still present after filtering, e.g.:
- There is an assistant message in the filtered list whose `reasoning_content` matches `"Thinking about how to help with that."`, and
- That message has been preserved (i.e., not removed by the empty-assistant-message filter), even though `content` is empty or omitted.
3. If other tests (e.g. something like `test_empty_assistant_messages_are_filtered_out`) already exercise the filter via a specific helper or by inspecting a particular mocked client call, mirror that pattern here so that this MiMo-specific test runs the messages through the exact same filtering flow before asserting on them.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize("model", MIMO_REASONING_MODELS) | ||
| async def test_mimo_reasoning_model_adds_empty_reasoning_content(model: str): |
There was a problem hiding this comment.
suggestion (testing): Extend tests to cover interaction with the assistant-message filtering logic when only reasoning_content is present
MiMo tests currently cover injection/preservation of reasoning_content, but not its interaction with the "empty assistant message" filter. Please add a test where an assistant message has reasoning_content set (possibly injected) and empty/omitted content, run it through the filtering flow, and assert that the message is not dropped. This will guard against regressions where MiMo-related messages are filtered out.
Suggested implementation:
payloads = {
"model": model,
"messages": [
{"role": "user", "content": "hello"},
{
"role": "assistant",
"content": "I will help with that.",
},
# assistant 消息只包含 reasoning_content,content 为空,用于验证过滤逻辑不会将其丢弃
{
"role": "assistant",
"content": "",
"reasoning_content": "Thinking about how to help with that.",
},
],To fully implement the requested coverage (asserting the message is not dropped by the "empty assistant message" filter), you should:
- Locate where this test currently asserts against the serialized / filtered messages (for example, inspecting the outgoing request body captured by an HTTP mock, or the result of an internal normalization/filtering helper).
- Extend those assertions to verify that the assistant message with
content == ""and non-emptyreasoning_contentis still present after filtering, e.g.:- There is an assistant message in the filtered list whose
reasoning_contentmatches"Thinking about how to help with that.", and - That message has been preserved (i.e., not removed by the empty-assistant-message filter), even though
contentis empty or omitted.
- There is an assistant message in the filtered list whose
- If other tests (e.g. something like
test_empty_assistant_messages_are_filtered_out) already exercise the filter via a specific helper or by inspecting a particular mocked client call, mirror that pattern here so that this MiMo-specific test runs the messages through the exact same filtering flow before asserting on them.
There was a problem hiding this comment.
Code Review
This pull request introduces support for MiMo reasoning models by ensuring that assistant messages include a reasoning_content field, which prevents 400 errors from the API. The changes include updated message filtering, payload conversion logic for specific MiMo models, and new unit tests. Feedback suggests refining the identification of MiMo models by also checking the endpoint host and optimizing performance by moving the model name set to a constant.
| "mimo-v2-omni", | ||
| "mimo-v2-flash", | ||
| } | ||
| is_mimo_reasoning = model in mimo_reasoning_models |
There was a problem hiding this comment.
根据 PR 描述,应该通过模型名称集合和 xiaomimimo.com 端点双重判断是否为 MiMo 推理模型。目前代码中缺少了对端点的判断。此外,根据项目规则,新功能的实现应配套相应的单元测试。
is_mimo_reasoning = (
model in mimo_reasoning_models
or "xiaomimimo.com" in self.client.base_url.host
)References
- New functionality, such as handling attachments, should be accompanied by corresponding unit tests.
| mimo_reasoning_models = { | ||
| "mimo-v2.5-pro", | ||
| "mimo-v2.5", | ||
| "mimo-v2-pro", | ||
| "mimo-v2-omni", | ||
| "mimo-v2-flash", | ||
| } |
- 添加test_mimo_filter_preserves_reasoning_only_assistant_message参数化测试 - 验证仅有reasoning_content的assistant消息不会被_sanitize过滤 - 确保包含reasoning_content的空content消息仍保留在对话历史中
Modifications / 改动点
close [Bug] mimo-v2.5适配问题 #8267
在
openai_source.py中参考deepseek的reasoning_content回传,增加了mimo的几个模型回传reasoning_content功能Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Handle MiMo reasoning models’ assistant messages to comply with API reasoning_content requirements and prevent unintended filtering.
Bug Fixes:
Enhancements: