Skip to content

fix: 兼容 Moonshot thinking 下 tool_choice required 冲突#7727

Open
Sisyphbaous-DT-Project wants to merge 5 commits intoAstrBotDevs:masterfrom
Sisyphbaous-DT-Project:master
Open

fix: 兼容 Moonshot thinking 下 tool_choice required 冲突#7727
Sisyphbaous-DT-Project wants to merge 5 commits intoAstrBotDevs:masterfrom
Sisyphbaous-DT-Project:master

Conversation

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor

@Sisyphbaous-DT-Project Sisyphbaous-DT-Project commented Apr 22, 2026

背景

Moonshot / Kimi 在开启 thinking 模式时,如果请求里带了 tool_choice="required",上游会直接返回 400
典型报错是 tool_choice 'required' is incompatible with thinking enabled
这个问题会让 skills_like 模式下的工具重查询直接中断

这次做了什么

  • 在 OpenAI 兼容 Provider 里识别这类报错
  • 命中后把 tool_choicerequired 降级为 auto,然后自动重试
  • 不改 runner 逻辑,尽量把影响范围控制在 Provider 这一层

后面补充的完善

  • 收紧了错误匹配规则,优先匹配已知的上游报错文本,避免误判
  • 降级重试时改用浅拷贝 payload,避免把原始 payloads 直接改掉
  • 补了日志,命中降级时能明确看到 tool_choice=required->auto,排查起来更直接

测试

补了几条针对性测试,主要覆盖这几种情况

  • 首次 required 失败后会自动切到 auto 并成功返回
  • 降级分支不会污染原始 payloads
  • 降级路径会输出日志

验证

已执行

  • uv run pytest tests/test_openai_source.py -k "required_thinking_conflict_uses_payload_copy or required_thinking_conflict_emits_fallback_log or incompatible_with_thinking" -q
  • uv run ruff check astrbot/core/provider/sources/openai_source.py tests/test_openai_source.py
  • uv run ruff format --check astrbot/core/provider/sources/openai_source.py tests/test_openai_source.py

结果通过

Summary by Sourcery

Handle Moonshot/Kimi thinking-mode incompatibility with tool_choice="required" by detecting specific upstream errors and retrying with tool_choice="auto" at the provider layer.

Bug Fixes:

  • Detect Moonshot/Kimi thinking-mode errors caused by tool_choice="required" and automatically downgrade the tool choice to "auto" and retry without altering the original payload.
  • Avoid mutating original payloads when applying tool_choice fallback logic for thinking-mode conflicts.
  • Log clear warnings when tool_choice is downgraded due to thinking-mode incompatibility to aid debugging.

Tests:

  • Add tests covering automatic retry with tool_choice downgraded from required to auto on thinking-mode conflicts, ensuring original payloads are preserved and fallback logging is emitted.

@auto-assign auto-assign Bot requested review from Fridemn and advent259141 April 22, 2026 08:03
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. labels Apr 22, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The detection logic in _is_tool_choice_required_incompatible_with_thinking_error is quite broad (just checking for the presence of several words), which risks false positives; consider first matching the known upstream message exactly and only falling back to the heuristic if you need to support variations.
  • When downgrading tool_choice from required to auto in _handle_api_error, you mutate payloads in place; if the original payloads is reused elsewhere in the call stack, it may be safer to work on a shallow copy for the retry to avoid unintended side effects.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The detection logic in `_is_tool_choice_required_incompatible_with_thinking_error` is quite broad (just checking for the presence of several words), which risks false positives; consider first matching the known upstream message exactly and only falling back to the heuristic if you need to support variations.
- When downgrading `tool_choice` from `required` to `auto` in `_handle_api_error`, you mutate `payloads` in place; if the original `payloads` is reused elsewhere in the call stack, it may be safer to work on a shallow copy for the retry to avoid unintended side effects.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to handle API errors when tool_choice='required' is incompatible with a provider's 'thinking' mode. It adds a detection method for this specific error and modifies the error handler to automatically downgrade tool_choice to auto and retry the request. A corresponding test case has been added to verify this behavior. I have no feedback to provide.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

收到,感谢你的及时反馈与补充。这些修改(收窄错误匹配条件、使用浅拷贝避免原地修改 payloads,以及新增的单元测试)非常完善,有效地解决了之前提到的潜在问题。PR 已更新至最新状态,我会进行最终确认。

@Sisyphbaous-DT-Project Sisyphbaous-DT-Project force-pushed the master branch 3 times, most recently from 3d4c5a6 to 51894c7 Compare April 22, 2026 09:07
@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In _handle_api_error, you call _extract_error_text_candidates(e) both inside _is_tool_choice_required_incompatible_with_thinking_error and again in the later required_tool_choice block; consider extracting candidates once and reusing them (possibly passing into the helper) to avoid duplication and keep the matching logic consistent.
  • The fallback logic for tool_choice=required is split between _is_tool_choice_required_incompatible_with_thinking_error and the subsequent has_related_keywords debug branch; it might be clearer to centralize or more explicitly separate these two paths so future readers understand why some errors trigger downgrade and others only log.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `_handle_api_error`, you call `_extract_error_text_candidates(e)` both inside `_is_tool_choice_required_incompatible_with_thinking_error` and again in the later `required_tool_choice` block; consider extracting candidates once and reusing them (possibly passing into the helper) to avoid duplication and keep the matching logic consistent.
- The fallback logic for `tool_choice=required` is split between `_is_tool_choice_required_incompatible_with_thinking_error` and the subsequent `has_related_keywords` debug branch; it might be clearer to centralize or more explicitly separate these two paths so future readers understand why some errors trigger downgrade and others only log.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to handle 'tool_choice=required' incompatibility with thinking mode in the OpenAI provider by automatically downgrading to 'auto' and retrying. It adds a specific error detection method and comprehensive unit tests. Feedback suggests optimizing the detection logic for performance and robustness, as well as streamlining the error handling flow to reduce redundant logging and duplicate computations.

Comment thread astrbot/core/provider/sources/openai_source.py
Comment thread astrbot/core/provider/sources/openai_source.py Outdated
Copy link
Copy Markdown
Contributor Author

@sourcery-ai 已根据这轮建议完成调整

  • _handle_api_error 中提取错误候选文本后复用,避免重复提取
  • 降级判定与 debug 判定共用同一份候选数据,逻辑更集中
  • 维持最小改动,不改变原有恢复语义

已补充并通过相关测试与格式检查

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements automatic error handling for cases where tool_choice='required' is incompatible with "thinking" mode in the OpenAI provider. A new detection method identifies this specific conflict in API error messages, allowing the system to automatically downgrade the tool_choice to auto and retry the request. The changes include comprehensive unit tests covering error detection, retry logic, and logging. I have no feedback to provide.

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

两个 bot 都没有提出新的可执行问题,当前实现看起来没有问题,可以继续推进合并

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

Labels

area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant