fix(anthropic): scope Kimi thinking guard to /coding endpoint only - #56730
liuhao1024 wants to merge 1 commit into
Conversation
Related: fixes #56727 (Kimi thinking wrongly suppressed on non- |
|
Follow-up from #56727: I re-tested the managed Kimi Coding endpoint with the essential omitted-parameter control case.
So omission does not disable reasoning; managed Kimi Coding enables it by default. I have corrected and closed #56727 because its original premise was not supported. This PR should therefore not claim to fix #56727 on that basis. Any separate non- |
The guard that suppressed Anthropic's thinking parameter for Kimi endpoints was based on a misunderstanding: when the parameter is omitted, Kimi enables extended thinking server-side by default (it does not disable thinking). Per reviewer @www654cc-pixel's live testing against api.kimi.com/coding: - thinking omitted → thinking + text blocks (server-side enabled) - thinking.enabled → thinking + text blocks (explicitly enabled) - thinking.disabled → text only (explicitly disabled) The original concern was that sending thinking.enabled would trigger a reasoning_content validation error, but the parameter is safe to send. Fixes NousResearch#56727, addresses reviewer feedback on NousResearch#56730.
4348e21 to
4166cc2
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for following up with the omitted-parameter control case. The current patch needs a different validation target before it can safely change this behavior.
Problems
- The patch removes the guard for all Kimi/Moonshot endpoints, while current main deliberately applies the family predicate at
agent/anthropic_adapter.py:2641. Maintainer commit83c288da01ebe48a64016d744abef166ef98d1fbexpanded that predicate to official and custom/proxied Kimi paths specifically for replayed tool-call compatibility. - The changed tests at
tests/agent/test_kimi_coding_anthropic_thinking.py:38and:140use only a fresh user message withtools=None. They do not test the replayed assistant tool-call path whose unsigned thinking blocks are preserved atagent/anthropic_adapter.py:2276-2279.
Suggested changes
- Validate the full multi-turn tool-call replay contract for each endpoint category before removing or narrowing the guard.
- Preserve the custom/proxy coverage unless that replay behavior is independently disproven.
Automated hermes-sweeper review.
| # 4.6 behavior and preserving the activity-feed UX during long tool runs. | ||
| _is_kimi_coding = _is_kimi_family_endpoint(base_url, model) | ||
| if reasoning_config and isinstance(reasoning_config, dict) and not _is_kimi_coding: | ||
| if reasoning_config and isinstance(reasoning_config, dict): |
There was a problem hiding this comment.
This makes thinking unconditional for all Kimi/Moonshot Anthropic-compatible endpoints. Current main intentionally uses _is_kimi_family_endpoint(base_url, model) here, and 83c288da01ebe48a64016d744abef166ef98d1fb broadened it to protect custom/proxied endpoints' replayed tool-call path. Please retain that coverage unless a multi-turn replay repro disproves it.
| assert "thinking" not in kwargs, ( | ||
| "Anthropic thinking must not be sent to Kimi /coding — " | ||
| "endpoint requires reasoning_content on history we don't preserve." | ||
| assert "thinking" in kwargs, ( |
There was a problem hiding this comment.
This only verifies kwargs for a fresh single-user-message request with tools=None. Please add coverage for an assistant tool-call followed by replay, which is the compatibility case guarded by current agent/anthropic_adapter.py:2276-2279.
|
Closing this one — two reasons, both about the PR as it stands rather than the underlying idea:
Also note the landscape shifted under this PR: #67391 (merged) changed replay behavior so Kimi-family endpoints keep thinking blocks in history, which removes part of the original motivation for the guard — that's exactly what #67228 builds on. If you believe the non- |
What does this PR do?
Narrows the Kimi thinking guard in
build_anthropic_kwargs()from_is_kimi_family_endpoint(matches ALL Kimi endpoints) to_is_kimi_coding_endpoint(matches only/coding). This restores thethinkingparameter for non-/codingKimi endpoints that were incorrectly blocked.Related Issue
Fixes #56727
Type of Change
Changes Made
agent/anthropic_adapter.py: Changed line 2627 from_is_kimi_family_endpoint(base_url, model)to_is_kimi_coding_endpoint(base_url), scoping the thinking guard to only the/codingendpoint. Updated comment to clarify the scope.tests/agent/test_kimi_coding_anthropic_thinking.py: Updated tests for non-/codingKimi endpoints to assert that thinking IS present (was incorrectly asserting omission). Renamed tests to reflect new behavior.How to Test
python -m pytest tests/agent/test_kimi_coding_anthropic_thinking.py -v— all 17 tests should pass/codingendpoints still omit thinking (4 parametrized tests pass)/codingKimi endpoints now receive thinking (7 parametrized tests pass)Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/agent/test_kimi_coding_anthropic_thinking.py -vand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A