diff --git a/generate_conversations/runner.py b/generate_conversations/runner.py index 014a4f5c..479a24d4 100644 --- a/generate_conversations/runner.py +++ b/generate_conversations/runner.py @@ -114,9 +114,7 @@ async def run_single_conversation( agent = LLMFactory.create_llm( model_name=self.agent_model_config["model"], name=self.agent_model_config.get("name", "Provider"), - system_prompt=self.agent_model_config.get( - "system_prompt", "You are a helpful AI assistant." - ), + system_prompt=self.agent_model_config.get("system_prompt"), role=Role.PROVIDER, **agent_kwargs, ) diff --git a/llm_clients/llm_interface.py b/llm_clients/llm_interface.py index 6fc94566..4873365f 100644 --- a/llm_clients/llm_interface.py +++ b/llm_clients/llm_interface.py @@ -9,6 +9,9 @@ T = TypeVar("T", bound=BaseModel) +# Default system prompt when none is provided (e.g. for provider/agent LLMs) +DEFAULT_SYSTEM_PROMPT = "You are a helpful AI assistant." + class Role(Enum): """Role of the LLM in a conversation.""" @@ -44,7 +47,9 @@ def __init__( ): self.name = name self.role = role - self.system_prompt = system_prompt or "" + self.system_prompt = ( + system_prompt if system_prompt is not None else DEFAULT_SYSTEM_PROMPT + ) self.first_message = first_message self.start_prompt = ( start_prompt if start_prompt is not None else DEFAULT_START_PROMPT diff --git a/tests/integration/test_conversation_runner.py b/tests/integration/test_conversation_runner.py index 073e0920..53f1fe41 100644 --- a/tests/integration/test_conversation_runner.py +++ b/tests/integration/test_conversation_runner.py @@ -16,7 +16,7 @@ import pytest from generate_conversations.runner import ConversationRunner -from llm_clients.llm_interface import Role +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT, Role from tests.mocks.mock_llm import MockLLM @@ -236,18 +236,22 @@ async def test_agent_system_prompt_default( basic_persona_config: Dict[str, Any], mock_llm_factory, ) -> None: - """When agent config has no system_prompt, create_llm gets the fallback.""" - default_prompt = "You are a helpful AI assistant." + """When agent config has no system_prompt, runner passes None to create_llm; + LLMInterface (MockLLM via super().__init__) applies DEFAULT_SYSTEM_PROMPT.""" agent_config = { "model": "mock-agent", "name": "test-agent", } - create_llm_calls = [] + create_llm_calls: list[dict] = [] + created_agents: list[MockLLM] = [] real_create = mock_llm_factory.side_effect def recording_create_llm(*args: Any, **kwargs: Any) -> MockLLM: create_llm_calls.append(copy.deepcopy(kwargs)) - return real_create(*args, **kwargs) + llm = real_create(*args, **kwargs) + if kwargs.get("role") == Role.PROVIDER: + created_agents.append(llm) + return llm mock_llm_factory.side_effect = recording_create_llm @@ -277,7 +281,11 @@ def recording_create_llm(*args: Any, **kwargs: Any) -> MockLLM: agent_calls = [c for c in create_llm_calls if c.get("role") == Role.PROVIDER] assert len(agent_calls) == 1 - assert agent_calls[0]["system_prompt"] == default_prompt + # Runner passes None when config has no system_prompt + assert agent_calls[0]["system_prompt"] is None + # LLMInterface applies default; created agent should have DEFAULT_SYSTEM_PROMPT + assert len(created_agents) == 1 + assert created_agents[0].system_prompt == DEFAULT_SYSTEM_PROMPT @pytest.mark.integration diff --git a/tests/unit/llm_clients/test_azure_llm.py b/tests/unit/llm_clients/test_azure_llm.py index 0c2ab55b..6291db03 100644 --- a/tests/unit/llm_clients/test_azure_llm.py +++ b/tests/unit/llm_clients/test_azure_llm.py @@ -8,6 +8,7 @@ from llm_clients import Role from llm_clients.azure_llm import AzureLLM +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT from .test_base_llm import TestJudgeLLMBase from .test_helpers import ( @@ -343,7 +344,9 @@ async def test_generate_response_without_system_prompt( mock_llm.ainvoke = AsyncMock(return_value=mock_response) mock_azure_model.return_value = mock_llm - llm = AzureLLM(name="TestAzure", role=Role.PERSONA) # No system prompt + llm = AzureLLM( + name="TestAzure", role=Role.PERSONA, system_prompt="" + ) # No system prompt response = await llm.generate_response(conversation_history=mock_system_message) assert response == "Response without system prompt" @@ -484,9 +487,8 @@ def test_set_system_prompt(self): role=Role.PERSONA, model_name="azure-gpt-5.2", name="TestAzure", - system_prompt="Initial prompt", ) - assert llm.system_prompt == "Initial prompt" + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT llm.set_system_prompt("Updated prompt") assert llm.system_prompt == "Updated prompt" diff --git a/tests/unit/llm_clients/test_base_llm.py b/tests/unit/llm_clients/test_base_llm.py index 31a77829..09ec798c 100644 --- a/tests/unit/llm_clients/test_base_llm.py +++ b/tests/unit/llm_clients/test_base_llm.py @@ -33,7 +33,7 @@ def get_mock_patches(self): from pydantic import BaseModel, Field from llm_clients import Role -from llm_clients.llm_interface import JudgeLLM, LLMInterface +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT, JudgeLLM, LLMInterface from .test_helpers import ( assert_error_metadata, @@ -125,11 +125,9 @@ def test_init_with_role_and_system_prompt(self): def test_set_system_prompt(self): """Test setting and updating system prompt.""" with self.get_mock_patches(): # pyright: ignore[reportGeneralTypeIssues] - llm = self.create_llm( - role=Role.PERSONA, name="TestLLM", system_prompt="Initial prompt" - ) + llm = self.create_llm(role=Role.PERSONA, name="TestLLM") - assert llm.system_prompt == "Initial prompt" + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT llm.set_system_prompt("Updated prompt") assert llm.system_prompt == "Updated prompt" diff --git a/tests/unit/llm_clients/test_claude_llm.py b/tests/unit/llm_clients/test_claude_llm.py index 152444e9..3d7c4dc4 100644 --- a/tests/unit/llm_clients/test_claude_llm.py +++ b/tests/unit/llm_clients/test_claude_llm.py @@ -7,6 +7,7 @@ from llm_clients import Role from llm_clients.claude_llm import ClaudeLLM +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT from .test_base_llm import TestJudgeLLMBase from .test_helpers import ( @@ -172,7 +173,9 @@ async def test_generate_response_without_system_prompt( mock_llm.ainvoke = AsyncMock(return_value=mock_response) mock_chat_anthropic.return_value = mock_llm - llm = ClaudeLLM(name="TestClaude", role=Role.PERSONA) # No system prompt + llm = ClaudeLLM( + name="TestClaude", role=Role.PERSONA, system_prompt="" + ) # No system prompt response = await llm.generate_response(conversation_history=mock_system_message) assert response == "Response without system prompt" @@ -285,10 +288,8 @@ def test_last_response_metadata_copy_returns_copy(self): def test_set_system_prompt(self): """Test set_system_prompt method.""" - llm = ClaudeLLM( - name="TestClaude", role=Role.PERSONA, system_prompt="Initial prompt" - ) - assert llm.system_prompt == "Initial prompt" + llm = ClaudeLLM(name="TestClaude", role=Role.PERSONA) + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT llm.set_system_prompt("Updated prompt") assert llm.system_prompt == "Updated prompt" diff --git a/tests/unit/llm_clients/test_endpoint_llm.py b/tests/unit/llm_clients/test_endpoint_llm.py index 7c149d2e..1a7362bd 100644 --- a/tests/unit/llm_clients/test_endpoint_llm.py +++ b/tests/unit/llm_clients/test_endpoint_llm.py @@ -7,7 +7,7 @@ from llm_clients import Role from llm_clients.endpoint_llm import EndpointLLM -from llm_clients.llm_interface import DEFAULT_START_PROMPT +from llm_clients.llm_interface import DEFAULT_START_PROMPT, DEFAULT_SYSTEM_PROMPT from .test_base_llm import TestLLMBase from .test_helpers import ( @@ -230,10 +230,8 @@ async def test_generate_response_none_history_delegates_to_start_conversation( def test_set_system_prompt(self): with self.get_mock_patches(): - llm = self.create_llm( - role=Role.PROVIDER, name="TestLLM", system_prompt="Initial" - ) - assert llm.system_prompt == "Initial" + llm = self.create_llm(role=Role.PROVIDER, name="TestLLM") + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT llm.set_system_prompt("Updated") assert llm.system_prompt == "Updated" diff --git a/tests/unit/llm_clients/test_gemini_llm.py b/tests/unit/llm_clients/test_gemini_llm.py index a8a876b0..63ac701f 100644 --- a/tests/unit/llm_clients/test_gemini_llm.py +++ b/tests/unit/llm_clients/test_gemini_llm.py @@ -7,6 +7,7 @@ from llm_clients import Role from llm_clients.gemini_llm import GeminiLLM +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT from .test_base_llm import TestJudgeLLMBase from .test_helpers import ( @@ -169,7 +170,9 @@ async def test_generate_response_without_system_prompt( mock_llm.ainvoke = AsyncMock(return_value=mock_response) mock_chat_gemini.return_value = mock_llm - llm = GeminiLLM(name="TestGemini", role=Role.PERSONA) # No system prompt + llm = GeminiLLM( + name="TestGemini", role=Role.PERSONA, system_prompt="" + ) # No system prompt response = await llm.generate_response(conversation_history=mock_system_message) assert response == "Response without system prompt" @@ -310,10 +313,8 @@ def test_last_response_metadata_copy_returns_copy(self): def test_set_system_prompt(self): """Test set_system_prompt method.""" - llm = GeminiLLM( - name="TestGemini", role=Role.PERSONA, system_prompt="Initial prompt" - ) - assert llm.system_prompt == "Initial prompt" + llm = GeminiLLM(name="TestGemini", role=Role.PERSONA) + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT llm.set_system_prompt("Updated prompt") assert llm.system_prompt == "Updated prompt" diff --git a/tests/unit/llm_clients/test_llm_interface.py b/tests/unit/llm_clients/test_llm_interface.py index f7d22752..9ebd69ea 100644 --- a/tests/unit/llm_clients/test_llm_interface.py +++ b/tests/unit/llm_clients/test_llm_interface.py @@ -5,7 +5,7 @@ import pytest from llm_clients import Role -from llm_clients.llm_interface import LLMInterface +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT, LLMInterface class ConcreteLLM(LLMInterface): @@ -42,11 +42,11 @@ class TestLLMInterface: """Unit tests for LLMInterface abstract base class.""" def test_init_with_name_only(self): - """Test initialization with only name parameter.""" + """Test initialization with only name parameter uses DEFAULT_SYSTEM_PROMPT.""" llm = ConcreteLLM(name="TestLLM", role=Role.PROVIDER) assert llm.name == "TestLLM" - assert llm.system_prompt == "" + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT def test_init_with_name_and_system_prompt(self): """Test initialization with name and system prompt.""" @@ -181,12 +181,17 @@ def test_getattr_with_callable_attribute(self): assert result == "method result" llm.llm.custom_method.assert_called_once() - def test_system_prompt_default_empty_string(self): - """Test that system_prompt defaults to empty string, not None.""" + def test_system_prompt_default_when_none(self): + """When system_prompt is None, base class uses DEFAULT_SYSTEM_PROMPT.""" llm = ConcreteLLM(name="TestLLM", role=Role.PROVIDER) - assert llm.system_prompt == "" + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT assert llm.system_prompt is not None + def test_system_prompt_explicit_empty_string(self): + """Explicit empty string is preserved, not replaced by default.""" + llm = ConcreteLLM(name="TestLLM", role=Role.PROVIDER, system_prompt="") + assert llm.system_prompt == "" + def test_getattr_preserves_attribute_type(self): """Test that __getattr__ preserves the type of delegated attributes.""" diff --git a/tests/unit/llm_clients/test_ollama_llm.py b/tests/unit/llm_clients/test_ollama_llm.py index ee063927..839d4de3 100644 --- a/tests/unit/llm_clients/test_ollama_llm.py +++ b/tests/unit/llm_clients/test_ollama_llm.py @@ -5,7 +5,7 @@ import pytest -from llm_clients.llm_interface import Role +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT, Role from .test_base_llm import TestLLMBase from .test_helpers import ( @@ -215,7 +215,7 @@ async def test_generate_response_without_system_prompt( mock_instance.ainvoke = AsyncMock(return_value="This is a test response") mock_ollama.return_value = mock_instance - llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER) + llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER, system_prompt="") response = await llm.generate_response(conversation_history=mock_system_message) # Verify message uses Human/Assistant format even without system prompt @@ -362,7 +362,7 @@ async def test_generate_response_with_none_message( mock_instance.ainvoke = AsyncMock(return_value="Default response") mock_ollama.return_value = mock_instance - llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER) + llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER, system_prompt="") response = await llm.generate_response(None) # None history: format uses default start_prompt message @@ -411,20 +411,18 @@ async def test_generate_response_preserves_multiline_messages(self, mock_ollama) assert "Line 1\nLine 2\nLine 3" in call_args @patch("llm_clients.ollama_llm.LangChainOllamaLLM") - def test_set_system_prompt_updates_prompt(self, mock_ollama): + def test_set_system_prompt(self, mock_ollama): """Test that set_system_prompt updates the system_prompt attribute.""" from llm_clients.ollama_llm import OllamaLLM llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER) - # Initially empty string (from LLMInterface base class) - assert llm.system_prompt == "" + # Initially default prompt (from LLMInterface when system_prompt is None) + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT - # Set prompt llm.set_system_prompt("You are a math tutor") assert llm.system_prompt == "You are a math tutor" - # Update prompt llm.set_system_prompt("You are a science tutor") assert llm.system_prompt == "You are a science tutor" @@ -438,7 +436,7 @@ async def test_set_system_prompt_affects_subsequent_calls(self, mock_ollama): mock_instance.ainvoke = AsyncMock(return_value="Response") mock_ollama.return_value = mock_instance - llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER) + llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER, system_prompt="") # First call without system prompt await llm.generate_response( @@ -517,7 +515,7 @@ async def test_generate_response_with_empty_conversation_history( mock_instance.ainvoke = AsyncMock(return_value="Response") mock_ollama.return_value = mock_instance - llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER) + llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER, system_prompt="") response = await llm.generate_response(conversation_history=mock_system_message) @@ -539,7 +537,7 @@ async def test_generate_response_with_none_conversation_history( mock_instance.ainvoke = AsyncMock(return_value="Response") mock_ollama.return_value = mock_instance - llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER) + llm = OllamaLLM(name="test-ollama", role=Role.PROVIDER, system_prompt="") response = await llm.generate_response(conversation_history=mock_system_message) diff --git a/tests/unit/llm_clients/test_openai_llm.py b/tests/unit/llm_clients/test_openai_llm.py index a69ab3cb..c6f14f8a 100644 --- a/tests/unit/llm_clients/test_openai_llm.py +++ b/tests/unit/llm_clients/test_openai_llm.py @@ -6,6 +6,7 @@ import pytest from llm_clients import Role +from llm_clients.llm_interface import DEFAULT_SYSTEM_PROMPT from llm_clients.openai_llm import OpenAILLM from .test_base_llm import TestJudgeLLMBase @@ -179,7 +180,9 @@ async def test_generate_response_without_system_prompt( mock_llm.ainvoke = AsyncMock(return_value=mock_response) mock_chat_openai.return_value = mock_llm - llm = OpenAILLM(name="TestOpenAI", role=Role.PERSONA) # No system prompt + llm = OpenAILLM( + name="TestOpenAI", role=Role.PERSONA, system_prompt="" + ) # No system prompt response = await llm.generate_response(conversation_history=mock_system_message) assert response == "Response without system prompt" @@ -321,10 +324,8 @@ def test_last_response_metadata_copy_returns_copy(self): def test_set_system_prompt(self): """Test set_system_prompt method.""" - llm = OpenAILLM( - name="TestOpenAI", role=Role.PERSONA, system_prompt="Initial prompt" - ) - assert llm.system_prompt == "Initial prompt" + llm = OpenAILLM(name="TestOpenAI", role=Role.PERSONA) + assert llm.system_prompt == DEFAULT_SYSTEM_PROMPT llm.set_system_prompt("Updated prompt") assert llm.system_prompt == "Updated prompt"