From f9c23074c1fd9d109f89e59283b397a2894d1fde Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Wed, 11 Dec 2024 16:00:20 -0800 Subject: [PATCH 1/7] feat(bedrock/): add bedrock converse top k param Closes https://github.com/BerriAI/litellm/issues/7087 --- .../bedrock/chat/converse_transformation.py | 20 ++++++++++++++++--- litellm/types/llms/bedrock.py | 1 + .../test_bedrock_completion.py | 19 ++++++++++++++++-- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/litellm/llms/bedrock/chat/converse_transformation.py b/litellm/llms/bedrock/chat/converse_transformation.py index 34c8b43657da..8ad7dc568e83 100644 --- a/litellm/llms/bedrock/chat/converse_transformation.py +++ b/litellm/llms/bedrock/chat/converse_transformation.py @@ -12,6 +12,10 @@ import litellm from litellm.litellm_core_utils.core_helpers import map_finish_reason from litellm.litellm_core_utils.litellm_logging import Logging +from litellm.litellm_core_utils.prompt_templates.factory import ( + _bedrock_converse_messages_pt, + _bedrock_tools_pt, +) from litellm.types.llms.bedrock import * from litellm.types.llms.openai import ( AllMessageValues, @@ -24,7 +28,6 @@ from litellm.types.utils import ModelResponse, Usage from litellm.utils import CustomStreamWrapper, add_dummy_tool, has_tool_call_blocks -from litellm.litellm_core_utils.prompt_templates.factory import _bedrock_converse_messages_pt, _bedrock_tools_pt from ..common_utils import BedrockError, get_bedrock_tool_name @@ -38,6 +41,7 @@ class AmazonConverseConfig: stopSequences: Optional[List[str]] temperature: Optional[int] topP: Optional[int] + topK: Optional[int] def __init__( self, @@ -45,6 +49,7 @@ def __init__( stopSequences: Optional[List[str]] = None, temperature: Optional[int] = None, topP: Optional[int] = None, + topK: Optional[int] = None, ) -> None: locals_ = locals() for key, value in locals_.items(): @@ -309,6 +314,11 @@ def _transform_system_message( messages.pop(idx) return messages, system_content_blocks + def _transform_inference_params(self, inference_params: dict) -> InferenceConfig: + if "top_k" in inference_params: + inference_params["topK"] = inference_params.pop("top_k") + return InferenceConfig(**inference_params) + def _transform_request( self, model: str, @@ -320,7 +330,9 @@ def _transform_request( inference_params = copy.deepcopy(optional_params) additional_request_keys = [] additional_request_params = {} - supported_converse_params = AmazonConverseConfig.__annotations__.keys() + supported_converse_params = list( + AmazonConverseConfig.__annotations__.keys() + ) + ["top_k"] supported_tool_call_params = ["tools", "tool_choice"] supported_guardrail_params = ["guardrailConfig"] inference_params.pop("json_mode", None) # used for handling json_schema @@ -363,7 +375,9 @@ def _transform_request( "messages": bedrock_messages, "additionalModelRequestFields": additional_request_params, "system": system_content_blocks, - "inferenceConfig": InferenceConfig(**inference_params), + "inferenceConfig": self._transform_inference_params( + inference_params=inference_params + ), } # Guardrail Config diff --git a/litellm/types/llms/bedrock.py b/litellm/types/llms/bedrock.py index e94ffd80a3b1..8d43243dcb2c 100644 --- a/litellm/types/llms/bedrock.py +++ b/litellm/types/llms/bedrock.py @@ -136,6 +136,7 @@ class InferenceConfig(TypedDict, total=False): stopSequences: List[str] temperature: float topP: float + topK: int class ToolBlockDeltaEvent(TypedDict): diff --git a/tests/llm_translation/test_bedrock_completion.py b/tests/llm_translation/test_bedrock_completion.py index 651424b313f0..dd33e4045cae 100644 --- a/tests/llm_translation/test_bedrock_completion.py +++ b/tests/llm_translation/test_bedrock_completion.py @@ -1262,7 +1262,9 @@ def test_bedrock_get_base_model(model, expected_base_model): assert litellm.AmazonConverseConfig()._get_base_model(model) == expected_base_model -from litellm.litellm_core_utils.prompt_templates.factory import _bedrock_converse_messages_pt +from litellm.litellm_core_utils.prompt_templates.factory import ( + _bedrock_converse_messages_pt, +) def test_bedrock_converse_translation_tool_message(): @@ -1603,7 +1605,9 @@ def test_bedrock_completion_test_3(): Check if content in tool result is formatted correctly """ from litellm.types.utils import ChatCompletionMessageToolCall, Function, Message - from litellm.litellm_core_utils.prompt_templates.factory import _bedrock_converse_messages_pt + from litellm.litellm_core_utils.prompt_templates.factory import ( + _bedrock_converse_messages_pt, + ) messages = [ { @@ -2106,3 +2110,14 @@ def get_base_rerank_call_args(self) -> dict: return { "model": "bedrock/arn:aws:bedrock:us-west-2::foundation-model/amazon.rerank-v1:0", } + + +@pytest.mark.parametrize("top_k_param", ["top_k", "topK"]) +def test_bedrock_nova_topk(top_k_param): + litellm.set_verbose = True + data = { + "model": "bedrock/us.amazon.nova-pro-v1:0", + "messages": [{"role": "user", "content": "Hello, world!"}], + top_k_param: 10, + } + litellm.completion(**data) From 8a7bb97b42c981317239d699696946e6da05f388 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Thu, 12 Dec 2024 01:02:58 +0100 Subject: [PATCH 2/7] Fix bedrock empty content error (#7177) * add resolver * handle empty content on bedrock with default content * use existing default message, tests * Update tests/llm_translation/test_bedrock_completion.py * fix tests * Revert "add resolver" This reverts commit c717e376ee09a7547cc2c7770405f9aebca64d62. * fallback to empty --------- Co-authored-by: Krish Dholakia --- .../prompt_templates/factory.py | 48 +++++++- .../test_bedrock_completion.py | 107 ++++++++++++++++++ 2 files changed, 151 insertions(+), 4 deletions(-) diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index 13b85a3dc2ce..3af385ae617e 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -2474,8 +2474,22 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 for element in messages[msg_i]["content"]: if isinstance(element, dict): if element["type"] == "text": - _part = BedrockContentBlock(text=element["text"]) - _parts.append(_part) + if element["text"].strip(): + _part = BedrockContentBlock(text=element["text"]) + else: + # bedrock requires non-empty content + # insert a default text block if the user provided it, or if modify_params is True + if user_continue_message is not None: + _part = BedrockContentBlock( + text=user_continue_message["content"][0]["text"] + ) + elif litellm.modify_params: + _part = BedrockContentBlock( + text=DEFAULT_USER_CONTINUE_MESSAGE["content"][0]["text"] + ) + else: + _part = BedrockContentBlock(text="") + _parts.append(_part) elif element["type"] == "image_url": if isinstance(element["image_url"], dict): image_url = element["image_url"]["url"] @@ -2494,7 +2508,19 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 _parts.append(_cache_point_block) user_content.extend(_parts) else: - _part = BedrockContentBlock(text=messages[msg_i]["content"]) + if messages[msg_i]["content"].strip(): + # bedrock requires non-empty content + # insert a default text block if the user provided it, or if modify_params is True + if user_continue_message is not None: + _part = BedrockContentBlock( + text=user_continue_message["content"][0]["text"] + ) + elif litellm.modify_params: + _part = BedrockContentBlock( + text=DEFAULT_USER_CONTINUE_MESSAGE["content"][0]["text"] + ) + else: + _part = BedrockContentBlock(text="") _cache_point_block = ( litellm.AmazonConverseConfig()._get_cache_point_block( messages[msg_i], block_type="content_block" @@ -2566,7 +2592,21 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 for element in messages[msg_i]["content"]: if isinstance(element, dict): if element["type"] == "text": - assistants_part = BedrockContentBlock(text=element["text"]) + if element["text"].strip(): + assistants_part = BedrockContentBlock(text=element["text"]) + else: + # bedrock requires non-empty content + # insert a default text block if the user provided it, or if modify_params is True + if assistant_continue_message is not None: + assistants_part = BedrockContentBlock( + text=assistant_continue_message["content"][0]["text"] + ) + elif litellm.modify_params: + assistants_part = BedrockContentBlock( + text=DEFAULT_ASSISTANT_CONTINUE_MESSAGE["content"][0]["text"] + ) + else: + assistants_part = BedrockContentBlock(text="") assistants_parts.append(assistants_part) elif element["type"] == "image_url": if isinstance(element["image_url"], dict): diff --git a/tests/llm_translation/test_bedrock_completion.py b/tests/llm_translation/test_bedrock_completion.py index dd33e4045cae..6333c1956c04 100644 --- a/tests/llm_translation/test_bedrock_completion.py +++ b/tests/llm_translation/test_bedrock_completion.py @@ -2112,6 +2112,112 @@ def get_base_rerank_call_args(self) -> dict: } +def test_bedrock_empty_content_handling(): + """ + Test that empty content in messages is handled correctly with default messages + """ + messages = [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "Hello!" + } + ] + }, + { + "role": "assistant", + "content": [ + { + "type": "text", + "text": "" + } + ] + } + ] + + # Test with default behavior (modify_params=True) + litellm.modify_params = True + formatted_messages = _bedrock_converse_messages_pt( + messages=messages, + model="anthropic.claude-3-sonnet-20240229-v1:0", + llm_provider="bedrock" + ) + + # Verify assistant message with default text was inserted + assert formatted_messages[1]["role"] == "assistant" + assert formatted_messages[1]["content"][0].text == "Please continue." + +def test_bedrock_custom_continue_message(): + """ + Test that custom continue messages are used when provided + """ + messages = [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "Hello!" + } + ] + }, + { + "role": "assistant", + "content": [ + { + "type": "text", + "text": " " + } + ] + } + ] + + custom_continue = { + "role": "assistant", + "content": [ + { + "text": "Custom continue message" + } + ] + } + + formatted_messages = _bedrock_converse_messages_pt( + messages=messages, + model="anthropic.claude-3-sonnet-20240229-v1:0", + llm_provider="bedrock", + assistant_continue_message=custom_continue + ) + + # Verify custom message was used + assert formatted_messages[1]["role"] == "assistant" + assert formatted_messages[1]["content"][0].text == "Custom continue message" + +def test_bedrock_no_default_message(): + """ + Test that empty content is handled correctly when modify_params=False + """ + messages = [ + {"role": "user", "content": "Hello!"}, + {"role": "assistant", "content": ""}, + {"role": "user", "content": "Hi again"}, + {"role": "assistant", "content": "Valid response"} + ] + + litellm.modify_params = False + formatted_messages = _bedrock_converse_messages_pt( + messages=messages, + model="anthropic.claude-3-sonnet-20240229-v1:0", + llm_provider="bedrock" + ) + + # Verify empty message is present and valid message remains + assistant_messages = [msg for msg in formatted_messages if msg["role"] == "assistant"] + assert len(assistant_messages) == 2 # Both empty and valid messages present + assert assistant_messages[0]["content"][0].text == "" # First message is empty + assert assistant_messages[1]["content"][0].text == "Valid response" # Second message is valid + @pytest.mark.parametrize("top_k_param", ["top_k", "topK"]) def test_bedrock_nova_topk(top_k_param): litellm.set_verbose = True @@ -2121,3 +2227,4 @@ def test_bedrock_nova_topk(top_k_param): top_k_param: 10, } litellm.completion(**data) + From b96a1e889dd3cc59e8fde7e19b3ea43fce87a056 Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Wed, 11 Dec 2024 18:21:25 -0800 Subject: [PATCH 3/7] fix(factory.py): handle empty content blocks in messages Fixes https://github.com/BerriAI/litellm/issues/7169 --- .../prompt_templates/common_utils.py | 7 + .../prompt_templates/factory.py | 306 +++++++++++++----- .../bedrock/chat/converse_transformation.py | 29 +- litellm/types/llms/openai.py | 14 +- .../test_bedrock_completion.py | 113 +++---- 5 files changed, 328 insertions(+), 141 deletions(-) diff --git a/litellm/litellm_core_utils/prompt_templates/common_utils.py b/litellm/litellm_core_utils/prompt_templates/common_utils.py index 5291f4082607..370258a66778 100644 --- a/litellm/litellm_core_utils/prompt_templates/common_utils.py +++ b/litellm/litellm_core_utils/prompt_templates/common_utils.py @@ -81,6 +81,13 @@ def convert_content_list_to_str(message: AllMessageValues) -> str: return texts +def is_non_content_values_set(message: AllMessageValues) -> bool: + ignore_keys = ["content", "role", "name"] + return any( + message.get(key, None) is not None for key in message if key not in ignore_keys + ) + + def _audio_or_image_in_message_content(message: AllMessageValues) -> bool: """ Checks if message content contains an image or audio diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index 3af385ae617e..b583d2841ad6 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -41,6 +41,7 @@ ) from litellm.types.utils import GenericImageParsingChunk +from .common_utils import convert_content_list_to_str, is_non_content_values_set from .image_handling import async_convert_url_to_base64, convert_url_to_base64 @@ -61,14 +62,15 @@ def prompt_injection_detection_default_pt(): } # similar to autogen. Only used if `litellm.modify_params=True`. # used to interweave assistant messages, to ensure user/assistant alternating -DEFAULT_ASSISTANT_CONTINUE_MESSAGE = { - "role": "assistant", - "content": [ +DEFAULT_ASSISTANT_CONTINUE_MESSAGE = ChatCompletionAssistantMessage( + role="assistant", + content=[ { + "type": "text", "text": "Please continue.", } ], -} # similar to autogen. Only used if `litellm.modify_params=True`. +) # similar to autogen. Only used if `litellm.modify_params=True`. def map_system_message_pt(messages: list) -> list: @@ -2404,7 +2406,9 @@ def _convert_to_bedrock_tool_call_result( def _insert_assistant_continue_message( messages: List[BedrockMessageBlock], - assistant_continue_message: Optional[str] = None, + assistant_continue_message: Optional[ + Union[str, ChatCompletionAssistantMessage] + ] = None, ) -> List[BedrockMessageBlock]: """ Add dummy message between user/tool result blocks. @@ -2412,23 +2416,216 @@ def _insert_assistant_continue_message( Conversation blocks and tool result blocks cannot be provided in the same turn. Issue: https://github.com/BerriAI/litellm/issues/6053 """ if assistant_continue_message is not None: + if isinstance(assistant_continue_message, str): + messages.append( + BedrockMessageBlock( + role="assistant", + content=[BedrockContentBlock(text=assistant_continue_message)], + ) + ) + elif isinstance(assistant_continue_message, dict): + text = convert_content_list_to_str(assistant_continue_message) + messages.append( + BedrockMessageBlock( + role="assistant", + content=[BedrockContentBlock(text=text)], + ) + ) + elif litellm.modify_params: + text = convert_content_list_to_str( + cast(ChatCompletionAssistantMessage, DEFAULT_ASSISTANT_CONTINUE_MESSAGE) + ) messages.append( BedrockMessageBlock( role="assistant", - content=[BedrockContentBlock(text=assistant_continue_message)], + content=[ + BedrockContentBlock(text=text), + ], ) ) - elif litellm.modify_params: - messages.append(BedrockMessageBlock(**DEFAULT_ASSISTANT_CONTINUE_MESSAGE)) # type: ignore return messages +def get_user_message_block_or_continue_message( + message: ChatCompletionUserMessage, + user_continue_message: Optional[ChatCompletionUserMessage] = None, +) -> ChatCompletionUserMessage: + """ + Returns the user content block + if content block is an empty string, then return the default continue message + + Relevant Issue: https://github.com/BerriAI/litellm/issues/7169 + """ + content_block = message.get("content", None) + + # Handle None case + if content_block is None or ( + user_continue_message is None and litellm.modify_params is False + ): + return message + + # Handle string case + if isinstance(content_block, str): + # check if content is empty + if content_block.strip(): + return message + else: + return ChatCompletionUserMessage( + **(user_continue_message or DEFAULT_USER_CONTINUE_MESSAGE) # type: ignore + ) + + # Handle list case + if isinstance(content_block, list): + """ + CHECK FOR + "content": [ + { + "type": "text", + "text": "" + } + ], + """ + if not content_block: + return ChatCompletionUserMessage( + **(user_continue_message or DEFAULT_USER_CONTINUE_MESSAGE) # type: ignore + ) + # Create a copy of the message to avoid modifying the original + modified_content_block = content_block.copy() + + for item in modified_content_block: + # Check if the list is empty + if item["type"] == "text": + + if not item["text"].strip(): + # Replace empty text with continue message + _user_continue_message = ChatCompletionUserMessage( + **(user_continue_message or DEFAULT_USER_CONTINUE_MESSAGE) # type: ignore + ) + text = convert_content_list_to_str(_user_continue_message) + item["text"] = text + break + modified_message = message.copy() + modified_message["content"] = modified_content_block + return modified_message + + # Handle unsupported type + raise ValueError(f"Unsupported content type: {type(content_block)}") + + +def return_assistant_continue_message( + assistant_continue_message: Optional[ + Union[str, ChatCompletionAssistantMessage] + ] = None +) -> ChatCompletionAssistantMessage: + if assistant_continue_message and isinstance(assistant_continue_message, str): + return ChatCompletionAssistantMessage( + role="assistant", + content=assistant_continue_message, + ) + elif assistant_continue_message and isinstance(assistant_continue_message, dict): + return ChatCompletionAssistantMessage(**assistant_continue_message) + else: + return DEFAULT_ASSISTANT_CONTINUE_MESSAGE + + +def process_empty_text_blocks( + message: ChatCompletionAssistantMessage, + assistant_continue_message: Optional[ + Union[str, ChatCompletionAssistantMessage] + ] = None, +) -> ChatCompletionAssistantMessage: + modified_content_block = message.get("content", None) + ## BASE CASE ## + if modified_content_block is None or not isinstance(modified_content_block, list): + return message + + # Check if all items are empty text blocks + if all( + item["type"] == "text" and not item["text"].strip() + for item in modified_content_block + ): + # Replace with a single continue message + _assistant_continue_message = return_assistant_continue_message( + assistant_continue_message + ) + modified_content_block = [ + { + "type": "text", + "text": convert_content_list_to_str(_assistant_continue_message), + } + ] + else: + # Filter out only empty text blocks, keeping non-empty text and other block types + modified_content_block = [ + item + for item in modified_content_block + if not (item["type"] == "text" and not item["text"].strip()) + ] + + modified_message = message.copy() + modified_message["content"] = modified_content_block + return modified_message + + +def get_assistant_message_block_or_continue_message( + message: ChatCompletionAssistantMessage, + assistant_continue_message: Optional[ + Union[str, ChatCompletionAssistantMessage] + ] = None, +) -> ChatCompletionAssistantMessage: + """ + Returns the user content block + if content block is an empty string, then return the default continue message + + Relevant Issue: https://github.com/BerriAI/litellm/issues/7169 + """ + content_block = message.get("content", None) + + # Handle Base case + if content_block is None or ( + assistant_continue_message is None and litellm.modify_params is False + ): + return message + + # Handle string case + if isinstance(content_block, str): + # check if content is empty + if content_block.strip(): + return message + else: + if is_non_content_values_set(message): + modified_message = message.copy() + modified_message["content"] = None + return modified_message + return return_assistant_continue_message(assistant_continue_message) + + # Handle list case + if isinstance(content_block, list): + """ + CHECK FOR + "content": [ + { + "type": "text", + "text": "" + } + ], + """ + return process_empty_text_blocks( + message=message, assistant_continue_message=assistant_continue_message + ) + + # Handle unsupported type + raise ValueError(f"Unsupported content type: {type(content_block)}") + + def _bedrock_converse_messages_pt( # noqa: PLR0915 messages: List, model: str, llm_provider: str, - user_continue_message: Optional[dict] = None, - assistant_continue_message: Optional[str] = None, + user_continue_message: Optional[ChatCompletionUserMessage] = None, + assistant_continue_message: Optional[ + Union[str, ChatCompletionAssistantMessage] + ] = None, ) -> List[BedrockMessageBlock]: """ Converts given messages from OpenAI format to Bedrock format @@ -2469,27 +2666,17 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 init_msg_i = msg_i ## MERGE CONSECUTIVE USER CONTENT ## while msg_i < len(messages) and messages[msg_i]["role"] == "user": - if isinstance(messages[msg_i]["content"], list): + message_block = get_user_message_block_or_continue_message( + message=messages[msg_i], + user_continue_message=user_continue_message, + ) + if isinstance(message_block["content"], list): _parts: List[BedrockContentBlock] = [] - for element in messages[msg_i]["content"]: + for element in message_block["content"]: if isinstance(element, dict): if element["type"] == "text": - if element["text"].strip(): - _part = BedrockContentBlock(text=element["text"]) - else: - # bedrock requires non-empty content - # insert a default text block if the user provided it, or if modify_params is True - if user_continue_message is not None: - _part = BedrockContentBlock( - text=user_continue_message["content"][0]["text"] - ) - elif litellm.modify_params: - _part = BedrockContentBlock( - text=DEFAULT_USER_CONTINUE_MESSAGE["content"][0]["text"] - ) - else: - _part = BedrockContentBlock(text="") - _parts.append(_part) + _part = BedrockContentBlock(text=element["text"]) + _parts.append(_part) elif element["type"] == "image_url": if isinstance(element["image_url"], dict): image_url = element["image_url"]["url"] @@ -2507,23 +2694,11 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 if _cache_point_block is not None: _parts.append(_cache_point_block) user_content.extend(_parts) - else: - if messages[msg_i]["content"].strip(): - # bedrock requires non-empty content - # insert a default text block if the user provided it, or if modify_params is True - if user_continue_message is not None: - _part = BedrockContentBlock( - text=user_continue_message["content"][0]["text"] - ) - elif litellm.modify_params: - _part = BedrockContentBlock( - text=DEFAULT_USER_CONTINUE_MESSAGE["content"][0]["text"] - ) - else: - _part = BedrockContentBlock(text="") + elif message_block["content"] and isinstance(message_block["content"], str): + _part = BedrockContentBlock(text=messages[msg_i]["content"]) _cache_point_block = ( litellm.AmazonConverseConfig()._get_cache_point_block( - messages[msg_i], block_type="content_block" + message_block, block_type="content_block" ) ) user_content.append(_part) @@ -2585,28 +2760,18 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 assistant_content: List[BedrockContentBlock] = [] ## MERGE CONSECUTIVE ASSISTANT CONTENT ## while msg_i < len(messages) and messages[msg_i]["role"] == "assistant": - if messages[msg_i].get("content", None) is not None and isinstance( - messages[msg_i]["content"], list - ): + assistant_message_block = get_assistant_message_block_or_continue_message( + message=messages[msg_i], + assistant_continue_message=assistant_continue_message, + ) + _assistant_content = assistant_message_block.get("content", None) + + if _assistant_content is not None and isinstance(_assistant_content, list): assistants_parts: List[BedrockContentBlock] = [] - for element in messages[msg_i]["content"]: + for element in _assistant_content: if isinstance(element, dict): if element["type"] == "text": - if element["text"].strip(): - assistants_part = BedrockContentBlock(text=element["text"]) - else: - # bedrock requires non-empty content - # insert a default text block if the user provided it, or if modify_params is True - if assistant_continue_message is not None: - assistants_part = BedrockContentBlock( - text=assistant_continue_message["content"][0]["text"] - ) - elif litellm.modify_params: - assistants_part = BedrockContentBlock( - text=DEFAULT_ASSISTANT_CONTINUE_MESSAGE["content"][0]["text"] - ) - else: - assistants_part = BedrockContentBlock(text="") + assistants_part = BedrockContentBlock(text=element["text"]) assistants_parts.append(assistants_part) elif element["type"] == "image_url": if isinstance(element["image_url"], dict): @@ -2618,19 +2783,12 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 ) assistants_parts.append(assistants_part) assistant_content.extend(assistants_parts) - elif messages[msg_i].get("content", None) is not None and isinstance( - messages[msg_i]["content"], str - ): - assistant_text = ( - messages[msg_i].get("content") or "" - ) # either string or none - if assistant_text: - assistant_content.append(BedrockContentBlock(text=assistant_text)) - if messages[msg_i].get( - "tool_calls", [] - ): # support assistant tool invoke convertion [TODO]: + elif _assistant_content is not None and isinstance(_assistant_content, str): + assistant_content.append(BedrockContentBlock(text=_assistant_content)) + _tool_calls = assistant_message_block.get("tool_calls", []) + if _tool_calls: assistant_content.extend( - _convert_to_bedrock_tool_call_invoke(messages[msg_i]["tool_calls"]) + _convert_to_bedrock_tool_call_invoke(_tool_calls) ) msg_i += 1 diff --git a/litellm/llms/bedrock/chat/converse_transformation.py b/litellm/llms/bedrock/chat/converse_transformation.py index 8ad7dc568e83..b1f6ef3da94e 100644 --- a/litellm/llms/bedrock/chat/converse_transformation.py +++ b/litellm/llms/bedrock/chat/converse_transformation.py @@ -20,10 +20,13 @@ from litellm.types.llms.openai import ( AllMessageValues, ChatCompletionResponseMessage, + ChatCompletionSystemMessage, ChatCompletionToolCallChunk, ChatCompletionToolCallFunctionChunk, ChatCompletionToolParam, ChatCompletionToolParamFunctionChunk, + ChatCompletionUserMessage, + OpenAIMessageContentListBlock, ) from litellm.types.utils import ModelResponse, Usage from litellm.utils import CustomStreamWrapper, add_dummy_tool, has_tool_call_blocks @@ -263,18 +266,36 @@ def map_openai_params( @overload def _get_cache_point_block( - self, message_block: dict, block_type: Literal["system"] + self, + message_block: Union[ + OpenAIMessageContentListBlock, + ChatCompletionUserMessage, + ChatCompletionSystemMessage, + ], + block_type: Literal["system"], ) -> Optional[SystemContentBlock]: pass @overload def _get_cache_point_block( - self, message_block: dict, block_type: Literal["content_block"] + self, + message_block: Union[ + OpenAIMessageContentListBlock, + ChatCompletionUserMessage, + ChatCompletionSystemMessage, + ], + block_type: Literal["content_block"], ) -> Optional[ContentBlock]: pass def _get_cache_point_block( - self, message_block: dict, block_type: Literal["system", "content_block"] + self, + message_block: Union[ + OpenAIMessageContentListBlock, + ChatCompletionUserMessage, + ChatCompletionSystemMessage, + ], + block_type: Literal["system", "content_block"], ) -> Optional[Union[SystemContentBlock, ContentBlock]]: if message_block.get("cache_control", None) is None: return None @@ -295,7 +316,7 @@ def _transform_system_message( if isinstance(message["content"], str) and len(message["content"]) > 0: _system_content_block = SystemContentBlock(text=message["content"]) _cache_point_block = self._get_cache_point_block( - cast(dict, message), block_type="system" + message, block_type="system" ) elif isinstance(message["content"], list): for m in message["content"]: diff --git a/litellm/types/llms/openai.py b/litellm/types/llms/openai.py index eb8f308ef083..26c0eab3a093 100644 --- a/litellm/types/llms/openai.py +++ b/litellm/types/llms/openai.py @@ -374,15 +374,15 @@ class ChatCompletionAudioObject(ChatCompletionContentPartInputAudioParam): pass +OpenAIMessageContentListBlock = Union[ + ChatCompletionTextObject, + ChatCompletionImageObject, + ChatCompletionAudioObject, +] + OpenAIMessageContent = Union[ str, - Iterable[ - Union[ - ChatCompletionTextObject, - ChatCompletionImageObject, - ChatCompletionAudioObject, - ] - ], + Iterable[OpenAIMessageContentListBlock], ] # The prompt(s) to generate completions for, encoded as a string, array of strings, array of tokens, or array of token arrays. diff --git a/tests/llm_translation/test_bedrock_completion.py b/tests/llm_translation/test_bedrock_completion.py index 6333c1956c04..6a59d813b907 100644 --- a/tests/llm_translation/test_bedrock_completion.py +++ b/tests/llm_translation/test_bedrock_completion.py @@ -1629,7 +1629,7 @@ def test_bedrock_completion_test_3(): ) ], function_call=None, - ), + ).model_dump(), { "tool_call_id": "tooluse_EF8PwJ1dSMSh6tLGKu9VdA", "role": "tool", @@ -2112,87 +2112,71 @@ def get_base_rerank_call_args(self) -> dict: } -def test_bedrock_empty_content_handling(): +@pytest.mark.parametrize( + "messages, continue_message_index", + [ + ( + [ + {"role": "user", "content": [{"type": "text", "text": ""}]}, + {"role": "assistant", "content": [{"type": "text", "text": "Hello!"}]}, + ], + 0, + ), + ( + [ + {"role": "user", "content": [{"type": "text", "text": "Hello!"}]}, + {"role": "assistant", "content": [{"type": "text", "text": " "}]}, + ], + 1, + ), + ], +) +def test_bedrock_empty_content_handling(messages, continue_message_index): """ Test that empty content in messages is handled correctly with default messages """ - messages = [ - { - "role": "user", - "content": [ - { - "type": "text", - "text": "Hello!" - } - ] - }, - { - "role": "assistant", - "content": [ - { - "type": "text", - "text": "" - } - ] - } - ] - # Test with default behavior (modify_params=True) litellm.modify_params = True formatted_messages = _bedrock_converse_messages_pt( messages=messages, model="anthropic.claude-3-sonnet-20240229-v1:0", - llm_provider="bedrock" + llm_provider="bedrock", ) - + print(formatted_messages) # Verify assistant message with default text was inserted + assert formatted_messages[0]["role"] == "user" assert formatted_messages[1]["role"] == "assistant" - assert formatted_messages[1]["content"][0].text == "Please continue." + assert ( + formatted_messages[continue_message_index]["content"][0]["text"] + == "Please continue." + ) + def test_bedrock_custom_continue_message(): """ Test that custom continue messages are used when provided """ messages = [ - { - "role": "user", - "content": [ - { - "type": "text", - "text": "Hello!" - } - ] - }, - { - "role": "assistant", - "content": [ - { - "type": "text", - "text": " " - } - ] - } + {"role": "user", "content": [{"type": "text", "text": "Hello!"}]}, + {"role": "assistant", "content": [{"type": "text", "text": " "}]}, ] custom_continue = { "role": "assistant", - "content": [ - { - "text": "Custom continue message" - } - ] + "content": [{"text": "Custom continue message", "type": "text"}], } formatted_messages = _bedrock_converse_messages_pt( messages=messages, model="anthropic.claude-3-sonnet-20240229-v1:0", llm_provider="bedrock", - assistant_continue_message=custom_continue + assistant_continue_message=custom_continue, ) # Verify custom message was used assert formatted_messages[1]["role"] == "assistant" - assert formatted_messages[1]["content"][0].text == "Custom continue message" + assert formatted_messages[1]["content"][0]["text"] == "Custom continue message" + def test_bedrock_no_default_message(): """ @@ -2202,21 +2186,26 @@ def test_bedrock_no_default_message(): {"role": "user", "content": "Hello!"}, {"role": "assistant", "content": ""}, {"role": "user", "content": "Hi again"}, - {"role": "assistant", "content": "Valid response"} + {"role": "assistant", "content": "Valid response"}, ] litellm.modify_params = False formatted_messages = _bedrock_converse_messages_pt( messages=messages, model="anthropic.claude-3-sonnet-20240229-v1:0", - llm_provider="bedrock" + llm_provider="bedrock", ) # Verify empty message is present and valid message remains - assistant_messages = [msg for msg in formatted_messages if msg["role"] == "assistant"] + assistant_messages = [ + msg for msg in formatted_messages if msg["role"] == "assistant" + ] assert len(assistant_messages) == 2 # Both empty and valid messages present - assert assistant_messages[0]["content"][0].text == "" # First message is empty - assert assistant_messages[1]["content"][0].text == "Valid response" # Second message is valid + assert assistant_messages[0]["content"][0]["text"] == "" # First message is empty + assert ( + assistant_messages[1]["content"][0]["text"] == "Valid response" + ) # Second message is valid + @pytest.mark.parametrize("top_k_param", ["top_k", "topK"]) def test_bedrock_nova_topk(top_k_param): @@ -2228,3 +2217,15 @@ def test_bedrock_nova_topk(top_k_param): } litellm.completion(**data) + +def test_bedrock_process_empty_text_blocks(): + from litellm.litellm_core_utils.prompt_templates.factory import ( + process_empty_text_blocks, + ) + + message = { + "message": {"role": "assistant", "content": [{"type": "text", "text": " "}]}, + "assistant_continue_message": None, + } + modified_message = process_empty_text_blocks(**message) + assert modified_message["content"][0]["text"] == "Please continue." From 700581eea88a127236d98757cf89bbc924bc1e4e Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Wed, 11 Dec 2024 20:01:28 -0800 Subject: [PATCH 4/7] feat(router.py): add stripped model check to model fallback search if model_name="openai/gpt-3.5-turbo" and fallback=[{"gpt-3.5-turbo"..}] the fallback should just work as expected --- litellm/router.py | 20 +++--- .../router_utils/fallback_event_handlers.py | 67 +++++++++++++++++++ tests/local_testing/test_router_fallbacks.py | 46 +++++++++++++ 3 files changed, 121 insertions(+), 12 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index 2f333bf6b38b..dc1435444e2e 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -79,6 +79,7 @@ _set_cooldown_deployments, ) from litellm.router_utils.fallback_event_handlers import ( + get_fallback_model_group, log_failure_fallback_event, log_success_fallback_event, run_async_fallback, @@ -2754,19 +2755,14 @@ async def async_function_with_fallbacks(self, *args, **kwargs): # noqa: PLR0915 ) e.message += "\n{}".format(error_message) - if fallbacks is not None: + if fallbacks is not None and model_group is not None: verbose_router_logger.debug(f"inside model fallbacks: {fallbacks}") - generic_fallback_idx: Optional[int] = None - ## check for specific model group-specific fallbacks - for idx, item in enumerate(fallbacks): - if isinstance(item, dict): - if list(item.keys())[0] == model_group: - fallback_model_group = item[model_group] - break - elif list(item.keys())[0] == "*": - generic_fallback_idx = idx - elif isinstance(item, str): - fallback_model_group = [fallbacks.pop(idx)] + fallback_model_group, generic_fallback_idx = ( + get_fallback_model_group( + fallbacks=fallbacks, + model_group=cast(str, model_group), + ) + ) ## if none, check for generic fallback if ( fallback_model_group is None diff --git a/litellm/router_utils/fallback_event_handlers.py b/litellm/router_utils/fallback_event_handlers.py index 41c3080e9a07..2845ec4769b1 100644 --- a/litellm/router_utils/fallback_event_handlers.py +++ b/litellm/router_utils/fallback_event_handlers.py @@ -1,6 +1,8 @@ +from enum import Enum from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple import litellm +from litellm import LlmProviders from litellm._logging import verbose_router_logger from litellm.integrations.custom_logger import CustomLogger from litellm.main import verbose_logger @@ -13,6 +15,71 @@ LitellmRouter = Any +def _check_stripped_model_group(model_group: str, fallback_key: str) -> bool: + """ + Handles wildcard routing scenario + + where fallbacks set like: + [{"gpt-3.5-turbo": ["claude-3-haiku"]}] + + but model_group is like: + "openai/gpt-3.5-turbo" + + Returns: + - True if the stripped model group == fallback_key + """ + for provider in litellm.provider_list: + if isinstance(provider, Enum): + _provider = provider.value + else: + _provider = provider + if model_group.startswith(f"{_provider}/"): + stripped_model_group = model_group.replace(f"{_provider}/", "") + if stripped_model_group == fallback_key: + return True + return False + + +def get_fallback_model_group( + fallbacks: List[Any], model_group: str +) -> Tuple[Optional[List[str]], Optional[int]]: + """ + Returns: + - fallback_model_group: List[str] of fallback model groups. example: ["gpt-4", "gpt-3.5-turbo"] + - generic_fallback_idx: int of the index of the generic fallback in the fallbacks list. + + Checks: + - exact match + - stripped model group match + - generic fallback + """ + generic_fallback_idx: Optional[int] = None + stripped_model_fallback: Optional[List[str]] = None + fallback_model_group: Optional[List[str]] = None + ## check for specific model group-specific fallbacks + for idx, item in enumerate(fallbacks): + if isinstance(item, dict): + if list(item.keys())[0] == model_group: # check exact match + fallback_model_group = item[model_group] + break + elif _check_stripped_model_group( + model_group=model_group, fallback_key=list(item.keys())[0] + ): # check generic fallback + stripped_model_fallback = item[list(item.keys())[0]] + elif list(item.keys())[0] == "*": # check generic fallback + generic_fallback_idx = idx + elif isinstance(item, str): + fallback_model_group = [fallbacks.pop(idx)] + ## if none, check for generic fallback + if fallback_model_group is None: + if stripped_model_fallback is not None: + fallback_model_group = stripped_model_fallback + elif generic_fallback_idx is not None: + fallback_model_group = fallbacks[generic_fallback_idx]["*"] + + return fallback_model_group, generic_fallback_idx + + async def run_async_fallback( *args: Tuple[Any], litellm_router: LitellmRouter, diff --git a/tests/local_testing/test_router_fallbacks.py b/tests/local_testing/test_router_fallbacks.py index e555b51d726f..41b2b9c9cd5f 100644 --- a/tests/local_testing/test_router_fallbacks.py +++ b/tests/local_testing/test_router_fallbacks.py @@ -1521,3 +1521,49 @@ def test_router_fallbacks_with_model_id(): messages=[{"role": "user", "content": "hi"}], mock_testing_fallbacks=True, ) + + +def test_router_fallbacks_with_wildcard_model_name(): + router = Router( + model_list=[ + { + "model_name": "openai/*", + "litellm_params": { + "model": "openai/*", + "api_key": os.getenv("OPENAI_API_KEY"), + }, + }, + { + "model_name": "claude-3-haiku", + "litellm_params": { + "model": "claude-3-haiku-20240307", + "api_key": os.getenv("ANTHROPIC_API_KEY"), + "mock_response": "Hi this is claude!", + }, + }, + ], + fallbacks=[{"gpt-3.5-turbo": ["claude-3-haiku"]}], + ) + + response = router.completion( + model="openai/gpt-3.5-turbo", + messages=[{"role": "user", "content": "Hey, how's it going?"}], + mock_testing_fallbacks=True, + ) + + print(response) + assert response["choices"][0]["message"]["content"] == "Hi this is claude!" + + +def test_get_fallback_model_group(): + from litellm.router_utils.fallback_event_handlers import get_fallback_model_group + + args = { + "fallbacks": [ + {"gpt-3.5-turbo": ["claude-3-haiku"]}, + {"*": ["claude-3-sonnet"]}, + ], + "model_group": "openai/gpt-3.5-turbo", + } + fallback_model_group, _ = get_fallback_model_group(**args) + assert fallback_model_group == ["claude-3-haiku"] From f43810c91bf68b989b9b98c2b6acb5a0590d59de Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Thu, 12 Dec 2024 10:57:29 -0800 Subject: [PATCH 5/7] fix: fix linting error --- litellm/litellm_core_utils/prompt_templates/factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index b583d2841ad6..ffc6bfb09363 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -2688,7 +2688,7 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 _parts.append(_part) # type: ignore _cache_point_block = ( litellm.AmazonConverseConfig()._get_cache_point_block( - element, block_type="content_block" + message_block=element, block_type="content_block" ) ) if _cache_point_block is not None: From 6b7c350f4848018e50028d45ace0b8c7b412c1d5 Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Thu, 12 Dec 2024 11:09:08 -0800 Subject: [PATCH 6/7] fix(factory.py): fix linting error --- litellm/litellm_core_utils/prompt_templates/factory.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index ffc6bfb09363..06a2c1abb871 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -38,6 +38,7 @@ ChatCompletionToolCallFunctionChunk, ChatCompletionToolMessage, ChatCompletionUserMessage, + OpenAIMessageContentListBlock, ) from litellm.types.utils import GenericImageParsingChunk @@ -2688,7 +2689,10 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 _parts.append(_part) # type: ignore _cache_point_block = ( litellm.AmazonConverseConfig()._get_cache_point_block( - message_block=element, block_type="content_block" + message_block=cast( + OpenAIMessageContentListBlock, element + ), + block_type="content_block", ) ) if _cache_point_block is not None: From dcf1806a7d2e2bdcc38b44a441f171347f97c515 Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Thu, 12 Dec 2024 21:05:43 -0800 Subject: [PATCH 7/7] fix(factory.py): in base case still support skip empty text blocks --- .../prompt_templates/factory.py | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index 06a2c1abb871..9877c683c92f 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -2529,6 +2529,38 @@ def return_assistant_continue_message( return DEFAULT_ASSISTANT_CONTINUE_MESSAGE +def skip_empty_text_blocks( + message: ChatCompletionAssistantMessage, +) -> ChatCompletionAssistantMessage: + """ + Skips empty text blocks in message content text blocks. + + Do not insert content here. This is a helper function, which can also be used in base case. + """ + content_block = message.get("content", None) + if content_block is None: + return message + if ( + isinstance(content_block, str) + and not content_block.strip() + and is_non_content_values_set(message) + ): + modified_message = message.copy() + modified_message["content"] = None + return modified_message + elif isinstance(content_block, list): + modified_content_block = [ + item + for item in content_block + if not (item["type"] == "text" and not item["text"].strip()) + ] + modified_message = message.copy() + modified_message["content"] = modified_content_block + return modified_message + + return message + + def process_empty_text_blocks( message: ChatCompletionAssistantMessage, assistant_continue_message: Optional[ @@ -2586,7 +2618,7 @@ def get_assistant_message_block_or_continue_message( if content_block is None or ( assistant_continue_message is None and litellm.modify_params is False ): - return message + return skip_empty_text_blocks(message=message) # Handle string case if isinstance(content_block, str):