From 3262d3ff484beaa840f044499937d70eed8935ff Mon Sep 17 00:00:00 2001 From: Federico Kamelhar Date: Mon, 6 Apr 2026 22:36:58 -0400 Subject: [PATCH] fix(oci): address PR review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - types/llms/oci.py: add "TOOL_CALL" to CohereChatResponse.finishReason Literal so Pydantic does not raise ValidationError on non-streaming Cohere tool-use calls (Greptile P1) - test_oci_cohere_tool_calls.py: add test covering TOOL_CALL finish reason - model_prices_and_context_window.json: remove 6 duplicate oci/cohere.embed-* keys that were silently overridden by the more complete entries already present in the file (Greptile P1) - common_utils.py: move OCI_API_VERSION here from chat/transformation.py so embed/transformation.py does not need to import chat/transformation; change Protocol stub body from ... to pass (CodeQL "statement no effect"); add comment to sha256_base64 clarifying it implements OCI HTTP signing spec, not password hashing (CodeQL false positive) - chat/transformation.py: import CustomStreamWrapper from litellm_core_utils.streaming_handler instead of litellm.utils to reduce import cycle depth (CodeQL cyclic import) - chat/cohere.py, chat/generic.py: import Usage and ChatCompletionMessageToolCall from litellm.types.utils instead of litellm.utils for the same reason - embed/transformation.py: import OCI_API_VERSION from common_utils instead of chat/transformation (removes the embed→chat import edge) --- litellm/llms/oci/chat/cohere.py | 2 +- litellm/llms/oci/chat/generic.py | 2 +- litellm/llms/oci/chat/transformation.py | 6 +-- litellm/llms/oci/common_utils.py | 11 +++- litellm/llms/oci/embed/transformation.py | 2 +- litellm/types/llms/oci.py | 8 ++- model_prices_and_context_window.json | 50 ------------------- .../oci/chat/test_oci_cohere_tool_calls.py | 46 +++++++++++++++++ 8 files changed, 68 insertions(+), 59 deletions(-) diff --git a/litellm/llms/oci/chat/cohere.py b/litellm/llms/oci/chat/cohere.py index 8bc97ddf9e2..62175a36ba6 100644 --- a/litellm/llms/oci/chat/cohere.py +++ b/litellm/llms/oci/chat/cohere.py @@ -37,7 +37,7 @@ from litellm.types.utils import ( ModelResponseStream, StreamingChoices, ) -from litellm.utils import Usage +from litellm.types.utils import Usage def _extract_text_content(content: Any) -> str: diff --git a/litellm/llms/oci/chat/generic.py b/litellm/llms/oci/chat/generic.py index 07811d59327..42fcbcf7eb9 100644 --- a/litellm/llms/oci/chat/generic.py +++ b/litellm/llms/oci/chat/generic.py @@ -38,7 +38,7 @@ from litellm.types.utils import ( ModelResponseStream, StreamingChoices, ) -from litellm.utils import ChatCompletionMessageToolCall, Usage +from litellm.types.utils import ChatCompletionMessageToolCall, Usage # Maps OpenAI role names to OCI GENERIC role names. open_ai_to_generic_oci_role_map: Dict[str, OCIRoles] = { diff --git a/litellm/llms/oci/chat/transformation.py b/litellm/llms/oci/chat/transformation.py index 9ddda9ed699..22163cb126f 100644 --- a/litellm/llms/oci/chat/transformation.py +++ b/litellm/llms/oci/chat/transformation.py @@ -48,6 +48,7 @@ from litellm.llms.oci.chat.generic import ( handle_generic_stream_chunk, ) from litellm.llms.oci.common_utils import ( + OCI_API_VERSION, OCIError, OCIRequestWrapper, # re-exported for backwards compatibility get_oci_base_url, @@ -68,7 +69,7 @@ from litellm.types.utils import ( ModelResponse, ModelResponseStream, ) -from litellm.utils import CustomStreamWrapper +from litellm.litellm_core_utils.streaming_handler import CustomStreamWrapper if TYPE_CHECKING: from litellm.litellm_core_utils.litellm_logging import Logging as _LiteLLMLoggingObj @@ -78,9 +79,6 @@ else: LiteLLMLoggingObj = Any -# OCI GenAI REST API version — stable since service launch, unlikely to change -OCI_API_VERSION = "20231130" - # Streaming timeout — generous because OCI models may need to warm up on first request STREAMING_TIMEOUT = 60 * 5 diff --git a/litellm/llms/oci/common_utils.py b/litellm/llms/oci/common_utils.py index f6ab6de3ff4..caad2b9aa18 100644 --- a/litellm/llms/oci/common_utils.py +++ b/litellm/llms/oci/common_utils.py @@ -25,6 +25,10 @@ except ImportError: _litellm_version = "0.0.0" +# OCI GenAI REST API version — stable since service launch, unlikely to change +OCI_API_VERSION = "20231130" + + def _require_cryptography() -> None: if not _CRYPTOGRAPHY_AVAILABLE: raise ImportError( @@ -62,7 +66,8 @@ class OCISignerProtocol(Protocol): def do_request_sign( self, request: Any, *, enforce_content_headers: bool = False - ) -> None: ... + ) -> None: + pass @dataclass @@ -87,6 +92,10 @@ class OCIRequestWrapper: def sha256_base64(data: bytes) -> str: + # SHA-256 is used here to compute the x-content-sha256 header required by the + # OCI HTTP signing specification (RSA-SHA256 request signing), not for password + # or secret hashing. This is the correct and mandated algorithm for this purpose. + # See: https://docs.oracle.com/en-us/iaas/Content/API/Concepts/signingrequests.htm digest = hashlib.sha256(data).digest() return base64.b64encode(digest).decode() diff --git a/litellm/llms/oci/embed/transformation.py b/litellm/llms/oci/embed/transformation.py index 85bcabdb621..cdea8e263fd 100644 --- a/litellm/llms/oci/embed/transformation.py +++ b/litellm/llms/oci/embed/transformation.py @@ -29,8 +29,8 @@ import httpx import litellm from litellm.llms.base_llm.chat.transformation import BaseLLMException from litellm.llms.base_llm.embedding.transformation import BaseEmbeddingConfig -from litellm.llms.oci.chat.transformation import OCI_API_VERSION from litellm.llms.oci.common_utils import ( + OCI_API_VERSION, OCIError, get_oci_base_url, resolve_oci_credentials, diff --git a/litellm/types/llms/oci.py b/litellm/types/llms/oci.py index 57512ec7c0c..95ef8d4fd72 100644 --- a/litellm/types/llms/oci.py +++ b/litellm/types/llms/oci.py @@ -379,7 +379,13 @@ class CohereChatResponse(BaseModel): text: str apiFormat: Literal["COHERE"] = "COHERE" finishReason: Literal[ - "COMPLETE", "ERROR_TOXIC", "ERROR_LIMIT", "ERROR", "USER_CANCEL", "MAX_TOKENS" + "COMPLETE", + "ERROR_TOXIC", + "ERROR_LIMIT", + "ERROR", + "USER_CANCEL", + "MAX_TOKENS", + "TOOL_CALL", ] # Optional fields diff --git a/model_prices_and_context_window.json b/model_prices_and_context_window.json index c165413bb37..55da4d413c5 100644 --- a/model_prices_and_context_window.json +++ b/model_prices_and_context_window.json @@ -25959,47 +25959,6 @@ "supports_response_schema": false, "supports_streaming": true }, - "oci/cohere.embed-english-v3.0": { - "input_cost_per_token": 1e-07, - "litellm_provider": "oci", - "max_input_tokens": 512, - "mode": "embedding", - "output_vector_size": 1024, - "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/" - }, - "oci/cohere.embed-multilingual-v3.0": { - "input_cost_per_token": 1e-07, - "litellm_provider": "oci", - "max_input_tokens": 512, - "mode": "embedding", - "output_vector_size": 1024, - "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/" - }, - "oci/cohere.embed-english-light-v3.0": { - "input_cost_per_token": 1e-07, - "litellm_provider": "oci", - "max_input_tokens": 512, - "mode": "embedding", - "output_vector_size": 384, - "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/" - }, - "oci/cohere.embed-multilingual-light-v3.0": { - "input_cost_per_token": 1e-07, - "litellm_provider": "oci", - "max_input_tokens": 512, - "mode": "embedding", - "output_vector_size": 384, - "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/" - }, - "oci/cohere.embed-english-image-v3.0": { - "input_cost_per_token": 1e-07, - "litellm_provider": "oci", - "max_input_tokens": 512, - "mode": "embedding", - "output_vector_size": 1024, - "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/", - "supports_vision": true - }, "oci/cohere.embed-multilingual-image-v3.0": { "input_cost_per_token": 1e-07, "litellm_provider": "oci", @@ -26009,15 +25968,6 @@ "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/", "supports_vision": true }, - "oci/cohere.embed-v4.0": { - "input_cost_per_token": 1e-07, - "litellm_provider": "oci", - "max_input_tokens": 128000, - "mode": "embedding", - "output_vector_size": 1536, - "source": "https://www.oracle.com/cloud/ai/generative-ai/pricing/", - "supports_vision": true - }, "oci/cohere.command-a-reasoning-08-2025": { "input_cost_per_token": 1.56e-06, "litellm_provider": "oci", diff --git a/tests/test_litellm/llms/oci/chat/test_oci_cohere_tool_calls.py b/tests/test_litellm/llms/oci/chat/test_oci_cohere_tool_calls.py index f945923ee63..252404759c5 100644 --- a/tests/test_litellm/llms/oci/chat/test_oci_cohere_tool_calls.py +++ b/tests/test_litellm/llms/oci/chat/test_oci_cohere_tool_calls.py @@ -507,6 +507,52 @@ class TestOCICohereToolCalls: assert "topP" not in chat_request assert "frequencyPenalty" not in chat_request + def test_cohere_response_finish_reason_tool_call(self): + """Test that finishReason='TOOL_CALL' is accepted by Pydantic and mapped to 'tool_calls'.""" + config = OCIChatConfig() + + mock_cohere_response = { + "modelId": "cohere.command-latest", + "modelVersion": "1.0", + "chatResponse": { + "apiFormat": "COHERE", + "text": "", + "finishReason": "TOOL_CALL", + "toolCalls": [ + {"name": "get_weather", "parameters": {"location": "London"}} + ], + "usage": { + "promptTokens": 20, + "completionTokens": 10, + "totalTokens": 30, + }, + }, + } + + response = httpx.Response( + status_code=200, + json=mock_cohere_response, + headers={"Content-Type": "application/json"}, + ) + + result = config.transform_response( + model="cohere.command-latest", + raw_response=response, + model_response=ModelResponse(), + logging_obj={}, # type: ignore + request_data={}, + messages=[], + optional_params={}, + litellm_params={}, + encoding={}, + ) + + assert isinstance(result, ModelResponse) + assert result.choices[0].finish_reason == "tool_calls" + assert result.choices[0].message.tool_calls is not None + assert len(result.choices[0].message.tool_calls) == 1 + assert result.choices[0].message.tool_calls[0].function.name == "get_weather" + def test_cohere_vendor_detection(self): """Test that Cohere models are correctly identified""" assert get_vendor_from_model("cohere.command-latest") == OCIVendors.COHERE