From b8fa9f0c44a557eca55aa2f0e1014e8c474e3ce3 Mon Sep 17 00:00:00 2001 From: Josh Gieringer Date: Thu, 12 Feb 2026 10:50:40 -0700 Subject: [PATCH 1/2] refactor default system prompt to be closer to LLM creation --- generate_conversations/runner.py | 4 +--- llm_clients/llm_interface.py | 7 ++++++- tests/integration/test_conversation_runner.py | 20 +++++++++++++------ tests/unit/llm_clients/test_azure_llm.py | 4 +++- tests/unit/llm_clients/test_claude_llm.py | 4 +++- tests/unit/llm_clients/test_gemini_llm.py | 4 +++- tests/unit/llm_clients/test_llm_interface.py | 17 ++++++++++------ tests/unit/llm_clients/test_ollama_llm.py | 16 +++++++-------- tests/unit/llm_clients/test_openai_llm.py | 4 +++- 9 files changed, 52 insertions(+), 28 deletions(-) diff --git a/generate_conversations/runner.py b/generate_conversations/runner.py index 27dd4ae3..90fa0282 100644 --- a/generate_conversations/runner.py +++ b/generate_conversations/runner.py @@ -101,9 +101,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 683b1904..e713303e 100644 --- a/llm_clients/llm_interface.py +++ b/llm_clients/llm_interface.py @@ -8,6 +8,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.""" @@ -32,7 +35,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._last_response_metadata: Dict[str, Any] = {} self.conversation_id = self.create_conversation_id() diff --git a/tests/integration/test_conversation_runner.py b/tests/integration/test_conversation_runner.py index 187b3650..254ab608 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 5e9e6aa7..ae67277e 100644 --- a/tests/unit/llm_clients/test_azure_llm.py +++ b/tests/unit/llm_clients/test_azure_llm.py @@ -318,7 +318,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" diff --git a/tests/unit/llm_clients/test_claude_llm.py b/tests/unit/llm_clients/test_claude_llm.py index 3b2cfa29..7c05cef1 100644 --- a/tests/unit/llm_clients/test_claude_llm.py +++ b/tests/unit/llm_clients/test_claude_llm.py @@ -172,7 +172,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" diff --git a/tests/unit/llm_clients/test_gemini_llm.py b/tests/unit/llm_clients/test_gemini_llm.py index a8a876b0..9b76f2be 100644 --- a/tests/unit/llm_clients/test_gemini_llm.py +++ b/tests/unit/llm_clients/test_gemini_llm.py @@ -169,7 +169,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" diff --git a/tests/unit/llm_clients/test_llm_interface.py b/tests/unit/llm_clients/test_llm_interface.py index e0f122c4..ab771af8 100644 --- a/tests/unit/llm_clients/test_llm_interface.py +++ b/tests/unit/llm_clients/test_llm_interface.py @@ -4,7 +4,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): @@ -37,11 +37,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.""" @@ -169,12 +169,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 e6626c33..b18a3935 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 @@ -361,7 +361,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) # Should handle None gracefully - message won't include current message part @@ -415,8 +415,8 @@ def test_set_system_prompt_updates_prompt(self, mock_ollama): 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") @@ -436,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( @@ -515,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) @@ -537,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..ef8b6854 100644 --- a/tests/unit/llm_clients/test_openai_llm.py +++ b/tests/unit/llm_clients/test_openai_llm.py @@ -179,7 +179,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" From e02290ed5ba174fddccc9ea3c6c780a0cee0a54b Mon Sep 17 00:00:00 2001 From: Josh Gieringer Date: Thu, 19 Feb 2026 11:48:30 -0700 Subject: [PATCH 2/2] test DEFAULT_SYSTEM_PROMPT --- tests/unit/llm_clients/test_azure_llm.py | 4 ++-- tests/unit/llm_clients/test_base_llm.py | 8 +++----- tests/unit/llm_clients/test_claude_llm.py | 7 +++---- tests/unit/llm_clients/test_endpoint_llm.py | 8 +++----- tests/unit/llm_clients/test_gemini_llm.py | 7 +++---- tests/unit/llm_clients/test_ollama_llm.py | 4 +--- tests/unit/llm_clients/test_openai_llm.py | 7 +++---- 7 files changed, 18 insertions(+), 27 deletions(-) diff --git a/tests/unit/llm_clients/test_azure_llm.py b/tests/unit/llm_clients/test_azure_llm.py index ac0a461d..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 ( @@ -486,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 9a7e8105..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 ( @@ -287,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 9b76f2be..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 ( @@ -312,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_ollama_llm.py b/tests/unit/llm_clients/test_ollama_llm.py index 33af1da4..839d4de3 100644 --- a/tests/unit/llm_clients/test_ollama_llm.py +++ b/tests/unit/llm_clients/test_ollama_llm.py @@ -411,7 +411,7 @@ 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 @@ -420,11 +420,9 @@ def test_set_system_prompt_updates_prompt(self, mock_ollama): # 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" diff --git a/tests/unit/llm_clients/test_openai_llm.py b/tests/unit/llm_clients/test_openai_llm.py index ef8b6854..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 @@ -323,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"