-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat: improve session loop for reasoning models #4933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
|
I meet the similar issue with official MiniMax API: #4112, the session stopped randomly after a thinking block or tool call. Seems fixed with this patch |
1b031a7 to
3f8383d
Compare
|
@no1wudi any reason why you closed this PR? it seems to work for me |
|
@hicdercpl I close it since I meet another issue that the session will stopped on parallel tool calls sometimes with this model, I'm not sure this patch is proper way to fix this issue. @rekram1-node Could you take a look? |
|
/review |
|
|
||
| if (lastAssistant?.info.role === "assistant" && lastAssistant.info.finish) { | ||
| const hasToolCalls = lastAssistant.info.finish === "tool-calls" | ||
| const hasTextAndToolCalls = hasTextContent && hasToolCalls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug/Redundancy: The condition lastAssistant?.info.role === "assistant" is redundant. The variable lastAssistant is already guaranteed to be an assistant message from the loop on lines 256-262 where it checks msg.info.role === "assistant".
You can simplify this to just check if (lastAssistant?.info.finish) since lastAssistant is either undefined or an assistant message.
|
So what's ur new exit condition exactly |
… tool execution - Add comprehensive exit logic handling reasoning blocks, text content, and tool calls - Prevent premature exit on reasoning-only messages (thinking/thoughts) - Properly handle combinations: reasoning+text, text+tools, reasoning+tools - Support both separated thinking (reasoning blocks) and interleaved thinking models - Check for pending or running tools before exiting session processing loop - Add hasRunningOrPendingTools() function with defensive error handling - Fix TypeScript type inference issues with MessageV2.User and MessageV2.WithParts - Add explicit type casting for proper property access and message comparison This makes session processing more reliable across different AI model types and response patterns, especially for reasoning-capable models. The fix ensures tools complete execution before considering the session done, regardless of execution pattern (sequential or parallel).
For this patch, I have made the following two modifications:
Regarding point 2, the issue I encountered is that the loop often exits prematurely when the model's thinking process and tool calls are interleaved. It seems like the Minimax API incorrectly returns a finish signal in the middle of this process? The Kimi thinking model exhibits similar behavior, but it doesn't exit the loop like this. |
|
@no1wudi what provider are u seeing it with is it specifically w/ minimax from minimax provider? And is it through the anthropic api endpoint or through openai "compat"? Or is it just using defaults for the provider from models.dev I wanna get this fixed but need to understand full scope of issue because i think there are issues w/ all interleavened thinking models but I thought if they sent it in anthropic format everything worked fine. |
@rekram1-node I use the default setting from models.dev with minimax-cn provider |

Improve the session prompt loop to properly support AI models that separate their reasoning process from final responses, with specific focus on interleaved thinking patterns where reasoning, tool calls, and text are mixed.
This enables proper handling of interleaved thinking models like MiniMax M2 that may output reasoning blocks interspersed with tool calls and text content, ensuring the loop continues until a complete response with actual text is available.