mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-01-27 09:08:14 +00:00
fix: handle empty documents list in KnowledgeStorage.save() and asave()
Fixes #4277 When KnowledgeStorage.save() or asave() is called with an empty documents list, the method now returns early instead of propagating a low-level ValueError from ChromaDB's upsert operation. This is a valid edge case that can occur in real-world workflows (e.g., after filtering, retrieval failures, or conditional logic), and should be handled gracefully as a no-op. Co-Authored-By: João <joao@crewai.com>
This commit is contained in:
@@ -99,6 +99,9 @@ class KnowledgeStorage(BaseKnowledgeStorage):
|
|||||||
)
|
)
|
||||||
|
|
||||||
def save(self, documents: list[str]) -> None:
|
def save(self, documents: list[str]) -> None:
|
||||||
|
if not documents:
|
||||||
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
client = self._get_client()
|
client = self._get_client()
|
||||||
collection_name = (
|
collection_name = (
|
||||||
@@ -177,6 +180,9 @@ class KnowledgeStorage(BaseKnowledgeStorage):
|
|||||||
Args:
|
Args:
|
||||||
documents: List of document strings to save.
|
documents: List of document strings to save.
|
||||||
"""
|
"""
|
||||||
|
if not documents:
|
||||||
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
client = self._get_client()
|
client = self._get_client()
|
||||||
collection_name = (
|
collection_name = (
|
||||||
|
|||||||
@@ -193,3 +193,40 @@ def test_dimension_mismatch_error_handling(mock_get_client: MagicMock) -> None:
|
|||||||
|
|
||||||
with pytest.raises(ValueError, match="Embedding dimension mismatch"):
|
with pytest.raises(ValueError, match="Embedding dimension mismatch"):
|
||||||
storage.save(["test document"])
|
storage.save(["test document"])
|
||||||
|
|
||||||
|
|
||||||
|
@patch("crewai.knowledge.storage.knowledge_storage.get_rag_client")
|
||||||
|
def test_save_empty_documents_list(mock_get_client: MagicMock) -> None:
|
||||||
|
"""Test that save() handles empty documents list gracefully.
|
||||||
|
|
||||||
|
Calling save() with an empty documents list should be a no-op and not
|
||||||
|
propagate low-level storage exceptions from ChromaDB.
|
||||||
|
"""
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_get_client.return_value = mock_client
|
||||||
|
|
||||||
|
storage = KnowledgeStorage(collection_name="empty_docs_test")
|
||||||
|
|
||||||
|
storage.save([])
|
||||||
|
|
||||||
|
mock_client.get_or_create_collection.assert_not_called()
|
||||||
|
mock_client.add_documents.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
@patch("crewai.knowledge.storage.knowledge_storage.get_rag_client")
|
||||||
|
async def test_asave_empty_documents_list(mock_get_client: MagicMock) -> None:
|
||||||
|
"""Test that asave() handles empty documents list gracefully.
|
||||||
|
|
||||||
|
Calling asave() with an empty documents list should be a no-op and not
|
||||||
|
propagate low-level storage exceptions from ChromaDB.
|
||||||
|
"""
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_get_client.return_value = mock_client
|
||||||
|
|
||||||
|
storage = KnowledgeStorage(collection_name="empty_docs_async_test")
|
||||||
|
|
||||||
|
await storage.asave([])
|
||||||
|
|
||||||
|
mock_client.aget_or_create_collection.assert_not_called()
|
||||||
|
mock_client.aadd_documents.assert_not_called()
|
||||||
|
|||||||
Reference in New Issue
Block a user