Skip to content

Commit 74d2418

Browse files
committed
fix: simplify snapshot cleaning to only update function result message
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
1 parent de3658b commit 74d2418

File tree

2 files changed

+96
-63
lines changed

2 files changed

+96
-63
lines changed

camel/agents/chat_agent.py

Lines changed: 15 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,6 @@ class _ToolOutputHistoryEntry:
169169
result_text: str
170170
record_uuids: List[str]
171171
record_timestamps: List[float]
172-
args: Optional[Dict[str, Any]] = None
173-
extra_content: Optional[Dict[str, Any]] = None
174172
cached: bool = False
175173

176174

@@ -1447,8 +1445,6 @@ def _register_tool_output_for_cache(
14471445
tool_call_id: str,
14481446
result_text: str,
14491447
records: List[MemoryRecord],
1450-
args: Optional[Dict[str, Any]] = None,
1451-
extra_content: Optional[Dict[str, Any]] = None,
14521448
) -> None:
14531449
if not records:
14541450
return
@@ -1459,8 +1455,6 @@ def _register_tool_output_for_cache(
14591455
result_text=result_text,
14601456
record_uuids=[str(record.uuid) for record in records],
14611457
record_timestamps=[record.timestamp for record in records],
1462-
args=args,
1463-
extra_content=extra_content,
14641458
)
14651459
self._tool_output_history.append(entry)
14661460
self._process_tool_output_cache()
@@ -1502,66 +1496,31 @@ def _clean_snapshot_in_memory(
15021496
if record["uuid"] not in entry.record_uuids
15031497
]
15041498

1505-
# Recreate both assistant and function messages
1506-
# For Gemini API, the tool call request needs to exist along with
1507-
# the tool call result.
1508-
assist_args = entry.args if entry.args is not None else {}
1509-
1510-
# Use timestamps from entry, ensuring proper ordering
1511-
if len(entry.record_timestamps) >= 2:
1512-
assist_timestamp = entry.record_timestamps[0]
1513-
func_timestamp = entry.record_timestamps[1]
1514-
elif entry.record_timestamps:
1515-
# If only one timestamp, use it for function and ensure
1516-
# assist comes before
1517-
func_timestamp = entry.record_timestamps[0]
1518-
assist_timestamp = func_timestamp - 1e-6
1519-
else:
1520-
# No timestamps, use current time with proper ordering
1521-
assist_timestamp = time.time_ns() / 1_000_000_000
1522-
func_timestamp = assist_timestamp + 1e-6
1523-
1524-
# Recreate assistant message (tool call request)
1525-
cleaned_assist_message = FunctionCallingMessage(
1526-
role_name=self.role_name,
1527-
role_type=self.role_type,
1528-
meta_dict={},
1529-
content="",
1530-
func_name=entry.tool_name,
1531-
args=assist_args,
1532-
tool_call_id=entry.tool_call_id,
1533-
extra_content=entry.extra_content,
1499+
# Recreate only the function result message with cleaned content.
1500+
# The assistant message with tool calls is already recorded
1501+
# separately by _record_assistant_tool_calls_from_requests and
1502+
# should not be modified here.
1503+
timestamp = (
1504+
entry.record_timestamps[0]
1505+
if entry.record_timestamps
1506+
else time.time_ns() / 1_000_000_000
15341507
)
1535-
1536-
# Recreate function message (tool result)
1537-
cleaned_func_message = FunctionCallingMessage(
1508+
cleaned_message = FunctionCallingMessage(
15381509
role_name=self.role_name,
15391510
role_type=self.role_type,
15401511
meta_dict={},
15411512
content="",
15421513
func_name=entry.tool_name,
15431514
result=cleaned_result,
15441515
tool_call_id=entry.tool_call_id,
1545-
extra_content=entry.extra_content,
1546-
)
1547-
1548-
# Create new records for both assistant and function messages
1549-
new_assist_record = MemoryRecord(
1550-
message=cleaned_assist_message,
1551-
role_at_backend=OpenAIBackendRole.ASSISTANT,
1552-
timestamp=assist_timestamp,
1553-
agent_id=self.agent_id,
15541516
)
1555-
new_func_record = MemoryRecord(
1556-
message=cleaned_func_message,
1517+
new_record = MemoryRecord(
1518+
message=cleaned_message,
15571519
role_at_backend=OpenAIBackendRole.FUNCTION,
1558-
timestamp=func_timestamp,
1520+
timestamp=timestamp,
15591521
agent_id=self.agent_id,
15601522
)
1561-
1562-
# Add both records back
1563-
updated_records.append(new_assist_record.to_dict())
1564-
updated_records.append(new_func_record.to_dict())
1523+
updated_records.append(new_record.to_dict())
15651524
updated_records.sort(key=lambda record: record["timestamp"])
15661525
storage.clear()
15671526
storage.save(updated_records)
@@ -1573,11 +1532,8 @@ def _clean_snapshot_in_memory(
15731532
)
15741533

15751534
entry.cached = True
1576-
entry.record_uuids = [
1577-
str(new_assist_record.uuid),
1578-
str(new_func_record.uuid),
1579-
]
1580-
entry.record_timestamps = [assist_timestamp, func_timestamp]
1535+
entry.record_uuids = [str(new_record.uuid)]
1536+
entry.record_timestamps = [timestamp]
15811537

15821538
def add_external_tool(
15831539
self, tool: Union[FunctionTool, Callable, Dict[str, Any]]
@@ -4128,17 +4084,13 @@ def _record_tool_calling(
41284084
)
41294085

41304086
# Register tool output for snapshot cleaning if enabled
4131-
# Include args and extra_content so both the assistant message
4132-
# (tool call) and function message (tool result) can be recreated
41334087
if self._enable_snapshot_clean and not mask_output and func_records:
41344088
serialized_result = self._serialize_tool_result(result_for_memory)
41354089
self._register_tool_output_for_cache(
41364090
func_name,
41374091
tool_call_id,
41384092
serialized_result,
41394093
cast(List[MemoryRecord], func_records),
4140-
args=args,
4141-
extra_content=extra_content,
41424094
)
41434095

41444096
if isinstance(result, ToolResult) and result.images:

test/agents/test_chat_agent.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,3 +1837,84 @@ class MathResult(BaseModel):
18371837
assert len(responses) > 1, "Should receive multiple streaming chunks"
18381838
assert responses[-1].msg.parsed.answer == 6
18391839
assert responses[-1].msg.parsed.explanation
1840+
1841+
1842+
def test_clean_snapshot_in_memory():
1843+
"""Test that snapshot content is properly cleaned in memory.
1844+
1845+
This tests the _clean_snapshot_in_memory functionality which removes
1846+
stale snapshot markers and references from tool output messages stored
1847+
in memory. The cleaning preserves the assistant message (tool call request)
1848+
and only updates the function result message.
1849+
"""
1850+
from unittest.mock import MagicMock, patch
1851+
1852+
from camel.agents.chat_agent import _ToolOutputHistoryEntry
1853+
from camel.messages import FunctionCallingMessage
1854+
1855+
# Create a mock model to avoid API calls
1856+
mock_model = MagicMock()
1857+
mock_model.model_type = ModelType.DEFAULT
1858+
mock_model.model_config_dict = {}
1859+
mock_model.token_counter = None
1860+
mock_model.model_platform_name = "openai"
1861+
1862+
with patch.object(ChatAgent, '_init_model', return_value=mock_model):
1863+
agent = ChatAgent(
1864+
system_message="Test agent",
1865+
model=mock_model,
1866+
)
1867+
1868+
# Manually enable snapshot cleaning
1869+
agent._enable_snapshot_clean = True
1870+
agent._tool_output_history = []
1871+
1872+
# Create a mock memory storage
1873+
mock_storage = MagicMock()
1874+
mock_chat_history_block = MagicMock()
1875+
mock_chat_history_block.storage = mock_storage
1876+
1877+
agent.memory._chat_history_block = mock_chat_history_block
1878+
1879+
# Create a test entry with snapshot markers
1880+
test_uuid = "test-uuid-123"
1881+
test_timestamp = 1234567890.0
1882+
entry = _ToolOutputHistoryEntry(
1883+
tool_name="test_tool",
1884+
tool_call_id="call_123",
1885+
result_text="- Item 1 [ref=abc]\n- Item 2 [ref=def]\n",
1886+
record_uuids=[test_uuid],
1887+
record_timestamps=[test_timestamp],
1888+
cached=False,
1889+
)
1890+
agent._tool_output_history.append(entry)
1891+
1892+
# Mock the storage to return the existing record
1893+
mock_storage.load.return_value = [
1894+
{
1895+
"uuid": test_uuid,
1896+
"timestamp": test_timestamp,
1897+
"message": {"content": "- Item 1 [ref=abc]\n- Item 2 [ref=def]\n"},
1898+
}
1899+
]
1900+
1901+
# Call the clean function
1902+
agent._clean_snapshot_in_memory(entry)
1903+
1904+
# Verify storage was updated
1905+
assert mock_storage.clear.called
1906+
assert mock_storage.save.called
1907+
1908+
# Get the saved records
1909+
saved_records = mock_storage.save.call_args[0][0]
1910+
1911+
# Should have one record (the cleaned function result)
1912+
assert len(saved_records) == 1
1913+
1914+
# The record should be a function message with cleaned content
1915+
saved_record = saved_records[0]
1916+
assert saved_record["role_at_backend"] == OpenAIBackendRole.FUNCTION.value
1917+
1918+
# Verify entry was marked as cached
1919+
assert entry.cached is True
1920+
assert len(entry.record_uuids) == 1 # Single function result record

0 commit comments

Comments
 (0)