fix(llm): prevent text response from overriding native tool calls#4806
fix(llm): prevent text response from overriding native tool calls#4806mvanhorn wants to merge 3 commits intocrewAIInc:mainfrom
Conversation
When the LLM returns both text and tool_calls, the executor passes available_functions=None. The condition `not available_functions` was True, so the text-return path matched first and tool calls were silently discarded. Reorder the checks so tool_calls are returned before the text-return path when available_functions is None. Fixes crewAIInc#4788 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alvinttang
left a comment
There was a problem hiding this comment.
The ordering fix is correct: by checking "tool_calls and not available_functions" before the text-return path, native tool calls are no longer silently discarded when the LLM returns both a text response and tool calls in the same message. This is a subtle but important behavior difference — many LLMs (especially Claude) routinely include a preamble text alongside tool calls. One concern: the new early-return path returns tool_calls without emitting any call events via _handle_emit_call_events, whereas the old code path did emit them before returning text. Verify that callers handling the returned tool_calls list are responsible for their own event emission, otherwise observability/tracing could silently drop tool call spans.
…ents Address review feedback: - Reorder async _ahandle_non_streaming_response to check tool_calls before text-return path, matching the sync method fix - Emit TOOL_CALL events via _handle_emit_call_events when returning raw tool_calls in both sync and async paths
|
Addressed both issues in c01d4e3:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
After the early return for tool_calls-without-available_functions, the `not available_functions` sub-condition is dead code. Simplify to `not tool_calls and text_response` in both sync and async paths.
Summary
tool_calls, the executor passesavailable_functions=None. The existing condition(not tool_calls or not available_functions) and text_responseat step 5 matched becausenot available_functionswasTrue, so the text response was returned and tool calls were silently discarded.Test plan
available_functions=None-- tool calls should be returned, not textavailable_functionsis provided, tool execution still works normallyFixes #4788
🤖 Generated with Claude Code
Note
Medium Risk
Changes LLM non-streaming return-path precedence so responses with
tool_callsno longer fall back to text whenavailable_functionsisNone, which may alter what downstream callers receive in this edge case.Overview
Prevents native
tool_callsfrom being silently dropped when the LLM returns both text and tool calls butavailable_functionsis not provided.Reorders the non-streaming and async non-streaming response handling to return
tool_callsfirst (and emitLLMCallType.TOOL_CALLcompletion events) before the text-return path, while keeping existing behavior when there are no tool calls or when tool execution is available.Written by Cursor Bugbot for commit 3398561. This will update automatically on new commits. Configure here.