-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(agents): preserve both assistant and function messages during snapshot clean for Gemini API #3819
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: master
Are you sure you want to change the base?
fix(agents): preserve both assistant and function messages during snapshot clean for Gemini API #3819
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
waleedalzarooni
left a comment
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.
Great job @Chase-Xuu,
Just a few comments on my side!
camel/agents/chat_agent.py
Outdated
| func_timestamp = assist_timestamp + 1e-6 | ||
|
|
||
| # Recreate assistant message (tool call request) | ||
| cleaned_assist_message = FunctionCallingMessage( |
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.
The recreated assistant message in _clean_snapshot_in_memory uses FunctionCallingMessage with args/func_name fields and an empty meta_dict, but the original message recorded _record_assistant_tool_calls_from_requests is a BaseMessage with tool calls in meta_dict["tool_calls"].
This format mismatch means the cleaned message serializes differently than the original, and doesn't support multi-tool-call responses, the original stores all tool calls from one model response in a single message, while the recreation produces one message per tool call.
| class _ToolOutputHistoryEntry: | ||
| tool_name: str | ||
| tool_call_id: str | ||
| result_text: str |
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.
Would be great if you could include some tests to verify the changes
…pshot clean for Gemini API Fixes camel-ai#3759 When `enable_snapshot_clean` is enabled, the `_clean_snapshot_in_memory` method previously only recreated the FUNCTION message (tool result) but removed both the ASSISTANT message (tool call request) and FUNCTION message from memory. This breaks Gemini API which requires both messages to exist together - the tool call request must be present alongside the tool call result. Without the ASSISTANT message, Gemini returns the error: `GenerateContentRequest.contents[0].parts[0].function_response.name: Name cannot be empty` Changes: - Added `args` and `extra_content` fields to `_ToolOutputHistoryEntry` to preserve tool call context for later reconstruction - Modified `_register_tool_output_for_cache` to accept and store args/extra_content - Modified `_clean_snapshot_in_memory` to recreate BOTH the ASSISTANT message (with tool_calls) and the FUNCTION message (with result) when cleaning snapshots - Updated `_record_tool_calling` to pass args and extra_content to the cache registration function This ensures proper tool call message reconstruction that is compatible with Gemini and other APIs that require paired tool call request/response messages.
Address reviewer feedback: - Remove unnecessary recreation of assistant message (already recorded separately by _record_assistant_tool_calls_from_requests) - Fixes format mismatch issue (FunctionCallingMessage vs BaseMessage) - Fixes multi-tool-call support (assistant message preserves all tool calls) - Add test for snapshot cleaning functionality
74d2418 to
3eef580
Compare
|
Thanks for the review feedback @waleedalzarooni! I've addressed both points in commit 3eef580:
Please let me know if you'd like any other changes! |
|
Hey @Chase-Xuu, thanks for the switch but I believe the current PR is now only implementing a refactor which won't solve the original problem. Would you mind having another quick look to ensure the original problem is fixed, I'll also have a look to see how to best implement this. |
Description
Fixes #3759
When
enable_snapshot_cleanis enabled, the_clean_snapshot_in_memorymethod previously only recreated the FUNCTION message (tool result) but removed both the ASSISTANT message (tool call request) and FUNCTION message from memory.This breaks Gemini API which requires both messages to exist together - the tool call request must be present alongside the tool call result. Without the ASSISTANT message, Gemini returns the error:
GenerateContentRequest.contents[0].parts[0].function_response.name: Name cannot be emptyChanges
argsandextra_contentfields to_ToolOutputHistoryEntryto preserve tool call context for later reconstruction_register_tool_output_for_cacheto accept and store args/extra_content_clean_snapshot_in_memoryto recreate BOTH the ASSISTANT message (with tool_calls) and the FUNCTION message (with result) when cleaning snapshots_record_tool_callingto pass args and extra_content to the cache registration functionTesting
This fix ensures proper tool call message reconstruction that is compatible with Gemini and other APIs that require paired tool call request/response messages.
The fix is based on @szhengac's investigation and initial fix, properly integrated and refined for the current codebase.
Checklist