diff --git a/src/pipecat/adapters/base_llm_adapter.py b/src/pipecat/adapters/base_llm_adapter.py index 95082a5d6..672004e7b 100644 --- a/src/pipecat/adapters/base_llm_adapter.py +++ b/src/pipecat/adapters/base_llm_adapter.py @@ -11,7 +11,7 @@ adapters that handle tool format conversion and standardization. """ from abc import ABC, abstractmethod -from typing import Any, Dict, Generic, List, TypeVar +from typing import Any, Dict, Generic, List, Optional, Tuple, TypeVar from loguru import logger @@ -39,10 +39,16 @@ class BaseLLMAdapter(ABC, Generic[TLLMInvocationParams]): - Converting standardized tools schema to provider-specific tool formats. - Extracting messages from the LLM context for the purposes of logging about the specific provider. + - Resolving conflicts between ``system_instruction`` and initial + system/developer messages in the conversation context. Subclasses must implement provider-specific conversion logic. """ + def __init__(self): + """Initialize the adapter.""" + self._warned_system_instruction = False + @property @abstractmethod def id_for_llm_specific_messages(self) -> str: @@ -129,4 +135,123 @@ class BaseLLMAdapter(ABC, Generic[TLLMInvocationParams]): # Fallback to return the same tools in case they are not in a standard format return tools - # TODO: we can move the logic to also handle the Messages here + def _extract_initial_system_or_developer( + self, + messages: list, + *, + system_instruction: Optional[str], + ) -> Tuple[Optional[str], Optional[str]]: + """Extract an initial system/developer message from messages, if appropriate. + + Checks ``messages[0]``. Behavior: + + - ``"system"`` role: always extract (pop from messages). + - ``"developer"`` role **without** ``system_instruction``: extract (pop). + - ``"developer"`` role **with** ``system_instruction``: don't extract; + convert to ``"user"`` in-place. + - Any other role: no-op. + + If extracting would leave the messages list empty (``len(messages) == 1``), + the message is converted to ``"user"`` role instead of being extracted. + This prevents sending an empty conversation history to providers that + require at least one non-system message (e.g. Anthropic, Bedrock). + + Args: + messages: Provider-formatted message list (mutated in-place). + system_instruction: The system instruction from service settings or + ``run_inference``, used to decide whether to extract a + ``"developer"`` message. + + Returns: + ``(extracted_content, original_role)`` where *original_role* is + ``"system"`` or ``"developer"``, or ``(None, None)`` if nothing + was extracted. + """ + if not messages: + return None, None + + role = messages[0].get("role") + if role not in ("system", "developer"): + return None, None + + # "developer" + system_instruction present → keep in messages as "user" + if role == "developer" and system_instruction: + messages[0]["role"] = "user" + return None, None + + # Would extracting empty the list? Convert to "user" instead. + if len(messages) == 1: + messages[0]["role"] = "user" + return None, None + + # Extract + content = messages[0].get("content", "") + if isinstance(content, list): + # Join text parts for providers that expect a string system instruction + content = " ".join( + part.get("text", "") for part in content if part.get("type") == "text" + ) + messages.pop(0) + return content, role + + def _resolve_system_instruction( + self, + initial_context_message: Optional[str], + initial_context_message_role: Optional[str], + system_instruction: Optional[str], + *, + discard_context_system: bool, + ) -> Optional[str]: + """Resolve conflict between ``system_instruction`` and an initial context message. + + Only warns when *initial_context_message_role* is ``"system"`` (not + ``"developer"``), since a developer message coexisting with + ``system_instruction`` is expected and handled elsewhere. + + Args: + initial_context_message: Content extracted from ``messages[0]`` + by :meth:`_extract_initial_system_or_developer`, or detected + inline (OpenAI adapters). + initial_context_message_role: ``"system"`` or ``"developer"`` — + the original role before extraction/detection. + system_instruction: From service settings or ``run_inference`` param. + discard_context_system: If ``True`` (non-OpenAI adapters), the + context system message is discarded when ``system_instruction`` + is also present. If ``False`` (OpenAI adapters), both are kept. + + Returns: + The effective system instruction to use, or ``None`` if the system + instruction is already represented in the messages (OpenAI path). + """ + both_present = initial_context_message and system_instruction + from_system_role = initial_context_message_role == "system" + + if both_present and from_system_role: + if not self._warned_system_instruction: + self._warned_system_instruction = True + if discard_context_system: + logger.warning( + "Both system_instruction and a system message in context are set." + " Using system_instruction." + ) + else: + logger.warning( + "Both system_instruction and an initial system message in context" + " are set, which may be unintended. Prefer system_instruction." + ) + + if system_instruction: + if discard_context_system: + return system_instruction + else: + # OpenAI path: caller prepends; return the instruction for prepending + return system_instruction + + if initial_context_message: + if discard_context_system: + return initial_context_message + else: + # Content is already in messages; nothing to prepend + return None + + return None diff --git a/src/pipecat/adapters/services/anthropic_adapter.py b/src/pipecat/adapters/services/anthropic_adapter.py index ecc87154c..dfd68375c 100644 --- a/src/pipecat/adapters/services/anthropic_adapter.py +++ b/src/pipecat/adapters/services/anthropic_adapter.py @@ -9,7 +9,7 @@ import copy import json from dataclasses import dataclass -from typing import Any, Dict, List, TypedDict +from typing import Any, Dict, List, Optional, TypedDict from anthropic import NOT_GIVEN, NotGiven from anthropic.types.message_param import MessageParam @@ -48,24 +48,37 @@ class AnthropicLLMAdapter(BaseLLMAdapter[AnthropicLLMInvocationParams]): return "anthropic" def get_llm_invocation_params( - self, context: LLMContext, enable_prompt_caching: bool + self, + context: LLMContext, + enable_prompt_caching: bool, + system_instruction: Optional[str] = None, ) -> AnthropicLLMInvocationParams: """Get Anthropic-specific LLM invocation parameters from a universal LLM context. Args: context: The LLM context containing messages, tools, etc. enable_prompt_caching: Whether prompt caching should be enabled. + system_instruction: Optional system instruction from service settings + or ``run_inference``. Returns: Dictionary of parameters for invoking Anthropic's LLM API. """ - messages = self._from_universal_context_messages(self.get_messages(context)) + converted = self._from_universal_context_messages( + self.get_messages(context), system_instruction=system_instruction + ) + system = self._resolve_system_instruction( + converted.system if converted.system is not NOT_GIVEN else None, + converted.system_role, + system_instruction, + discard_context_system=True, + ) return { - "system": messages.system, + "system": system if system is not None else NOT_GIVEN, "messages": ( - self._with_cache_control_markers(messages.messages) + self._with_cache_control_markers(converted.messages) if enable_prompt_caching - else messages.messages + else converted.messages ), # NOTE: LLMContext's tools are guaranteed to be a ToolsSchema (or NOT_GIVEN) "tools": self.from_standard_tools(context.tools) or [], @@ -105,35 +118,39 @@ class AnthropicLLMAdapter(BaseLLMAdapter[AnthropicLLMInvocationParams]): messages: List[MessageParam] system: str | NotGiven + system_role: Optional[str] = None # "system" or "developer" — origin of extracted system def _from_universal_context_messages( - self, universal_context_messages: List[LLMContextMessage] + self, + universal_context_messages: List[LLMContextMessage], + *, + system_instruction: Optional[str] = None, ) -> ConvertedMessages: system = NOT_GIVEN - messages = [] + system_role = None - # First, map messages using self._from_universal_context_message(m) + # Extract initial system/developer from universal messages BEFORE conversion, + # so the helper works with standard message format (not provider-specific). + remaining = list(universal_context_messages) + if remaining and not isinstance(remaining[0], LLMSpecificMessage): + extracted_content, extracted_role = self._extract_initial_system_or_developer( + remaining, system_instruction=system_instruction + ) + if extracted_content is not None: + system = extracted_content + system_role = extracted_role + + # Convert remaining messages to Anthropic format + messages = [] try: - messages = [self._from_universal_context_message(m) for m in universal_context_messages] + messages = [self._from_universal_context_message(m) for m in remaining] except Exception as e: logger.error(f"Error mapping messages: {e}") - # See if we should pull the system message out of our messages list. - if messages and messages[0]["role"] == "system": - if len(messages) == 1: - # If we have only have a system message in the list, all we can really do - # without introducing too much magic is change the role to "user". - messages[0]["role"] = "user" - else: - # If we have more than one message, we'll pull the system message out of the - # list. - system = messages[0]["content"] - messages.pop(0) - - # Convert any subsequent "system"-role messages to "user"-role - # messages, as Anthropic doesn't support system input messages. + # Convert any subsequent "system"/"developer"-role messages to "user"-role + # messages, as Anthropic doesn't support system or developer input messages. for message in messages: - if message["role"] == "system": + if message["role"] in ("system", "developer"): message["role"] = "user" # Merge consecutive messages with the same role. @@ -163,7 +180,7 @@ class AnthropicLLMAdapter(BaseLLMAdapter[AnthropicLLMInvocationParams]): elif isinstance(message["content"], list) and len(message["content"]) == 0: message["content"] = [{"type": "text", "text": "(empty)"}] - return self.ConvertedMessages(messages=messages, system=system) + return self.ConvertedMessages(messages=messages, system=system, system_role=system_role) def _from_universal_context_message(self, message: LLMContextMessage) -> MessageParam: if isinstance(message, LLMSpecificMessage): diff --git a/src/pipecat/adapters/services/bedrock_adapter.py b/src/pipecat/adapters/services/bedrock_adapter.py index d63c5cf0f..7df924880 100644 --- a/src/pipecat/adapters/services/bedrock_adapter.py +++ b/src/pipecat/adapters/services/bedrock_adapter.py @@ -47,19 +47,31 @@ class AWSBedrockLLMAdapter(BaseLLMAdapter[AWSBedrockLLMInvocationParams]): """Get the identifier used in LLMSpecificMessage instances for AWS Bedrock.""" return "aws" - def get_llm_invocation_params(self, context: LLMContext) -> AWSBedrockLLMInvocationParams: + def get_llm_invocation_params( + self, context: LLMContext, *, system_instruction: Optional[str] = None + ) -> AWSBedrockLLMInvocationParams: """Get AWS Bedrock-specific LLM invocation parameters from a universal LLM context. Args: context: The LLM context containing messages, tools, etc. + system_instruction: Optional system instruction from service settings + or ``run_inference``. Returns: Dictionary of parameters for invoking AWS Bedrock's LLM API. """ - messages = self._from_universal_context_messages(self.get_messages(context)) + converted = self._from_universal_context_messages( + self.get_messages(context), system_instruction=system_instruction + ) + effective_system = self._resolve_system_instruction( + converted.system, + converted.system_role, + system_instruction, + discard_context_system=True, + ) return { - "system": messages.system, - "messages": messages.messages, + "system": [{"text": effective_system}] if effective_system else None, + "messages": converted.messages, # NOTE: LLMContext's tools are guaranteed to be a ToolsSchema (or NOT_GIVEN) "tools": self.from_standard_tools(context.tools) or [], # To avoid refactoring in AWSBedrockLLMService, we just pass through tool_choice. @@ -96,32 +108,43 @@ class AWSBedrockLLMAdapter(BaseLLMAdapter[AWSBedrockLLMInvocationParams]): @dataclass class ConvertedMessages: - """Container for Anthropic-formatted messages converted from universal context.""" + """Container for Bedrock-formatted messages converted from universal context.""" messages: List[dict[str, Any]] system: Optional[str] + system_role: Optional[str] = None # "system" or "developer" — origin of extracted system def _from_universal_context_messages( - self, universal_context_messages: List[LLMContextMessage] + self, + universal_context_messages: List[LLMContextMessage], + *, + system_instruction: Optional[str] = None, ) -> ConvertedMessages: system = None - messages = [] + system_role = None - # First, map messages using self._from_universal_context_message(m) + # Extract initial system/developer from universal messages BEFORE conversion, + # so the helper works with standard message format (not provider-specific). + remaining = list(universal_context_messages) + if remaining and not isinstance(remaining[0], LLMSpecificMessage): + extracted_content, extracted_role = self._extract_initial_system_or_developer( + remaining, system_instruction=system_instruction + ) + if extracted_content is not None: + system = extracted_content + system_role = extracted_role + + # Convert remaining messages to Bedrock format + messages = [] try: - messages = [self._from_universal_context_message(m) for m in universal_context_messages] + messages = [self._from_universal_context_message(m) for m in remaining] except Exception as e: logger.error(f"Error mapping messages: {e}") - # See if we should pull the system message out of our messages list - if messages and messages[0]["role"] == "system": - system = messages[0]["content"] - messages.pop(0) - - # Convert any subsequent "system"-role messages to "user"-role - # messages, as AWS Bedrock doesn't support system input messages. + # Convert any subsequent "system"/"developer"-role messages to "user"-role + # messages, as AWS Bedrock doesn't support system or developer input messages. for message in messages: - if message["role"] == "system": + if message["role"] in ("system", "developer"): message["role"] = "user" # Merge consecutive messages with the same role. @@ -151,7 +174,7 @@ class AWSBedrockLLMAdapter(BaseLLMAdapter[AWSBedrockLLMInvocationParams]): elif isinstance(message["content"], list) and len(message["content"]) == 0: message["content"] = [{"type": "text", "text": "(empty)"}] - return self.ConvertedMessages(messages=messages, system=system) + return self.ConvertedMessages(messages=messages, system=system, system_role=system_role) def _from_universal_context_message(self, message: LLMContextMessage) -> dict[str, Any]: if isinstance(message, LLMSpecificMessage): diff --git a/src/pipecat/adapters/services/gemini_adapter.py b/src/pipecat/adapters/services/gemini_adapter.py index 4968c2719..d73ac388d 100644 --- a/src/pipecat/adapters/services/gemini_adapter.py +++ b/src/pipecat/adapters/services/gemini_adapter.py @@ -53,19 +53,31 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): """Get the identifier used in LLMSpecificMessage instances for Google.""" return "google" - def get_llm_invocation_params(self, context: LLMContext) -> GeminiLLMInvocationParams: + def get_llm_invocation_params( + self, context: LLMContext, *, system_instruction: Optional[str] = None + ) -> GeminiLLMInvocationParams: """Get Gemini-specific LLM invocation parameters from a universal LLM context. Args: context: The LLM context containing messages, tools, etc. + system_instruction: Optional system instruction from service settings + or ``run_inference``. Returns: Dictionary of parameters for Gemini's API. """ - messages = self._from_universal_context_messages(self.get_messages(context)) + converted = self._from_universal_context_messages( + self.get_messages(context), system_instruction=system_instruction + ) + effective_system = self._resolve_system_instruction( + converted.system_instruction, + converted.system_instruction_role, + system_instruction, + discard_context_system=True, + ) return { - "system_instruction": messages.system_instruction, - "messages": messages.messages, + "system_instruction": effective_system, + "messages": converted.messages, # NOTE: LLMContext's tools are guaranteed to be a ToolsSchema (or NOT_GIVEN) "tools": self.from_standard_tools(context.tools), } @@ -164,57 +176,65 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): messages: List[Content] system_instruction: Optional[str] = None + system_instruction_role: Optional[str] = None # "system" or "developer" @dataclass class MessageConversionResult: """Result of converting a single universal context message to Google format. - Either content (a Google Content object) or a system instruction string - is guaranteed to be set. - - Also returns a tool call ID to name mapping for any tool calls - discovered in the message. + Contains a Google Content object and a tool call ID to name mapping + for any tool calls discovered in the message. """ content: Optional[Content] = None - system_instruction: Optional[str] = None tool_call_id_to_name_mapping: Dict[str, str] = field(default_factory=dict) @dataclass class MessageConversionParams: """Parameters for converting a single universal context message to Google format.""" - already_have_system_instruction: bool tool_call_id_to_name_mapping: Dict[str, str] def _from_universal_context_messages( - self, universal_context_messages: List[LLMContextMessage] + self, + universal_context_messages: List[LLMContextMessage], + *, + system_instruction: Optional[str] = None, ) -> ConvertedMessages: """Restructures messages to ensure proper Google format and message ordering. This method handles conversion of OpenAI-formatted messages to Google format, - with special handling for function calls, function responses, and system messages. - System messages are added back to the context as user messages when needed. + with special handling for function calls, function responses, and system/developer + messages. - The final message order is preserved as: + Initial system/developer messages are extracted as the system instruction + (only from ``messages[0]``). Subsequent system/developer messages are + converted to user role. - 1. Function calls (from model) - 2. Function responses (from user) - 3. Text messages (converted from system messages) - - Note:: - - System messages are only added back when there are no regular text - messages in the context, ensuring proper conversation continuity - after function calls. + Args: + universal_context_messages: Messages from the LLM context. + system_instruction: Optional system instruction from service settings, + used to decide whether to extract an initial "developer" message. """ - system_instruction = None + # Extract initial system/developer message from universal messages before conversion. + # We work on a mutable copy so we can pop messages[0] if needed. + remaining_messages = list(universal_context_messages) + extracted_system = None + extracted_role = None + + # Extract initial system/developer from universal messages BEFORE conversion, + # so the helper works with standard message format. + if remaining_messages and not isinstance(remaining_messages[0], LLMSpecificMessage): + extracted_system, extracted_role = self._extract_initial_system_or_developer( + remaining_messages, system_instruction=system_instruction + ) + messages = [] tool_call_id_to_name_mapping = {} thought_signature_dicts = [] # Process each message, converting to Google format as needed - for message in universal_context_messages: + for message in remaining_messages: # We have a Google-specific message; this may either be a # thought-signature-containing message that we need to handle in a # special way, or a message already in Google format that we can @@ -237,16 +257,12 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): result = self._from_standard_message( message, params=self.MessageConversionParams( - already_have_system_instruction=bool(system_instruction), tool_call_id_to_name_mapping=tool_call_id_to_name_mapping, ), ) - # Each result is either a Content or a system instruction if result.content: messages.append(result.content) - elif result.system_instruction: - system_instruction = result.system_instruction # Merge tool call ID to name mapping if result.tool_call_id_to_name_mapping: @@ -259,6 +275,7 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): messages = self._merge_parallel_tool_calls_for_thinking(thought_signature_dicts, messages) # Check if we only have function-related messages (no regular text) + effective_system = extracted_system or system_instruction has_regular_messages = any( len(msg.parts) == 1 and getattr(msg.parts[0], "text", None) @@ -268,13 +285,17 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): ) # Add system instruction back as a user message if we only have function messages - if system_instruction and not has_regular_messages: - messages.append(Content(role="user", parts=[Part(text=system_instruction)])) + if effective_system and not has_regular_messages: + messages.append(Content(role="user", parts=[Part(text=effective_system)])) # Remove any empty messages messages = [m for m in messages if m.parts] - return self.ConvertedMessages(messages=messages, system_instruction=system_instruction) + return self.ConvertedMessages( + messages=messages, + system_instruction=extracted_system, + system_instruction_role=extracted_role, + ) def _from_standard_message( self, message: LLMStandardMessage, *, params: MessageConversionParams @@ -282,17 +303,16 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): """Convert standard universal context message to Google Content object. Handles conversion of text, images, and function calls to Google's - format. - System instructions are returned as a plain string. + format. System and developer messages at this stage (i.e. non-initial + ones, since the initial one is already extracted) are converted to + user role. Args: message: Message in standard universal context format. - already_have_system_instruction: Whether we already have a system instruction params: Parameters for conversion. Returns: - MessageConversionResult containing either a Content object or a - system instruction string. + MessageConversionResult containing a Content object. Examples: Standard text message:: @@ -333,20 +353,10 @@ class GeminiLLMAdapter(BaseLLMAdapter[GeminiLLMInvocationParams]): role = message["role"] content = message.get("content", []) - if role == "system": - if params.already_have_system_instruction: - role = "user" # Convert system message to user role if we already have a system instruction - else: - system_instruction: str = None - if isinstance(content, str): - system_instruction = content - elif isinstance(content, list): - # If content is a list, we assume it's a list of text parts, per the standard - system_instruction = " ".join( - part["text"] for part in content if part.get("type") == "text" - ) - if system_instruction: - return self.MessageConversionResult(system_instruction=system_instruction) + # Convert non-initial system/developer messages to user role, + # as Gemini doesn't support these as input messages. + if role in ("system", "developer"): + role = "user" elif role == "assistant": role = "model" diff --git a/src/pipecat/adapters/services/open_ai_adapter.py b/src/pipecat/adapters/services/open_ai_adapter.py index f4b534f2c..9e8b449d0 100644 --- a/src/pipecat/adapters/services/open_ai_adapter.py +++ b/src/pipecat/adapters/services/open_ai_adapter.py @@ -7,7 +7,7 @@ """OpenAI LLM adapter for Pipecat.""" import copy -from typing import Any, Dict, List, TypedDict +from typing import Any, Dict, List, Optional, TypedDict from openai._types import NotGiven as OpenAINotGiven from openai.types.chat import ( @@ -51,17 +51,35 @@ class OpenAILLMAdapter(BaseLLMAdapter[OpenAILLMInvocationParams]): """Get the identifier used in LLMSpecificMessage instances for OpenAI.""" return "openai" - def get_llm_invocation_params(self, context: LLMContext) -> OpenAILLMInvocationParams: + def get_llm_invocation_params( + self, context: LLMContext, *, system_instruction: Optional[str] = None + ) -> OpenAILLMInvocationParams: """Get OpenAI-specific LLM invocation parameters from a universal LLM context. Args: context: The LLM context containing messages, tools, etc. + system_instruction: Optional system instruction from service settings + or ``run_inference``. If provided, prepended as a system message. Returns: Dictionary of parameters for OpenAI's ChatCompletion API. """ + messages = self._from_universal_context_messages(self.get_messages(context)) + + if system_instruction: + # Detect initial system message for warning purposes (don't extract) + initial_role = messages[0].get("role") if messages else None + initial_content = messages[0].get("content", "") if initial_role == "system" else None + self._resolve_system_instruction( + initial_content, + initial_role if initial_role == "system" else None, + system_instruction, + discard_context_system=False, + ) + messages = [{"role": "system", "content": system_instruction}] + messages + return { - "messages": self._from_universal_context_messages(self.get_messages(context)), + "messages": messages, # NOTE; LLMContext's tools are guaranteed to be a ToolsSchema (or NOT_GIVEN) "tools": self.from_standard_tools(context.tools), "tool_choice": context.tool_choice, diff --git a/src/pipecat/adapters/services/open_ai_responses_adapter.py b/src/pipecat/adapters/services/open_ai_responses_adapter.py index 70627fe5d..89968a721 100644 --- a/src/pipecat/adapters/services/open_ai_responses_adapter.py +++ b/src/pipecat/adapters/services/open_ai_responses_adapter.py @@ -9,7 +9,6 @@ import copy from typing import Any, Dict, List, Optional, TypedDict -from loguru import logger from openai._types import NotGiven as OpenAINotGiven from openai.types.responses import FunctionToolParam, ResponseInputItemParam @@ -41,11 +40,6 @@ class OpenAIResponsesLLMAdapter(BaseLLMAdapter[OpenAIResponsesLLMInvocationParam - Extracting and sanitizing messages from the LLM context for logging """ - def __init__(self): - """Initialize the adapter.""" - super().__init__() - self._warned_system_instruction = False - @property def id_for_llm_specific_messages(self) -> str: """Get the identifier used in LLMSpecificMessage instances.""" @@ -67,6 +61,18 @@ class OpenAIResponsesLLMAdapter(BaseLLMAdapter[OpenAIResponsesLLMInvocationParam Dictionary of parameters for the Responses API. """ messages = self.get_messages(context) + + # Check for conflict: system_instruction + initial system message + if system_instruction and messages: + first_msg = messages[0] if not isinstance(messages[0], LLMSpecificMessage) else None + if first_msg and first_msg.get("role") == "system": + self._resolve_system_instruction( + first_msg.get("content", ""), + "system", + system_instruction, + discard_context_system=False, + ) + input_items = self._convert_messages_to_input(messages) params: OpenAIResponsesLLMInvocationParams = { @@ -158,24 +164,15 @@ class OpenAIResponsesLLMAdapter(BaseLLMAdapter[OpenAIResponsesLLMInvocationParam List of Responses API input items. """ result: List[ResponseInputItemParam] = [] - is_first = True for message in messages: if isinstance(message, LLMSpecificMessage): result.append(message.message) - is_first = False continue role = message.get("role") - if role == "system": - if is_first and not self._warned_system_instruction: - logger.warning( - "System messages in LLMContext are converted to 'developer' role for the " - "Responses API. Consider using settings.system_instruction instead, which " - "maps to the 'instructions' parameter." - ) - self._warned_system_instruction = True + if role in ("system", "developer"): content = message.get("content", "") if isinstance(content, list): content = self._convert_multimodal_content(content) @@ -218,8 +215,6 @@ class OpenAIResponsesLLMAdapter(BaseLLMAdapter[OpenAIResponsesLLMInvocationParam } ) - is_first = False - return result def _convert_multimodal_content(self, content: list) -> list: diff --git a/src/pipecat/adapters/services/perplexity_adapter.py b/src/pipecat/adapters/services/perplexity_adapter.py index a8fbe3c18..5359baddc 100644 --- a/src/pipecat/adapters/services/perplexity_adapter.py +++ b/src/pipecat/adapters/services/perplexity_adapter.py @@ -28,7 +28,7 @@ the messages are sent to Perplexity's API. """ import copy -from typing import List +from typing import List, Optional from openai.types.chat import ChatCompletionMessageParam @@ -49,17 +49,21 @@ class PerplexityLLMAdapter(OpenAILLMAdapter): ``build_chat_completion_params`` prepends ``system_instruction``. """ - def get_llm_invocation_params(self, context: LLMContext) -> OpenAILLMInvocationParams: + def get_llm_invocation_params( + self, context: LLMContext, *, system_instruction: Optional[str] = None + ) -> OpenAILLMInvocationParams: """Get OpenAI-compatible invocation parameters with Perplexity message fixes applied. Args: context: The LLM context containing messages, tools, etc. + system_instruction: Optional system instruction from service settings + or ``run_inference``. Forwarded to the parent adapter. Returns: Dictionary of parameters for Perplexity's ChatCompletion API, with messages transformed to satisfy Perplexity's constraints. """ - params = super().get_llm_invocation_params(context) + params = super().get_llm_invocation_params(context, system_instruction=system_instruction) params["messages"] = self._transform_messages(list(params["messages"])) return params diff --git a/src/pipecat/services/anthropic/llm.py b/src/pipecat/services/anthropic/llm.py index 2f375df82..f35d6226c 100644 --- a/src/pipecat/services/anthropic/llm.py +++ b/src/pipecat/services/anthropic/llm.py @@ -370,10 +370,13 @@ class AnthropicLLMService(LLMService): messages = [] system = NOT_GIVEN tools = [] + effective_instruction = system_instruction or self._settings.system_instruction if isinstance(context, LLMContext): adapter: AnthropicLLMAdapter = self.get_llm_adapter() invocation_params = adapter.get_llm_invocation_params( - context, enable_prompt_caching=self._settings.enable_prompt_caching + context, + enable_prompt_caching=self._settings.enable_prompt_caching, + system_instruction=effective_instruction, ) messages = invocation_params["messages"] system = invocation_params["system"] @@ -384,15 +387,6 @@ class AnthropicLLMService(LLMService): system = getattr(context, "system", NOT_GIVEN) tools = context.tools or [] - # Override system instruction if provided - if system_instruction is not None: - if system and system is not NOT_GIVEN: - logger.warning( - f"{self}: Both system_instruction and a system message in context are set." - " Using system_instruction." - ) - system = system_instruction - # Build params using the same method as streaming completions params = { "model": self._settings.model, @@ -460,15 +454,10 @@ class AnthropicLLMService(LLMService): if isinstance(context, LLMContext): adapter: AnthropicLLMAdapter = self.get_llm_adapter() params: AnthropicLLMInvocationParams = adapter.get_llm_invocation_params( - context, enable_prompt_caching=self._settings.enable_prompt_caching + context, + enable_prompt_caching=self._settings.enable_prompt_caching, + system_instruction=self._settings.system_instruction, ) - if self._settings.system_instruction: - if params["system"] is not NOT_GIVEN: - logger.warning( - f"{self}: Both system_instruction and a system message in context are" - " set. Using system_instruction." - ) - params["system"] = self._settings.system_instruction return params # Anthropic-specific context diff --git a/src/pipecat/services/aws/llm.py b/src/pipecat/services/aws/llm.py index 92049dffb..cda186167 100644 --- a/src/pipecat/services/aws/llm.py +++ b/src/pipecat/services/aws/llm.py @@ -942,25 +942,19 @@ class AWSBedrockLLMService(LLMService): """ messages = [] system = [] + effective_instruction = system_instruction or self._settings.system_instruction if isinstance(context, LLMContext): adapter: AWSBedrockLLMAdapter = self.get_llm_adapter() - params: AWSBedrockLLMInvocationParams = adapter.get_llm_invocation_params(context) + params: AWSBedrockLLMInvocationParams = adapter.get_llm_invocation_params( + context, system_instruction=effective_instruction + ) messages = params["messages"] - system = params["system"] # [{"text": "system message"}] + system = params["system"] # [{"text": "system message"}] or None else: context = AWSBedrockLLMContext.upgrade_to_bedrock(context) messages = context.messages system = getattr(context, "system", None) # [{"text": "system message"}] - # Override system instruction if provided - if system_instruction is not None: - if system: - logger.warning( - f"{self}: Both system_instruction and a system message in context are set." - " Using system_instruction." - ) - system = [{"text": system_instruction}] - # Prepare request parameters using the same method as streaming inference_config = self._build_inference_config() @@ -1086,14 +1080,9 @@ class AWSBedrockLLMService(LLMService): # Universal LLMContext if isinstance(context, LLMContext): adapter: AWSBedrockLLMAdapter = self.get_llm_adapter() - params: AWSBedrockLLMInvocationParams = adapter.get_llm_invocation_params(context) - if self._settings.system_instruction: - if params["system"]: - logger.warning( - f"{self}: Both system_instruction and a system message in context are" - " set. Using system_instruction." - ) - params["system"] = [{"text": self._settings.system_instruction}] + params: AWSBedrockLLMInvocationParams = adapter.get_llm_invocation_params( + context, system_instruction=self._settings.system_instruction + ) return params # AWS Bedrock-specific context diff --git a/src/pipecat/services/cerebras/llm.py b/src/pipecat/services/cerebras/llm.py index dfb62baf8..5be270ae1 100644 --- a/src/pipecat/services/cerebras/llm.py +++ b/src/pipecat/services/cerebras/llm.py @@ -114,15 +114,4 @@ class CerebrasLLMService(OpenAILLMService): params.update(self._settings.extra) - # Prepend system instruction if set - if self._settings.system_instruction: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [ - {"role": "system", "content": self._settings.system_instruction} - ] + messages - return params diff --git a/src/pipecat/services/fireworks/llm.py b/src/pipecat/services/fireworks/llm.py index 5efa60793..7d2997987 100644 --- a/src/pipecat/services/fireworks/llm.py +++ b/src/pipecat/services/fireworks/llm.py @@ -115,15 +115,4 @@ class FireworksLLMService(OpenAILLMService): params.update(self._settings.extra) - # Prepend system instruction if set - if self._settings.system_instruction: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [ - {"role": "system", "content": self._settings.system_instruction} - ] + messages - return params diff --git a/src/pipecat/services/google/llm.py b/src/pipecat/services/google/llm.py index 26ad46311..c8d54830e 100644 --- a/src/pipecat/services/google/llm.py +++ b/src/pipecat/services/google/llm.py @@ -904,9 +904,12 @@ class GoogleLLMService(LLMService): messages = [] system = [] tools = [] + effective_instruction = system_instruction or self._settings.system_instruction if isinstance(context, LLMContext): adapter = self.get_llm_adapter() - params: GeminiLLMInvocationParams = adapter.get_llm_invocation_params(context) + params: GeminiLLMInvocationParams = adapter.get_llm_invocation_params( + context, system_instruction=effective_instruction + ) messages = params["messages"] system = params["system_instruction"] tools = params["tools"] @@ -916,15 +919,6 @@ class GoogleLLMService(LLMService): system = getattr(context, "system_message", None) tools = context.tools or [] - # Override system instruction if provided - if system_instruction is not None: - if system: - logger.warning( - f"{self}: Both system_instruction and a system message in context are set." - " Using system_instruction." - ) - system = system_instruction - # Build generation config using the same method as streaming generation_params = self._build_generation_params( system_instruction=system, tools=tools if tools else None @@ -1015,15 +1009,8 @@ class GoogleLLMService(LLMService): ) -> AsyncIterator[GenerateContentResponse]: messages = params_from_context["messages"] - # Constructor/settings system instruction takes priority over context. - if self._settings.system_instruction and params_from_context["system_instruction"]: - logger.warning( - f"{self}: Both system_instruction and a system message in context are" - " set. Using system_instruction." - ) - system_instruction = ( - self._settings.system_instruction or params_from_context["system_instruction"] - ) + # The adapter already resolved system_instruction vs context system message. + system_instruction = params_from_context["system_instruction"] tools = [] if params_from_context["tools"]: @@ -1072,7 +1059,9 @@ class GoogleLLMService(LLMService): self, context: LLMContext ) -> AsyncIterator[GenerateContentResponse]: adapter = self.get_llm_adapter() - params: GeminiLLMInvocationParams = adapter.get_llm_invocation_params(context) + params: GeminiLLMInvocationParams = adapter.get_llm_invocation_params( + context, system_instruction=self._settings.system_instruction + ) logger.debug( f"{self}: Generating chat from universal context [{params['system_instruction']}] | {adapter.get_messages_for_logging(context)}" diff --git a/src/pipecat/services/mistral/llm.py b/src/pipecat/services/mistral/llm.py index 063dac3aa..a2edb4cb6 100644 --- a/src/pipecat/services/mistral/llm.py +++ b/src/pipecat/services/mistral/llm.py @@ -233,15 +233,4 @@ class MistralLLMService(OpenAILLMService): # Add any extra parameters params.update(self._settings.extra) - # Prepend system instruction if set - if self._settings.system_instruction: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [ - {"role": "system", "content": self._settings.system_instruction} - ] + messages - return params diff --git a/src/pipecat/services/openai/base_llm.py b/src/pipecat/services/openai/base_llm.py index eb8ce3cc6..d2b19e2df 100644 --- a/src/pipecat/services/openai/base_llm.py +++ b/src/pipecat/services/openai/base_llm.py @@ -327,17 +327,6 @@ class BaseOpenAILLMService(LLMService): params.update(self._settings.extra) - # Prepend system instruction from constructor - if self._settings.system_instruction: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [ - {"role": "system", "content": self._settings.system_instruction} - ] + messages - return params async def run_inference( @@ -358,10 +347,11 @@ class BaseOpenAILLMService(LLMService): Returns: The LLM's response as a string, or None if no response is generated. """ + effective_instruction = system_instruction or self._settings.system_instruction if isinstance(context, LLMContext): adapter = self.get_llm_adapter() invocation_params: OpenAILLMInvocationParams = adapter.get_llm_invocation_params( - context + context, system_instruction=effective_instruction ) else: invocation_params = OpenAILLMInvocationParams( @@ -375,15 +365,6 @@ class BaseOpenAILLMService(LLMService): params["stream"] = False params.pop("stream_options", None) - # Prepend system instruction if provided - if system_instruction is not None: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [{"role": "system", "content": system_instruction}] + messages - # Override max_tokens if provided if max_tokens is not None: # Use max_completion_tokens for newer models, fallback to max_tokens @@ -439,7 +420,9 @@ class BaseOpenAILLMService(LLMService): f"{self}: Generating chat from universal context {adapter.get_messages_for_logging(context)}" ) - params: OpenAILLMInvocationParams = adapter.get_llm_invocation_params(context) + params: OpenAILLMInvocationParams = adapter.get_llm_invocation_params( + context, system_instruction=self._settings.system_instruction + ) chunks = await self.get_chat_completions(params) return chunks diff --git a/src/pipecat/services/perplexity/llm.py b/src/pipecat/services/perplexity/llm.py index 9ea323c5d..b4aedc3f7 100644 --- a/src/pipecat/services/perplexity/llm.py +++ b/src/pipecat/services/perplexity/llm.py @@ -121,17 +121,6 @@ class PerplexityLLMService(OpenAILLMService): if self._settings.max_tokens is not None: params["max_tokens"] = self._settings.max_tokens - # Prepend system instruction if set - if self._settings.system_instruction: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [ - {"role": "system", "content": self._settings.system_instruction} - ] + messages - return params async def _process_context(self, context: OpenAILLMContext | LLMContext): diff --git a/src/pipecat/services/sambanova/llm.py b/src/pipecat/services/sambanova/llm.py index 710a22db2..35cf60886 100644 --- a/src/pipecat/services/sambanova/llm.py +++ b/src/pipecat/services/sambanova/llm.py @@ -131,17 +131,6 @@ class SambaNovaLLMService(OpenAILLMService): # type: ignore params.update(self._settings.extra) - # Prepend system instruction if set - if self._settings.system_instruction: - messages = params.get("messages", []) - if messages and messages[0].get("role") == "system": - logger.warning( - f"{self}: Both system_instruction and an initial system message in context are set. This may be unintended." - ) - params["messages"] = [ - {"role": "system", "content": self._settings.system_instruction} - ] + messages - return params @traced_llm # type: ignore diff --git a/tests/test_adapter_system_instruction.py b/tests/test_adapter_system_instruction.py new file mode 100644 index 000000000..125702dd6 --- /dev/null +++ b/tests/test_adapter_system_instruction.py @@ -0,0 +1,587 @@ +# +# Copyright (c) 2024-2026, Daily +# +# SPDX-License-Identifier: BSD 2-Clause License +# + +"""Unit tests for system_instruction and developer message handling in LLM adapters. + +Tests cover: + +1. system_instruction only (no system/developer in context) +2. Initial "system" message only (no system_instruction) +3. Initial "developer" message only (no system_instruction) -> promoted to system instruction +4. Both system_instruction and initial "system" message -> warns +5. Both system_instruction and initial "developer" message -> does NOT warn; developer becomes "user" +6. Non-OpenAI adapters: subsequent "developer" messages converted to "user" +7. Non-OpenAI adapters: initial "system" discarded when system_instruction provided +8. Gemini: non-initial "system" message is converted to "user" (not extracted) +9. Single system-only message: converted to "user" instead of extracting (empty list prevention) +""" + +import unittest +from unittest.mock import patch + +from pipecat.adapters.services.open_ai_adapter import OpenAILLMAdapter +from pipecat.processors.aggregators.llm_context import LLMContext, LLMStandardMessage + + +class TestOpenAIAdapterSystemInstruction(unittest.TestCase): + """Tests for the OpenAI ChatCompletion adapter.""" + + def setUp(self): + self.adapter = OpenAILLMAdapter() + + def test_system_instruction_only(self): + """system_instruction alone is prepended as a system message.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, system_instruction="Be helpful.") + + self.assertEqual(params["messages"][0]["role"], "system") + self.assertEqual(params["messages"][0]["content"], "Be helpful.") + self.assertEqual(params["messages"][1]["role"], "user") + + def test_initial_system_message_only(self): + """Initial system message without system_instruction passes through.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(len(params["messages"]), 2) + self.assertEqual(params["messages"][0]["role"], "system") + self.assertEqual(params["messages"][0]["content"], "You are helpful.") + + def test_both_system_instruction_and_system_message_warns(self): + """system_instruction + initial system message warns but allows both.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, system_instruction="Be concise." + ) + mock_logger.warning.assert_called_once() + warning_msg = mock_logger.warning.call_args[0][0] + self.assertIn("may be unintended", warning_msg) + + # Both are present: prepended system_instruction + original system message + self.assertEqual(params["messages"][0]["content"], "Be concise.") + self.assertEqual(params["messages"][1]["content"], "You are helpful.") + + def test_both_system_instruction_and_developer_message_no_warning(self): + """system_instruction + initial developer message does NOT warn.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, system_instruction="Be concise." + ) + mock_logger.warning.assert_not_called() + + # system_instruction prepended, developer message stays in messages + self.assertEqual(params["messages"][0]["content"], "Be concise.") + self.assertEqual(params["messages"][1]["role"], "developer") + + def test_warning_fires_only_once(self): + """Conflict warning fires only once per adapter instance.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + self.adapter.get_llm_invocation_params(context, system_instruction="Be concise.") + self.adapter.get_llm_invocation_params(context, system_instruction="Be concise.") + mock_logger.warning.assert_called_once() + + +class TestAnthropicAdapterSystemInstruction(unittest.TestCase): + """Tests for the Anthropic adapter.""" + + def setUp(self): + from pipecat.adapters.services.anthropic_adapter import AnthropicLLMAdapter + + self.adapter = AnthropicLLMAdapter() + + def test_system_instruction_only(self): + """system_instruction alone becomes the system parameter.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params( + context, enable_prompt_caching=False, system_instruction="Be helpful." + ) + + self.assertEqual(params["system"], "Be helpful.") + self.assertEqual(len(params["messages"]), 1) + self.assertEqual(params["messages"][0]["role"], "user") + + def test_initial_system_message_only(self): + """Initial system message is extracted as the system parameter.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, enable_prompt_caching=False) + + self.assertEqual(params["system"], "You are helpful.") + self.assertEqual(len(params["messages"]), 1) + self.assertEqual(params["messages"][0]["role"], "user") + + def test_initial_developer_message_promoted(self): + """Initial developer message without system_instruction is promoted to system.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, enable_prompt_caching=False) + + self.assertEqual(params["system"], "Extra context.") + self.assertEqual(len(params["messages"]), 1) + + def test_both_system_instruction_and_system_message_warns(self): + """system_instruction + initial system message warns and uses system_instruction.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, + enable_prompt_caching=False, + system_instruction="Be concise.", + ) + mock_logger.warning.assert_called_once() + warning_msg = mock_logger.warning.call_args[0][0] + self.assertIn("Using system_instruction", warning_msg) + + self.assertEqual(params["system"], "Be concise.") + + def test_both_system_instruction_and_developer_message_no_warning(self): + """system_instruction + initial developer message: no warning, developer becomes user.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "assistant", "content": "Hi"}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, + enable_prompt_caching=False, + system_instruction="Be concise.", + ) + mock_logger.warning.assert_not_called() + + self.assertEqual(params["system"], "Be concise.") + # Developer message should have been converted to "user" + self.assertEqual(params["messages"][0]["role"], "user") + self.assertEqual(params["messages"][0]["content"], "Extra context.") + + def test_subsequent_developer_messages_converted_to_user(self): + """Subsequent developer messages are converted to user role.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi"}, + {"role": "developer", "content": "More instructions"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, enable_prompt_caching=False) + + # Developer message was converted to "user" + self.assertEqual(params["messages"][2]["role"], "user") + self.assertEqual(params["messages"][2]["content"], "More instructions") + + def test_initial_system_discarded_when_system_instruction_provided(self): + """Initial system message is discarded when system_instruction is provided.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "Old instruction."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger"): + params = self.adapter.get_llm_invocation_params( + context, + enable_prompt_caching=False, + system_instruction="New instruction.", + ) + + self.assertEqual(params["system"], "New instruction.") + # Only the user message should remain + self.assertEqual(len(params["messages"]), 1) + self.assertEqual(params["messages"][0]["role"], "user") + + def test_single_system_message_becomes_user(self): + """A lone system message is converted to user (not extracted) to prevent empty history.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, enable_prompt_caching=False) + + from anthropic import NOT_GIVEN + + self.assertEqual(params["system"], NOT_GIVEN) + self.assertEqual(len(params["messages"]), 1) + self.assertEqual(params["messages"][0]["role"], "user") + + +class TestBedrockAdapterSystemInstruction(unittest.TestCase): + """Tests for the AWS Bedrock adapter.""" + + def setUp(self): + from pipecat.adapters.services.bedrock_adapter import AWSBedrockLLMAdapter + + self.adapter = AWSBedrockLLMAdapter() + + def test_system_instruction_only(self): + """system_instruction alone becomes the system parameter.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, system_instruction="Be helpful.") + + self.assertEqual(params["system"], [{"text": "Be helpful."}]) + + def test_initial_system_message_only(self): + """Initial system message is extracted as the system parameter.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(params["system"], [{"text": "You are helpful."}]) + self.assertEqual(len(params["messages"]), 1) + + def test_initial_developer_message_promoted(self): + """Initial developer message without system_instruction is promoted.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(params["system"], [{"text": "Extra context."}]) + + def test_both_system_instruction_and_system_message_warns(self): + """system_instruction + initial system message warns and uses system_instruction.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, system_instruction="Be concise." + ) + mock_logger.warning.assert_called_once() + + self.assertEqual(params["system"], [{"text": "Be concise."}]) + + def test_both_system_instruction_and_developer_message_no_warning(self): + """system_instruction + initial developer message: no warning, developer becomes user.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, system_instruction="Be concise." + ) + mock_logger.warning.assert_not_called() + + self.assertEqual(params["system"], [{"text": "Be concise."}]) + self.assertEqual(params["messages"][0]["role"], "user") + + def test_subsequent_developer_messages_converted_to_user(self): + """Subsequent developer messages are converted to user role.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi"}, + {"role": "developer", "content": "More instructions"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(params["messages"][2]["role"], "user") + + def test_single_system_message_becomes_user(self): + """A lone system message is converted to user to prevent empty history.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertIsNone(params["system"]) + self.assertEqual(len(params["messages"]), 1) + self.assertEqual(params["messages"][0]["role"], "user") + + +class TestGeminiAdapterSystemInstruction(unittest.TestCase): + """Tests for the Gemini adapter.""" + + def setUp(self): + from pipecat.adapters.services.gemini_adapter import GeminiLLMAdapter + + self.adapter = GeminiLLMAdapter() + + def test_system_instruction_only(self): + """system_instruction alone becomes the system_instruction parameter.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context, system_instruction="Be helpful.") + + self.assertEqual(params["system_instruction"], "Be helpful.") + + def test_initial_system_message_only(self): + """Initial system message is extracted as system_instruction.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(params["system_instruction"], "You are helpful.") + self.assertEqual(len(params["messages"]), 1) + + def test_initial_developer_message_promoted(self): + """Initial developer message without system_instruction is promoted.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(params["system_instruction"], "Extra context.") + + def test_both_system_instruction_and_system_message_warns(self): + """system_instruction + initial system message warns and uses system_instruction.""" + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, system_instruction="Be concise." + ) + mock_logger.warning.assert_called_once() + + self.assertEqual(params["system_instruction"], "Be concise.") + + def test_both_system_instruction_and_developer_message_no_warning(self): + """system_instruction + initial developer message: no warning, developer becomes user.""" + messages: list[LLMStandardMessage] = [ + {"role": "developer", "content": "Extra context."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = self.adapter.get_llm_invocation_params( + context, system_instruction="Be concise." + ) + mock_logger.warning.assert_not_called() + + self.assertEqual(params["system_instruction"], "Be concise.") + + def test_non_initial_system_message_not_extracted(self): + """Non-initial system message is converted to user, not extracted as system instruction.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + {"role": "system", "content": "Late system message"}, + {"role": "user", "content": "How are you?"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + # No system instruction should be extracted from non-initial position + self.assertIsNone(params["system_instruction"]) + # The system message should have been converted to user role in the Gemini Content + # (we check that 3 messages are present, meaning no extraction happened) + self.assertEqual(len(params["messages"]), 3) + + def test_subsequent_developer_messages_converted_to_user(self): + """Subsequent developer messages are converted to user role.""" + messages: list[LLMStandardMessage] = [ + {"role": "user", "content": "Hello"}, + {"role": "developer", "content": "More instructions"}, + ] + context = LLMContext(messages=messages) + params = self.adapter.get_llm_invocation_params(context) + + self.assertEqual(len(params["messages"]), 2) + # Second message (developer) should be converted to user in Google format + self.assertEqual(params["messages"][1].role, "user") + + +class TestBaseLLMAdapterHelpers(unittest.TestCase): + """Tests for the shared helper methods on BaseLLMAdapter.""" + + def setUp(self): + # Use OpenAILLMAdapter as a concrete implementation for testing the base helpers + self.adapter = OpenAILLMAdapter() + + def test_extract_system_message(self): + """System message is extracted from messages[0].""" + messages = [ + {"role": "system", "content": "Be helpful."}, + {"role": "user", "content": "Hello"}, + ] + content, role = self.adapter._extract_initial_system_or_developer( + messages, system_instruction=None + ) + + self.assertEqual(content, "Be helpful.") + self.assertEqual(role, "system") + self.assertEqual(len(messages), 1) # popped + + def test_extract_developer_without_system_instruction(self): + """Developer message is extracted when no system_instruction.""" + messages = [ + {"role": "developer", "content": "Context."}, + {"role": "user", "content": "Hello"}, + ] + content, role = self.adapter._extract_initial_system_or_developer( + messages, system_instruction=None + ) + + self.assertEqual(content, "Context.") + self.assertEqual(role, "developer") + self.assertEqual(len(messages), 1) + + def test_developer_with_system_instruction_converts_to_user(self): + """Developer message with system_instruction is converted to user, not extracted.""" + messages = [ + {"role": "developer", "content": "Context."}, + {"role": "user", "content": "Hello"}, + ] + content, role = self.adapter._extract_initial_system_or_developer( + messages, system_instruction="Be helpful." + ) + + self.assertIsNone(content) + self.assertIsNone(role) + self.assertEqual(len(messages), 2) # not popped + self.assertEqual(messages[0]["role"], "user") # converted to user + + def test_single_system_message_becomes_user(self): + """Single system message is converted to user instead of extracting (empty prevention).""" + messages = [ + {"role": "system", "content": "Be helpful."}, + ] + content, role = self.adapter._extract_initial_system_or_developer( + messages, system_instruction=None + ) + + self.assertIsNone(content) + self.assertIsNone(role) + self.assertEqual(len(messages), 1) # not popped + self.assertEqual(messages[0]["role"], "user") + + def test_non_system_message_ignored(self): + """Non-system/developer first message is ignored.""" + messages = [ + {"role": "user", "content": "Hello"}, + ] + content, role = self.adapter._extract_initial_system_or_developer( + messages, system_instruction=None + ) + + self.assertIsNone(content) + self.assertIsNone(role) + self.assertEqual(len(messages), 1) + + def test_empty_messages(self): + """Empty messages list returns None.""" + messages = [] + content, role = self.adapter._extract_initial_system_or_developer( + messages, system_instruction=None + ) + + self.assertIsNone(content) + self.assertIsNone(role) + + def test_resolve_both_system_discard(self): + """Resolve with discard=True: system_instruction wins, warns.""" + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + result = self.adapter._resolve_system_instruction( + "from context", "system", "from settings", discard_context_system=True + ) + mock_logger.warning.assert_called_once() + + self.assertEqual(result, "from settings") + + def test_resolve_both_system_keep(self): + """Resolve with discard=False: warns but returns system_instruction.""" + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + result = self.adapter._resolve_system_instruction( + "from context", "system", "from settings", discard_context_system=False + ) + mock_logger.warning.assert_called_once() + + self.assertEqual(result, "from settings") + + def test_resolve_only_system_instruction(self): + """Only system_instruction: returns it, no warning.""" + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + result = self.adapter._resolve_system_instruction( + None, None, "from settings", discard_context_system=True + ) + mock_logger.warning.assert_not_called() + + self.assertEqual(result, "from settings") + + def test_resolve_only_context_system_discard(self): + """Only context system (discard=True): returns it.""" + result = self.adapter._resolve_system_instruction( + "from context", "system", None, discard_context_system=True + ) + + self.assertEqual(result, "from context") + + def test_resolve_only_context_system_keep(self): + """Only context system (discard=False): returns None (already in messages).""" + result = self.adapter._resolve_system_instruction( + "from context", "system", None, discard_context_system=False + ) + + self.assertIsNone(result) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_get_llm_invocation_params.py b/tests/test_get_llm_invocation_params.py index 9cfeb8933..279288661 100644 --- a/tests/test_get_llm_invocation_params.py +++ b/tests/test_get_llm_invocation_params.py @@ -424,10 +424,11 @@ class TestGeminiGetLLMInvocationParams(unittest.TestCase): context = LLMContext(messages=messages) params = self.adapter.get_llm_invocation_params(context) - # System instruction should be extracted - self.assertEqual(params["system_instruction"], "You are a helpful assistant.") + # When there's only one message, it's converted to user in-place (not extracted) + # so system_instruction is None + self.assertIsNone(params["system_instruction"]) - # But since there are no other messages, it should also be added back as a user message + # The system message should be converted to a user message self.assertEqual(len(params["messages"]), 1) self.assertEqual(params["messages"][0].role, "user") self.assertEqual(params["messages"][0].parts[0].text, "You are a helpful assistant.") @@ -973,7 +974,7 @@ class TestAWSBedrockGetLLMInvocationParams(unittest.TestCase): self.assertEqual(params["messages"][2]["content"][0]["text"], "Remember to be concise.") def test_single_system_message_handling(self): - """Test that a single system message is extracted as system parameter and no messages remain.""" + """Test that a single system message is converted to user role when no other messages exist.""" messages = [ {"role": "system", "content": "You are a helpful assistant."}, ] @@ -984,13 +985,16 @@ class TestAWSBedrockGetLLMInvocationParams(unittest.TestCase): # Get invocation params params = self.adapter.get_llm_invocation_params(context) - # System should be extracted (in AWS Bedrock format) - self.assertIsInstance(params["system"], list) - self.assertEqual(len(params["system"]), 1) - self.assertEqual(params["system"][0]["text"], "You are a helpful assistant.") + # When there's only one message, it's converted to user in-place (not extracted) + # so system is None + self.assertIsNone(params["system"]) - # No messages should remain after system extraction - self.assertEqual(len(params["messages"]), 0) + # Single system message should be converted to user role + self.assertEqual(len(params["messages"]), 1) + self.assertEqual(params["messages"][0]["role"], "user") + self.assertEqual( + params["messages"][0]["content"][0]["text"], "You are a helpful assistant." + ) class TestPerplexityGetLLMInvocationParams(unittest.TestCase): diff --git a/tests/test_openai_responses_adapter.py b/tests/test_openai_responses_adapter.py index 973c05c8c..9b0237baf 100644 --- a/tests/test_openai_responses_adapter.py +++ b/tests/test_openai_responses_adapter.py @@ -58,9 +58,8 @@ class TestOpenAIResponsesAdapter(unittest.TestCase): self.assertEqual(params["input"][0]["role"], "developer") self.assertEqual(params["input"][0]["content"], "You are helpful.") - def test_first_system_message_triggers_warning(self): - """First system message triggers a warning about using system_instruction.""" - # Use a fresh adapter so the warning hasn't been emitted yet + def test_system_message_without_system_instruction_no_warning(self): + """System message without system_instruction does not trigger a warning.""" adapter = OpenAIResponsesLLMAdapter() messages: list[LLMStandardMessage] = [ {"role": "system", "content": "You are helpful."}, @@ -68,8 +67,21 @@ class TestOpenAIResponsesAdapter(unittest.TestCase): ] context = LLMContext(messages=messages) - with patch("pipecat.adapters.services.open_ai_responses_adapter.logger") as mock_logger: + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: adapter.get_llm_invocation_params(context) + mock_logger.warning.assert_not_called() + + def test_system_message_with_system_instruction_triggers_warning(self): + """System message + system_instruction triggers a conflict warning.""" + adapter = OpenAIResponsesLLMAdapter() + messages: list[LLMStandardMessage] = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "Hello"}, + ] + context = LLMContext(messages=messages) + + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + adapter.get_llm_invocation_params(context, system_instruction="Be concise.") mock_logger.warning.assert_called_once() warning_msg = mock_logger.warning.call_args[0][0] self.assertIn("system_instruction", warning_msg) @@ -83,15 +95,15 @@ class TestOpenAIResponsesAdapter(unittest.TestCase): context = LLMContext(messages=messages) adapter = OpenAIResponsesLLMAdapter() - with patch("pipecat.adapters.services.open_ai_responses_adapter.logger") as mock_logger: - params = adapter.get_llm_invocation_params(context) + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + params = adapter.get_llm_invocation_params(context, system_instruction="Be helpful.") mock_logger.warning.assert_not_called() self.assertEqual(params["input"][1]["role"], "developer") self.assertEqual(params["input"][1]["content"], "New instruction") - def test_first_system_message_warning_fires_only_once(self): - """The first-system-message warning fires only once per adapter instance.""" + def test_conflict_warning_fires_only_once(self): + """The conflict warning fires only once per adapter instance.""" messages: list[LLMStandardMessage] = [ {"role": "system", "content": "You are helpful."}, {"role": "user", "content": "Hello"}, @@ -99,9 +111,9 @@ class TestOpenAIResponsesAdapter(unittest.TestCase): context = LLMContext(messages=messages) adapter = OpenAIResponsesLLMAdapter() - with patch("pipecat.adapters.services.open_ai_responses_adapter.logger") as mock_logger: - adapter.get_llm_invocation_params(context) - adapter.get_llm_invocation_params(context) + with patch("pipecat.adapters.base_llm_adapter.logger") as mock_logger: + adapter.get_llm_invocation_params(context, system_instruction="Be concise.") + adapter.get_llm_invocation_params(context, system_instruction="Be concise.") # Warning should have been emitted exactly once, not twice mock_logger.warning.assert_called_once() diff --git a/tests/test_run_inference.py b/tests/test_run_inference.py index cef13fb27..884b1b922 100644 --- a/tests/test_run_inference.py +++ b/tests/test_run_inference.py @@ -60,7 +60,9 @@ async def test_openai_run_inference_with_llm_context(): # Verify assert result == "Hello! How can I help you today?" service.get_llm_adapter.assert_called_once() - mock_adapter.get_llm_invocation_params.assert_called_once_with(mock_context) + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, system_instruction=None + ) service._client.chat.completions.create.assert_called_once_with( model="gpt-4", stream=False, @@ -187,7 +189,7 @@ async def test_anthropic_run_inference_with_llm_context(): assert result == "Hello! How can I help you today?" service.get_llm_adapter.assert_called_once() mock_adapter.get_llm_invocation_params.assert_called_once_with( - mock_context, enable_prompt_caching=False + mock_context, enable_prompt_caching=False, system_instruction=None ) service._client.beta.messages.create.assert_called_once_with( model="claude-3-sonnet-20240229", @@ -302,7 +304,9 @@ async def test_google_run_inference_with_llm_context(): # Verify assert result == "Hello! How can I help you today?" service.get_llm_adapter.assert_called_once() - mock_adapter.get_llm_invocation_params.assert_called_once_with(mock_context) + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, system_instruction=None + ) service._client.aio.models.generate_content.assert_called_once() @@ -421,7 +425,9 @@ async def test_aws_bedrock_run_inference_with_llm_context(): # Verify assert result == "Hello! How can I help you today?" service.get_llm_adapter.assert_called_once() - mock_adapter.get_llm_invocation_params.assert_called_once_with(mock_context) + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, system_instruction=None + ) # Verify the call includes configured parameters call_kwargs = mock_client.converse.call_args.kwargs @@ -543,15 +549,10 @@ async def test_openai_run_inference_system_instruction_overrides_context(): ) assert result == "Response" - call_kwargs = service._client.chat.completions.create.call_args.kwargs - messages = call_kwargs["messages"] - # system_instruction should be prepended as the first message - assert messages[0] == {"role": "system", "content": "New system instruction"} - # Original system message should still be present - assert messages[1] == {"role": "system", "content": "Original system message"} - # User message should still be present - assert messages[2] == {"role": "user", "content": "Hello"} - assert len(messages) == 3 + # Verify the adapter was called with the correct system_instruction + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, system_instruction="New system instruction" + ) @pytest.mark.asyncio @@ -608,9 +609,12 @@ async def test_anthropic_run_inference_system_instruction_overrides_context(): result = await service.run_inference(mock_context, system_instruction="New system instruction") assert result == "Response" - call_kwargs = service._client.beta.messages.create.call_args.kwargs - assert call_kwargs["system"] == "New system instruction" - assert call_kwargs["messages"] == test_messages + # Verify the adapter was called with the correct system_instruction + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, + enable_prompt_caching=False, + system_instruction="New system instruction", + ) @pytest.mark.asyncio @@ -665,9 +669,10 @@ async def test_google_run_inference_system_instruction_overrides_context(): result = await service.run_inference(mock_context, system_instruction="New system instruction") assert result == "Response" - call_kwargs = service._client.aio.models.generate_content.call_args.kwargs - config = call_kwargs["config"] - assert config.system_instruction == "New system instruction" + # Verify the adapter was called with the correct system_instruction + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, system_instruction="New system instruction" + ) @pytest.mark.asyncio @@ -731,9 +736,10 @@ async def test_aws_bedrock_run_inference_system_instruction_overrides_context(): ) assert result == "Response" - call_kwargs = mock_client.converse.call_args.kwargs - assert call_kwargs["system"] == [{"text": "New system instruction"}] - assert call_kwargs["messages"] == test_messages + # Verify the adapter was called with the correct system_instruction + mock_adapter.get_llm_invocation_params.assert_called_once_with( + mock_context, system_instruction="New system instruction" + ) @pytest.mark.asyncio