fix: moonshot reasoning_content on tool_calls#2022
Conversation
Cache reasoning_content during streaming and fall back to an empty string when no cached value is available, so Moonshot doesn't reject multi-turn tool-call requests with "reasoning_content is missing". Co-Authored-By: Claude Opus 4.6 <[email protected]>
Map client-sent `reasoning` to `reasoning_content` when present, else fall back to an empty string. Removes the Redis round-trip that was caching reasoning by tool-call id — an empty string is enough to satisfy Moonshot's requirement. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughRemoves Redis-based caching/lookup of Moonshot Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant Redis
participant Moonshot
rect rgba(200,200,255,0.5)
Client->>Gateway: send message (includes reasoning?)
Gateway->>Redis: GET reasoning_content:<toolCallId>
Redis-->>Gateway: cached reasoning_content (or null)
alt cached
Gateway->>Moonshot: forward message with reasoning_content
else not cached
Gateway->>Moonshot: forward message without reasoning_content
Moonshot-->>Gateway: response with toolResults
Gateway->>Redis: SET reasoning_content:<toolCallId>
end
end
sequenceDiagram
participant Client
participant Gateway
participant Moonshot
rect rgba(200,255,200,0.5)
Client->>Gateway: send message (includes reasoning?)
Gateway->>Gateway: set reasoning_content = client.reasoning || ""
Gateway->>Moonshot: forward message with reasoning_content
Moonshot-->>Gateway: response with toolResults
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes multi-turn tool-call conversations for Moonshot “thinking” models by ensuring prior assistant tool-call messages include reasoning_content, and removes the previous Redis-based caching approach that attempted to restore it later.
Changes:
- Removed Redis caching of
reasoning_contentkeyed by tool-call ID fromparse-provider-response.ts. - Added an inline request-time transform for Moonshot in
chat.tsto populatereasoning_contentfrom client-echoedreasoning, falling back to""when absent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/gateway/src/chat/tools/parse-provider-response.ts | Removes Moonshot-specific Redis caching of reasoning_content during response parsing. |
| apps/gateway/src/chat/chat.ts | Adds Moonshot request preprocessing to ensure assistant tool-call messages include reasoning_content (mapped from reasoning or ""). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| message.role === "assistant" && | ||
| message.tool_calls && | ||
| Array.isArray(message.tool_calls) && | ||
| message.tool_calls.length > 0 && | ||
| !(message as any).reasoning_content // Only add if not already present | ||
| !(message as any).reasoning_content | ||
| ) { | ||
| // Get reasoning_content from the first tool call (all tool calls share the same reasoning) | ||
| const firstToolCall = message.tool_calls[0]; | ||
| if (firstToolCall?.id) { | ||
| try { | ||
| const cachedReasoningContent = await redisClient.get( | ||
| `reasoning_content:${firstToolCall.id}`, | ||
| ); | ||
| if (cachedReasoningContent) { | ||
| // Add reasoning_content to the message for Moonshot | ||
| (message as any).reasoning_content = cachedReasoningContent; | ||
| } | ||
| } catch { | ||
| // Silently fail - reasoning_content caching is optional | ||
| } | ||
| } | ||
| (message as any).reasoning_content = (message as any).reasoning ?? ""; | ||
| } |
There was a problem hiding this comment.
When message.reasoning is present, this copies it into reasoning_content but leaves the original reasoning field on the message. For Moonshot this means the same reasoning text can be sent twice in the upstream payload (once as reasoning, once as reasoning_content), increasing request size/token usage and risking context-limit issues. Consider deleting reasoning after copying (or only setting reasoning_content if reasoning exists and then removing it) so the upstream payload contains a single copy of the reasoning text.
| Array.isArray(message.tool_calls) && | ||
| message.tool_calls.length > 0 && | ||
| !(message as any).reasoning_content // Only add if not already present | ||
| !(message as any).reasoning_content |
There was a problem hiding this comment.
The guard !(message as any).reasoning_content treats an empty string as “missing”. Since this code can set reasoning_content to "" intentionally, subsequent passes will still enter the block and may overwrite an explicit empty reasoning_content (or redo work unnecessarily). Use an explicit undefined/null check (e.g., message.reasoning_content == null or typeof ... === "undefined") to mean “not present”.
| !(message as any).reasoning_content | |
| (message as any).reasoning_content == null |
| // Moonshot's thinking models reject assistant tool_call messages that lack | ||
| // reasoning_content. If the client echoes `reasoning` (OpenAI-style) we map | ||
| // it across; otherwise fall back to an empty string so multi-turn tool | ||
| // conversations don't 400. | ||
| if (usedProvider === "moonshot") { | ||
| const { redisClient } = await import("@llmgateway/cache"); | ||
| for (const message of messages) { | ||
| if ( | ||
| message.role === "assistant" && | ||
| message.tool_calls && | ||
| Array.isArray(message.tool_calls) && | ||
| message.tool_calls.length > 0 && | ||
| !(message as any).reasoning_content // Only add if not already present | ||
| !(message as any).reasoning_content | ||
| ) { | ||
| // Get reasoning_content from the first tool call (all tool calls share the same reasoning) | ||
| const firstToolCall = message.tool_calls[0]; | ||
| if (firstToolCall?.id) { | ||
| try { | ||
| const cachedReasoningContent = await redisClient.get( | ||
| `reasoning_content:${firstToolCall.id}`, | ||
| ); | ||
| if (cachedReasoningContent) { | ||
| // Add reasoning_content to the message for Moonshot | ||
| (message as any).reasoning_content = cachedReasoningContent; | ||
| } | ||
| } catch { | ||
| // Silently fail - reasoning_content caching is optional | ||
| } | ||
| } | ||
| (message as any).reasoning_content = (message as any).reasoning ?? ""; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This Moonshot-specific message enrichment is central to preventing multi-turn tool-call 400s, but there doesn’t appear to be automated coverage asserting the behavior (mapping reasoning -> reasoning_content, and falling back to "" only when absent, without overwriting an existing reasoning_content). Adding a focused unit/integration test around this transformation would help prevent regressions, especially since Moonshot may not be exercised in the existing e2e matrix when provider keys aren’t configured.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 3769-3771: The code is using "as any" on the message object when
normalizing Moonshot fields; define a narrow local type (e.g., interface
MoonshotMessage { reasoning?: string; reasoning_content?: string }) and then
cast the message to that type (message as MoonshotMessage) into a local const
(e.g., const msg = message as MoonshotMessage), and replace uses of (message as
any).reasoning and reasoning_content with msg.reasoning and
msg.reasoning_content so you keep strict typing while performing the same
normalization in the code paths that set reasoning_content from reasoning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 73116075-3aaf-4a1c-b629-8a361a6ccd61
📒 Files selected for processing (2)
apps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/parse-provider-response.ts
💤 Files with no reviewable changes (1)
- apps/gateway/src/chat/tools/parse-provider-response.ts
| !(message as any).reasoning_content | ||
| ) { | ||
| // Get reasoning_content from the first tool call (all tool calls share the same reasoning) | ||
| const firstToolCall = message.tool_calls[0]; | ||
| if (firstToolCall?.id) { | ||
| try { | ||
| const cachedReasoningContent = await redisClient.get( | ||
| `reasoning_content:${firstToolCall.id}`, | ||
| ); | ||
| if (cachedReasoningContent) { | ||
| // Add reasoning_content to the message for Moonshot | ||
| (message as any).reasoning_content = cachedReasoningContent; | ||
| } | ||
| } catch { | ||
| // Silently fail - reasoning_content caching is optional | ||
| } | ||
| } | ||
| (message as any).reasoning_content = (message as any).reasoning ?? ""; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether BaseMessage (or adjacent message types) already define reasoning/reasoning_content
# so we can avoid local casting entirely.
rg -n -C3 --type=ts '\b(type|interface)\s+BaseMessage\b|reasoning_content|reasoning'Repository: theopenco/llmgateway
Length of output: 50377
🏁 Script executed:
#!/bin/bash
# Verify the exact code at lines 3758-3771 in apps/gateway/src/chat/chat.ts
sed -n '3758,3771p' apps/gateway/src/chat/chat.ts | cat -nRepository: theopenco/llmgateway
Length of output: 760
Remove as any from Moonshot message normalization.
Lines 3769 and 3771 use as any, which bypasses type checks and violates the TypeScript rule in this repo. Use a narrow local type instead.
💡 Proposed fix
if (usedProvider === "moonshot") {
+ type MoonshotAssistantMessage = BaseMessage & {
+ tool_calls?: unknown[];
+ reasoning?: string | null;
+ reasoning_content?: string;
+ };
for (const message of messages) {
+ const moonshotMessage = message as MoonshotAssistantMessage;
if (
- message.role === "assistant" &&
- message.tool_calls &&
- Array.isArray(message.tool_calls) &&
- message.tool_calls.length > 0 &&
- !(message as any).reasoning_content
+ moonshotMessage.role === "assistant" &&
+ moonshotMessage.tool_calls &&
+ Array.isArray(moonshotMessage.tool_calls) &&
+ moonshotMessage.tool_calls.length > 0 &&
+ moonshotMessage.reasoning_content == null
) {
- (message as any).reasoning_content = (message as any).reasoning ?? "";
+ moonshotMessage.reasoning_content = moonshotMessage.reasoning ?? "";
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/chat/chat.ts` around lines 3769 - 3771, The code is using
"as any" on the message object when normalizing Moonshot fields; define a narrow
local type (e.g., interface MoonshotMessage { reasoning?: string;
reasoning_content?: string }) and then cast the message to that type (message as
MoonshotMessage) into a local const (e.g., const msg = message as
MoonshotMessage), and replace uses of (message as any).reasoning and
reasoning_content with msg.reasoning and msg.reasoning_content so you keep
strict typing while performing the same normalization in the code paths that set
reasoning_content from reasoning.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/gateway/src/chat/chat.ts (1)
3816-3818:⚠️ Potential issue | 🟡 MinorTreat only
null/undefinedas missing here.
!reasoning_contentalso matches the explicit fallback value"", so this block will keep treating already-normalized messages as missing and can overwrite a client-supplied empty string fromreasoning. While fixing that, the remainingas anycasts can be replaced with a narrow local type.💡 Proposed fix
if (usedProvider === "moonshot") { + type MoonshotAssistantMessage = BaseMessage & { + tool_calls?: unknown[]; + reasoning?: string | null; + reasoning_content?: string; + }; for (const message of messages) { + const moonshotMessage = message as MoonshotAssistantMessage; if ( - message.role === "assistant" && - message.tool_calls && - Array.isArray(message.tool_calls) && - message.tool_calls.length > 0 && - !(message as any).reasoning_content + moonshotMessage.role === "assistant" && + Array.isArray(moonshotMessage.tool_calls) && + moonshotMessage.tool_calls.length > 0 && + moonshotMessage.reasoning_content == null ) { - (message as any).reasoning_content = (message as any).reasoning ?? ""; + moonshotMessage.reasoning_content = moonshotMessage.reasoning ?? ""; } } }#!/bin/bash set -euo pipefail # Show the exact Moonshot normalization block under review. sed -n '3809,3819p' apps/gateway/src/chat/chat.ts # Inspect nearby message typings/usages to confirm a narrow local type is sufficient. rg -n -C2 --type=ts '\b(type|interface)\s+BaseMessage\b|\breasoning_content\b|\breasoning\b'As per coding guidelines,
**/*.{ts,tsx}: Never useanyoras anyin TypeScript unless absolutely necessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/chat.ts` around lines 3816 - 3818, The current check uses "!reasoning_content" and wide "as any" casts which treat an explicit empty string as missing and overwrite client values; change to a narrow local type (e.g., declare const m = message as { reasoning_content?: string | null; reasoning?: string | null }) and replace the condition with a null/undefined-only test (m.reasoning_content == null). Then only when that is true assign m.reasoning_content = m.reasoning ?? "" and remove the other "as any" casts so the code no longer overwrites an intentionally empty reasoning_content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 3816-3818: The current check uses "!reasoning_content" and wide
"as any" casts which treat an explicit empty string as missing and overwrite
client values; change to a narrow local type (e.g., declare const m = message as
{ reasoning_content?: string | null; reasoning?: string | null }) and replace
the condition with a null/undefined-only test (m.reasoning_content == null).
Then only when that is true assign m.reasoning_content = m.reasoning ?? "" and
remove the other "as any" casts so the code no longer overwrites an
intentionally empty reasoning_content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 306060a8-945e-42a8-9f23-f437a2401f95
📒 Files selected for processing (2)
apps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/parse-provider-response.ts
💤 Files with no reviewable changes (1)
- apps/gateway/src/chat/tools/parse-provider-response.ts
|
superseded by #2092 |
Summary
Moonshot's thinking models (e.g. kimi-k2.5) reject multi-turn tool-call requests when
reasoning_contentis missing on prior assistant messages. The previous fix used Redis to cache and restorereasoning_contentby tool-call ID, but this was fragile (cache misses, cross-instance lookups, streaming path never cached).Replaced with a simple inline transform: map the client-echoed
reasoningfield toreasoning_content, falling back to an empty string. Also removed the now-unused Redis caching inparse-provider-response.ts.Test plan
pnpm test:unit— 950 passed)pnpm build)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor