fix(agent): use full tool schema for DeepSeek V4#7862
fix(agent): use full tool schema for DeepSeek V4#7862imwyvern wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
dosubot
Bot
added
the
size:S
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The DeepSeek-specific handling in
_normalize_tool_schema_modehardcodes model names in the runner; consider centralizing model capability logic (e.g., in the provider or a capability map) so adding or changing DeepSeek variants doesn’t require touching agent runner internals. - The info-level log in
_normalize_tool_schema_modemay be noisy if many requests are made with DeepSeek V4 models; consider downgrading to debug or adding a one-time warning mechanism if this is expected to be a common code path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The DeepSeek-specific handling in `_normalize_tool_schema_mode` hardcodes model names in the runner; consider centralizing model capability logic (e.g., in the provider or a capability map) so adding or changing DeepSeek variants doesn’t require touching agent runner internals.
- The info-level log in `_normalize_tool_schema_mode` may be noisy if many requests are made with DeepSeek V4 models; consider downgrading to debug or adding a one-time warning mechanism if this is expected to be a common code path.
## Individual Comments
### Comment 1
<location path="tests/test_tool_loop_agent_runner.py" line_range="1286-1290" />
<code_context>
+ handler=AsyncMock(),
+ )
+ tool_set = ToolSet(tools=[tool])
+ req = ProviderRequest(
+ prompt="test",
+ func_tool=tool_set,
+ contexts=[],
+ model="deepseek-v4-flash",
+ )
+ runner = ToolLoopAgentRunner()
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test where the model name comes from the provider instead of the request.
This path supports models from both `request.model` and `provider.get_model()`, but this test only covers the former. Please also cover the case where `request.model` is `None` and `MockProvider.get_model()` returns a DeepSeek V4 name (either via a separate test or parametrization) to ensure normalization works in that scenario.
Suggested implementation:
```python
from unittest.mock import AsyncMock, Mock
```
```python
@pytest.mark.asyncio
async def test_deepseek_v4_uses_full_tool_schema_instead_of_skills_like():
provider = MockProvider()
tool = FunctionTool(
name="test_tool",
description="测试",
parameters={"type": "object", "properties": {"query": {"type": "string"}}},
handler=AsyncMock(),
)
tool_set = ToolSet(tools=[tool])
req = ProviderRequest(
prompt="test",
func_tool=tool_set,
contexts=[],
model="deepseek-v4-flash",
)
runner = ToolLoopAgentRunner()
await runner.reset(
provider=provider,
request=req,
run_context=ContextWrapper(context=None),
tool_executor=cast(Any, MockToolExecutor()),
agent_hooks=MockHooks(),
tool_schema_mode="skills_like",
)
@pytest.mark.asyncio
async def test_deepseek_v4_uses_full_tool_schema_when_model_from_provider():
provider = MockProvider()
# Ensure provider.get_model returns a DeepSeek V4 model name when request.model is None
provider.get_model = Mock(return_value="deepseek-v4-flash")
tool = FunctionTool(
name="test_tool",
description="测试",
parameters={"type": "object", "properties": {"query": {"type": "string"}}},
handler=AsyncMock(),
)
tool_set = ToolSet(tools=[tool])
req = ProviderRequest(
prompt="test",
func_tool=tool_set,
contexts=[],
model=None,
)
runner = ToolLoopAgentRunner()
await runner.reset(
provider=provider,
request=req,
run_context=ContextWrapper(context=None),
tool_executor=cast(Any, MockToolExecutor()),
agent_hooks=MockHooks(),
tool_schema_mode="skills_like",
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| req = ProviderRequest( | ||
| prompt="test", | ||
| func_tool=tool_set, | ||
| contexts=[], | ||
| model="deepseek-v4-flash", |
There was a problem hiding this comment.
suggestion (testing): Consider adding a test where the model name comes from the provider instead of the request.
This path supports models from both request.model and provider.get_model(), but this test only covers the former. Please also cover the case where request.model is None and MockProvider.get_model() returns a DeepSeek V4 name (either via a separate test or parametrization) to ensure normalization works in that scenario.
Suggested implementation:
from unittest.mock import AsyncMock, Mock@pytest.mark.asyncio
async def test_deepseek_v4_uses_full_tool_schema_instead_of_skills_like():
provider = MockProvider()
tool = FunctionTool(
name="test_tool",
description="测试",
parameters={"type": "object", "properties": {"query": {"type": "string"}}},
handler=AsyncMock(),
)
tool_set = ToolSet(tools=[tool])
req = ProviderRequest(
prompt="test",
func_tool=tool_set,
contexts=[],
model="deepseek-v4-flash",
)
runner = ToolLoopAgentRunner()
await runner.reset(
provider=provider,
request=req,
run_context=ContextWrapper(context=None),
tool_executor=cast(Any, MockToolExecutor()),
agent_hooks=MockHooks(),
tool_schema_mode="skills_like",
)
@pytest.mark.asyncio
async def test_deepseek_v4_uses_full_tool_schema_when_model_from_provider():
provider = MockProvider()
# Ensure provider.get_model returns a DeepSeek V4 model name when request.model is None
provider.get_model = Mock(return_value="deepseek-v4-flash")
tool = FunctionTool(
name="test_tool",
description="测试",
parameters={"type": "object", "properties": {"query": {"type": "string"}}},
handler=AsyncMock(),
)
tool_set = ToolSet(tools=[tool])
req = ProviderRequest(
prompt="test",
func_tool=tool_set,
contexts=[],
model=None,
)
runner = ToolLoopAgentRunner()
await runner.reset(
provider=provider,
request=req,
run_context=ContextWrapper(context=None),
tool_executor=cast(Any, MockToolExecutor()),
agent_hooks=MockHooks(),
tool_schema_mode="skills_like",
)
dosubot
Bot
added
the
area:provider
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a normalization step for tool schema modes in the ToolLoopAgentRunner, specifically forcing the 'full' schema for DeepSeek V4 models which do not support the 'skills-like' mode. A new test case has been added to verify this logic. The review feedback suggests using a prefix-based check for the model name to improve robustness against future model variants within the same family.
| if model_name not in {"deepseek-v4-flash", "deepseek-v4-pro"}: | ||
| return tool_schema_mode |
There was a problem hiding this comment.
Instead of hardcoding specific model variants, consider using a prefix check for deepseek-v4. This makes the logic more robust against future model releases within the same family (e.g., deepseek-v4-chat or simply deepseek-v4) that likely share the same tool schema limitations.
| if model_name not in {"deepseek-v4-flash", "deepseek-v4-pro"}: | |
| return tool_schema_mode | |
| if not model_name.startswith("deepseek-v4"): | |
| return tool_schema_mode |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2edcd3f61d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| model = (request.model or provider.get_model() or "").lower().strip() | ||
| model_name = model.rsplit("/", 1)[-1] | ||
| if model_name not in {"deepseek-v4-flash", "deepseek-v4-pro"}: |
There was a problem hiding this comment.
The override only triggers when model_name is exactly deepseek-v4-flash or deepseek-v4-pro, so DeepSeek V4 identifiers with valid suffixes/prefix variants (for example provider-qualified or tier-suffixed IDs like deepseek-v4-flash:free) will miss this branch and remain in skills_like mode. In that case the runner still sends light tool schemas and can hit the same function-calling rejection this fix is meant to prevent.
Useful? React with 👍 / 👎.
1 participant
Fixes #7855
DeepSeek V4 rejects the skills-like light tool schema, so AstrBot can fall back as if the model does not support function calling. For DeepSeek V4 models, the local agent runner now uses the full tool schema instead of mutating
func_toolto the light schema.Added a regression test that verifies
deepseek-v4-flashkeeps the original full tool set whenskills_likeis selected.Modifications / 改动点
Normalize
skills_liketofullfordeepseek-v4-flashanddeepseek-v4-pro.Keep existing
skills_likebehavior unchanged for other models.Add coverage for the DeepSeek V4 schema-mode override.
This is NOT a breaking change. / 这不是一个破坏性变更。
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
Normalize tool schema mode handling for DeepSeek V4 models to ensure compatible tool invocation while preserving behavior for other providers.
Enhancements:
skills_liketool schema mode, forcing DeepSeek V4 models to use the full tool schema with logging for this override.Tests:
skills_likeis requested and that internal state is updated accordingly.