Skip to content

Fix Mem0 OSS #2604

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

Merged
merged 15 commits into from
Apr 28, 2025
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pandas = [
openpyxl = [
"openpyxl>=3.1.5",
]
mem0 = ["mem0ai>=0.1.29"]
mem0 = ["mem0ai>=0.1.90"]
docling = [
"docling>=2.12.0",
]
Expand Down
10 changes: 7 additions & 3 deletions src/crewai/memory/storage/mem0_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,17 @@ def save(self, value: Any, metadata: Dict[str, Any]) -> None:
}

if params:
self.memory.add(value, **params | {"output_format": "v1.1"})
if not hasattr(self.memory, "llm"):
params["output_format"] = "v1.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq, double checking..
If the memory does not support llm, you set output_format, right? But I noticed there’s another logic at line 121 in the search method over there, you’re doing: “if memory has llm, delete parameters”

That feels a bit confusing. Can you confirm if that’s expected behavior, or if we might need to align those two cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucasgomide, I think what's happening in here is a class comparision, whether the memory object belongs to Memory or Memory Client
Basically MemoryClient does not have any llm attribute to it, so the first check is to make sure that if self.memory is not a MemoryClient class object, we need to have a parameter called output_format. and in the second case check I believe they are checking whether it's a object of Memory class, and if yes then we do not need this parameter.

Bit confusing how mem0 is configured.
Still author can confirm more on this.

One suggestion from my side would be if I am right about class comparision, would be to that there are better way to check whether a object belongs to a particular class, like isinstance, I think better to use this for better code visibility. Lucas can surely add more context on this part.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vidit-Ostwal I also prefer class comparison, but even after changing it, the question remains… I’m asking because the test didn’t confirm that.
From reading this line, I expected that when running with Memory, the output_format shouldn’t be included in the add() parameters - but is not happing (on test at least)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dev-Khant can you help with that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lucasgomide,

output_format="v1.1" -> {"results": }
output_format="v1.0" -> []

We’ve now fully deprecated the output_format parameter from the OSS version, though it’s still in the process of being removed from the Platform. To maintain consistency in output format, I’ve added output_format="v1.1" on line 92.

On line 121, there's a check to determine if the call is targeting OSS. If so, it removes the output_format parameter since it's deprecated in OSS but still required for the Platform or Mem0 client.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dev-Khant that is the point..

It's a bit tricky because in your tests
both implementation are propagating output_format, but by reading your code it should be sent ONLY when the memory does not has llm.
This is the last touchable point that isnot clear to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lucasgomide Then can you please help with the test?

self.memory.add(value, **params)

def search(
self,
query: str,
limit: int = 3,
score_threshold: float = 0.35,
) -> List[Any]:
params = {"query": query, "limit": limit}
params = {"query": query, "limit": limit, "output_format": "v1.1"}
if user_id := self._get_user_id():
params["user_id"] = user_id

Expand All @@ -116,8 +118,10 @@ def search(

# Discard the filters for now since we create the filters
# automatically when the crew is created.
if hasattr(self.memory, "llm"):
del params["metadata"], params["output_format"]
results = self.memory.search(**params)
return [r for r in results if r["score"] >= score_threshold]
return [r for r in results["results"] if r["score"] >= score_threshold]

def _get_user_id(self) -> str:
return self._get_config().get("user_id", "")
Expand Down
3 changes: 1 addition & 2 deletions tests/memory/user_memory_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

from unittest.mock import MagicMock, patch

import pytest
Expand Down Expand Up @@ -65,4 +64,4 @@ def test_save_and_search(user_memory):
with patch.object(UserMemory, 'search', return_value=expected_result) as mock_search:
find = UserMemory.search("test value", score_threshold=0.01)[0]
mock_search.assert_called_once_with("test value", score_threshold=0.01)
assert find == expected_result[0]
assert find == expected_result[0]
97 changes: 94 additions & 3 deletions tests/storage/test_mem0_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
class MockCrew:
def __init__(self, memory_config):
self.memory_config = memory_config
self.agents = [MagicMock(role="Test Agent")]


@pytest.fixture
Expand Down Expand Up @@ -107,11 +108,13 @@ def mem0_storage_with_memory_client_using_config_from_crew(mock_mem0_memory_clie


@pytest.fixture
def mem0_storage_with_memory_client_using_explictly_config(mock_mem0_memory_client):
def mem0_storage_with_memory_client_using_explictly_config(mock_mem0_memory_client, mock_mem0_memory):
"""Fixture to create a Mem0Storage instance with mocked dependencies"""

# We need to patch the MemoryClient before it's instantiated
with patch.object(MemoryClient, "__new__", return_value=mock_mem0_memory_client):
# We need to patch both MemoryClient and Memory to prevent actual initialization
with patch.object(MemoryClient, "__new__", return_value=mock_mem0_memory_client), \
patch.object(Memory, "__new__", return_value=mock_mem0_memory):

crew = MockCrew(
memory_config={
"provider": "mem0",
Expand Down Expand Up @@ -155,3 +158,91 @@ def test_mem0_storage_with_explict_config(
mem0_storage_with_memory_client_using_explictly_config.memory_config
== expected_config
)


def test_save_method(mem0_storage_with_mocked_config):
"""Test save method for different memory types"""
mem0_storage, _, _ = mem0_storage_with_mocked_config
mem0_storage.memory.add = MagicMock()

# Test short_term memory type (already set in fixture)
test_value = "This is a test memory"
test_metadata = {"key": "value"}

mem0_storage.save(test_value, test_metadata)

mem0_storage.memory.add.assert_called_once_with(
test_value,
agent_id="Test_Agent",
infer=False,
metadata={"type": "short_term", "key": "value"},
output_format="v1.1"
)

# Reset mock and test long_term memory type
mem0_storage.memory.add.reset_mock()
mem0_storage.memory_type = "long_term"

mem0_storage.save(test_value, test_metadata)

mem0_storage.memory.add.assert_called_once_with(
test_value,
agent_id="Test_Agent",
infer=False,
metadata={"type": "long_term", "key": "value"},
output_format="v1.1"
)


def test_search_method(mem0_storage_with_mocked_config):
"""Test search method for different memory types"""
mem0_storage, _, _ = mem0_storage_with_mocked_config
mock_results = {"results": [{"score": 0.9, "content": "Result 1"}, {"score": 0.4, "content": "Result 2"}]}
mem0_storage.memory.search = MagicMock(return_value=mock_results)

# Test short_term memory type (already set in fixture)
results = mem0_storage.search("test query", limit=5, score_threshold=0.5)

mem0_storage.memory.search.assert_called_once_with(
query="test query",
limit=5,
agent_id="Test_Agent",
metadata={"type": "short_term"},
output_format="v1.1",
user_id="test_user"
)

# Only one result should remain after filtering by score_threshold
assert len(results) == 1
assert results[0]["content"] == "Result 1"

# Reset mock and test long_term memory type
mem0_storage.memory.search.reset_mock()
mem0_storage.memory_type = "long_term"

mem0_storage.search("test query", limit=3)

mem0_storage.memory.search.assert_called_once_with(
query="test query",
limit=3,
agent_id="Test_Agent",
metadata={"type": "long_term"},
output_format="v1.1",
user_id="test_user"
)

# Test with llm attribute (special case)
mem0_storage.memory.search.reset_mock()
mem0_storage.memory_type = "short_term"
# Add llm attribute to memory mock
mem0_storage.memory.llm = "fake_llm"

mem0_storage.search("test query")

# When llm attribute exists, metadata and output_format should not be passed
mem0_storage.memory.search.assert_called_once_with(
query="test query",
limit=3,
agent_id="Test_Agent",
user_id="test_user"
)
53 changes: 49 additions & 4 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading