From ffa386e3028a2c33d04530ab15b18b3b62bfef0e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 17 Mar 2025 04:55:07 +0000 Subject: [PATCH] Enhance set_knowledge implementation with better validation and documentation Co-Authored-By: Joe Moura --- src/crewai/agent.py | 33 ++++++++++++--- src/crewai/agents/agent_builder/base_agent.py | 42 +++++++++++++++---- tests/agent_test.py | 3 +- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/crewai/agent.py b/src/crewai/agent.py index 01a0e5f54..e41fbe421 100644 --- a/src/crewai/agent.py +++ b/src/crewai/agent.py @@ -153,33 +153,54 @@ class Agent(BaseAgent): Raises: ValueError: If the provided knowledge sources are invalid. + TypeError: If knowledge_sources is not a list or None. + ValueError: If embedder_config is missing required keys. + + Example: + ```python + from crewai.knowledge.source import StringKnowledgeSource + + content = "The capital of France is Paris." + source = StringKnowledgeSource(content=content) + + agent.set_knowledge( + knowledge_sources=[source], + embedder_config={"provider": "openai", "model": "text-embedding-3-small"} + ) + ``` """ try: # Handle backward compatibility with crew_embedder if embedder_config and self.embedder is None: self.embedder = embedder_config - # Set knowledge sources if provided - if knowledge_sources: + # Validate knowledge sources + if knowledge_sources is not None: if not isinstance(knowledge_sources, list): - raise ValueError("Knowledge sources must be a list") + raise TypeError("knowledge_sources must be a list or None") if not all(isinstance(k, BaseKnowledgeSource) for k in knowledge_sources): raise ValueError("All knowledge sources must be instances of BaseKnowledgeSource") self.knowledge_sources = knowledge_sources + # Create knowledge object if knowledge sources are provided if self.knowledge_sources: full_pattern = re.compile(r"[^a-zA-Z0-9\-_\r\n]|(\.\.)") - knowledge_agent_name = f"{re.sub(full_pattern, '_', self.role)}" + # Create a unique collection name based on agent role and id + knowledge_agent_name = f"{re.sub(full_pattern, '_', self.role)}_{id(self)}" self.knowledge = Knowledge( sources=self.knowledge_sources, embedder=self.embedder, collection_name=knowledge_agent_name, storage=self.knowledge_storage or None, ) - except (TypeError, ValueError) as e: - raise ValueError(f"Invalid Knowledge Configuration: {str(e)}") + except TypeError as e: + raise TypeError(f"Invalid Knowledge Configuration Type: {str(e)}") + except ValueError as e: + raise ValueError(f"Invalid Knowledge Configuration Value: {str(e)}") + except Exception as e: + raise ValueError(f"Error setting knowledge: {str(e)}") def execute_task( self, diff --git a/src/crewai/agents/agent_builder/base_agent.py b/src/crewai/agents/agent_builder/base_agent.py index 7a178ba6b..accb340e8 100644 --- a/src/crewai/agents/agent_builder/base_agent.py +++ b/src/crewai/agents/agent_builder/base_agent.py @@ -2,7 +2,7 @@ import uuid from abc import ABC, abstractmethod from copy import copy as shallow_copy from hashlib import md5 -from typing import Any, Dict, List, Optional, TypeVar +from typing import Any, Dict, List, Optional, TypeVar, Union, cast from pydantic import ( UUID4, @@ -385,27 +385,55 @@ class BaseAgent(ABC, BaseModel): Raises: ValueError: If the provided knowledge sources are invalid. + TypeError: If knowledge_sources is not a list or None. + ValueError: If embedder_config is missing required keys. + + Example: + ```python + from crewai.knowledge.source import StringKnowledgeSource + + content = "The capital of France is Paris." + source = StringKnowledgeSource(content=content) + + agent.set_knowledge( + knowledge_sources=[source], + embedder_config={"provider": "openai", "model": "text-embedding-3-small"} + ) + ``` """ try: # Validate knowledge sources first - if knowledge_sources: + if knowledge_sources is not None: if not isinstance(knowledge_sources, list): - raise ValueError("Knowledge sources must be a list") + raise TypeError("knowledge_sources must be a list or None") if not all(isinstance(k, BaseKnowledgeSource) for k in knowledge_sources): raise ValueError("All knowledge sources must be instances of BaseKnowledgeSource") self.knowledge_sources = knowledge_sources + + # Validate embedder configuration + if embedder_config is not None: + if not isinstance(embedder_config, dict): + raise TypeError("embedder_config must be a dictionary or None") + + if "provider" not in embedder_config: + raise ValueError("embedder_config must contain a 'provider' key") - if embedder_config: self.embedder_config = embedder_config + # Create knowledge object if knowledge sources are provided if self.knowledge_sources: - knowledge_agent_name = f"{self.role.replace(' ', '_')}" + # Create a unique collection name based on agent role and id + knowledge_agent_name = f"{self.role.replace(' ', '_')}_{id(self)}" self.knowledge = Knowledge( sources=self.knowledge_sources, embedder_config=self.embedder_config, collection_name=knowledge_agent_name, ) - except (TypeError, ValueError) as e: - raise ValueError(f"Invalid Knowledge Configuration: {str(e)}") + except TypeError as e: + raise TypeError(f"Invalid Knowledge Configuration Type: {str(e)}") + except ValueError as e: + raise ValueError(f"Invalid Knowledge Configuration Value: {str(e)}") + except Exception as e: + raise ValueError(f"Error setting knowledge: {str(e)}") diff --git a/tests/agent_test.py b/tests/agent_test.py index f5d2ecd69..84b2fbf62 100644 --- a/tests/agent_test.py +++ b/tests/agent_test.py @@ -1632,7 +1632,8 @@ def test_base_agent_set_knowledge(): assert agent.knowledge_sources == [string_source] assert agent.knowledge is not None assert MockKnowledge.called - assert MockKnowledge.call_args[1]["collection_name"] == "Test_Agent" + # Check that collection name starts with the agent role (now includes unique ID) + assert MockKnowledge.call_args[1]["collection_name"].startswith("Test_Agent_") # Test with embedder config embedder_config = {