Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| tool_calls_to_execute.append(tool_call_data) | ||
|
|
||
| # Record the assistant message with ALL tool calls | ||
| self._record_assistant_tool_calls_message(accumulated_tool_calls) |
There was a problem hiding this comment.
Ran this against 21928d4 in a clean container, python:3.12-slim, core deps only.
_execute_tools_sync_with_status_accumulator (and the async version at line 6094) call _record_assistant_tool_calls_message(accumulated_tool_calls) with no content arg, so the recorded assistant message always has empty content. The non-streaming path threads it explicitly (step(), response_content from output_messages[0].content), so a turn where the model says something before calling a tool keeps that text in memory there but not here.
Repro: mocked stream with a content delta ("Let me double that for you.") then an external tool call, through agent.step().
=== agent.chat_history (persisted memory) ===
assistant | content: '' | tool_calls: [{'id': 'call_mock_ext', ...}]
The caller-facing response does carry the content (the first yielded partial), it's only the persisted memory entry that drops it, so a later turn never sees what the model said before calling the tool.
Fix would be threading content_accumulator.get_full_content() into both call sites and passing it as content= to _record_assistant_tool_calls_message.
In streaming mode, a tool call whose function name is not in _internal_tools was never marked complete, so it was silently dropped: unknown tools produced no tool-result or error message in memory and the step yielded no responses, and external tool calls were never surfaced even though their schemas are sent to the model. - _accumulate_tool_calls no longer requires the function name to be a registered internal tool to mark the call complete; completeness is id, name and parseable arguments, so the existing unknown-tool branch of _execute_tool_from_stream_data (and its async twin) is reachable again and records a recoverable error tool result, like the non-streaming path. - Streamed calls for tools in _external_tool_schemas are collected as ToolCallRequest objects instead of being executed, recorded in the assistant message, returned via info['external_tool_requests'] in a final response, and the step ends so the caller can execute them, mirroring _step_impl.
The streaming tool-execution paths called _record_assistant_tool_calls_message with no content, so the assistant message persisted in memory for a turn with tool calls always had empty content: text streamed before the tool call survived in the caller-facing response but was lost in memory, while the non-streaming path keeps it by threading the response content into the recorded message. - _execute_tools_sync_with_status_accumulator now receives the content accumulator from the stream-processing loop, like its async sibling. - Both the sync and the async executors pass content_accumulator.get_full_content() as the content of the recorded assistant tool call message.
21928d4 to
4e220e8
Compare
|
Thanks @AmirF194 for the clean container verification — addressed in |
Related Issue
Closes #4366
Description
In streaming mode, a tool call whose function name is not in
_internal_toolswas silently dropped: the streaming step executed nothing, recorded nothing to memory, surfaced noexternal_tool_requests, and yielded 0 responses. This affected both hallucinated tool names and tools registered viaexternal_tools=/add_external_tool(), whose schemas are sent to the model (_get_full_tool_schemas,camel/agents/chat_agent.py:954) and which theChatAgentdocstring says should be returned directly to the caller instead of processed.Root cause:
_accumulate_tool_callsonly marked a streamed tool call complete when its function name was inself._internal_tools(pre-fixcamel/agents/chat_agent.py:4999-5003). A call for any other name was accumulated but never marked complete, so:complete=Trueentries and returned without executing or yielding anything;_execute_tool_from_stream_data(pre-fixcamel/agents/chat_agent.py:5202sync,5366async), which records a recoverableTool 'x' not found in registered toolstool result just like the non-streaming_execute_tool(camel/agents/chat_agent.py:4117-4123), was unreachable;"external_tool_requests": None.Changes (all in
camel/agents/chat_agent.py, sync and async paths kept in lockstep):_accumulate_tool_calls(chat_agent.py:4915): a tool call is now complete when it has id + name + JSON-parseable arguments, regardless of the name being a registered internal tool. Unknown names now flow into the existing unknown-tool branch of_execute_tool_from_stream_data(and its async twin), which records a recoverable error tool result and lets the model continue, matching the non-streaming path.chat_agent.py:5036,6080): complete calls whose name is in_external_tool_schemasare collected asToolCallRequestobjects instead of being executed. The sync executor now records the assistant message with all complete tool calls before its "nothing to execute" early return (the async executor already did), so an all-external turn still records the assistant tool-call message.chat_agent.py:4708,5865) thread the collected requests through and no longer emit a final content response when external tool calls were requested (that response would duplicate the one carrying the requests)._stream_response/_astream_response(chat_agent.py:4533,5596): when external tool calls were requested, yield a final response carryinginfo["external_tool_requests"]and end the step so the caller can execute them, mirroring_step_impl(camel/agents/chat_agent.py:3077-3118). Mixed turns (internal + external tools in one response) execute and record the internal tools, then surface the external requests and stop — same as non-streaming.What is the purpose of this pull request?
Checklist
pyproject.tomland runuv lock— no dependency changes in this PRTesting
Added 4 regression tests to
test/agents/test_chat_agent.py(sync + async, hallucinated name / external tool). They are fully offline: the model backend is mocked and only a dummyOPENAI_API_KEYis needed for the constructor-time check.New tests on unpatched master (
8c791b7b) — all 4 fail, reproducing the bug:Same tests on this branch:
Full module suite on this branch:
The 4 failures are pre-existing and environmental (they need a live OpenAI API key /
ANTHROPIC_API_KEY; re-running them on unpatched master8c791b7bfails identically:test_tool_calling_async,test_chat_agent_creation_methods,test_chat_agent_stream_with_structured_output,test_chat_agent_async_stream_with_structured_output). The 34 previously-passing tests still pass, plus the 4 new ones.Lint/type checks on the changed files, using the versions pinned in
.pre-commit-config.yaml:The reproduction script from the issue also now shows streaming matching non-streaming for every scenario: a hallucinated name records
Tool 'hallucinated_tool' not found in registered toolsas a tool result in memory and the model recovers on the next iteration; an external tool call is returned viaexternal_tool_requestswith the assistant tool-call message recorded and the step ended.