From 802a5f833d40309f60df513d3aa5680bc3900060 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 21 May 2026 05:35:33 +0000 Subject: [PATCH] fix(oci): stable tool-call ids across stream chunks; lenient Cohere finishReason - Replace random uuid4 per chunk with a deterministic content-derived digest for synthetic tool-call ids in both Cohere and Generic OCI handlers. Previously, when OCI omitted 'id' (always for Cohere, often for Generic streaming deltas), every chunk for the same logical tool call received a new uuid, causing downstream stream-mergers (which key off id) to treat each fragment as a distinct call. - Relax CohereChatResponse.finishReason from a strict Literal[...] to Optional[str], matching CohereStreamChunk.finishReason. The handle_cohere_response 'elif oci_finish_reason is not None' fallback was previously unreachable because Pydantic raised ValidationError on any unknown value before the fallback executed. Now non-streaming responses degrade unknown reasons to 'stop' just like the streaming path. Co-authored-by: Yassin Kortam --- litellm/llms/oci/chat/cohere.py | 18 +++++--- litellm/llms/oci/chat/generic.py | 23 ++++++++-- litellm/types/llms/oci.py | 15 +++---- .../oci/chat/test_oci_cohere_tool_calls.py | 44 +++++++++++++++++++ .../oci/chat/test_oci_streaming_tool_calls.py | 33 ++++++++++++++ 5 files changed, 116 insertions(+), 17 deletions(-) diff --git a/litellm/llms/oci/chat/cohere.py b/litellm/llms/oci/chat/cohere.py index d5d136d179d..4134a065ccf 100644 --- a/litellm/llms/oci/chat/cohere.py +++ b/litellm/llms/oci/chat/cohere.py @@ -8,9 +8,9 @@ response parsing, and streaming chunk parsing for models served with import datetime import json -import uuid from typing import Any, Dict, List, Optional +from litellm.llms.oci.chat.generic import _synthesize_oci_tool_call_id from litellm.llms.oci.common_utils import ( OCI_JSON_TO_PYTHON_TYPES, OCIError, @@ -230,14 +230,16 @@ def handle_cohere_response( if cohere_response.chatResponse.toolCalls: tool_calls = [ { - "id": f"call_{uuid.uuid4().hex[:24]}", + "id": _synthesize_oci_tool_call_id( + i, tc.name, json.dumps(tc.parameters, sort_keys=True) + ), "type": "function", "function": { "name": tc.name, "arguments": json.dumps(tc.parameters), }, } - for tc in cohere_response.chatResponse.toolCalls + for i, tc in enumerate(cohere_response.chatResponse.toolCalls) ] content: Optional[str] = response_text if response_text else None @@ -303,14 +305,20 @@ def handle_cohere_stream_chunk(dict_chunk: dict) -> ModelResponseStream: if cohere_tool_calls: tool_calls = [ { - "id": f"call_{uuid.uuid4().hex[:24]}", + # Cohere protocol has no tool-call id, so we synthesize one + # deterministically from the call's content/position. A random + # uuid4 per chunk would cause downstream stream-mergers to + # treat each chunk as a distinct tool call. + "id": _synthesize_oci_tool_call_id( + i, tc.name, json.dumps(tc.parameters, sort_keys=True) + ), "type": "function", "function": { "name": tc.name, "arguments": json.dumps(tc.parameters), }, } - for tc in cohere_tool_calls + for i, tc in enumerate(cohere_tool_calls) ] finish_reason = typed_chunk.finishReason diff --git a/litellm/llms/oci/chat/generic.py b/litellm/llms/oci/chat/generic.py index 00f3469a1a6..ced9bd5ff81 100644 --- a/litellm/llms/oci/chat/generic.py +++ b/litellm/llms/oci/chat/generic.py @@ -7,7 +7,7 @@ parsing, and streaming chunk parsing for models served with """ import datetime -import uuid +import hashlib from typing import Dict, List, Optional, Union import httpx @@ -270,17 +270,34 @@ def adapt_tool_definition_to_oci_standard( return new_tools +def _synthesize_oci_tool_call_id(position: int, name: str, arguments: str) -> str: + """Deterministic synthetic tool-call id derived from chunk content. + + Used as a fallback when OCI omits ``id`` (always the case for the OCI + Cohere protocol, occasionally the case for OCI GENERIC streaming chunks). + A random ``uuid4`` per chunk would cause downstream stream-merging + consumers — which key off the tool-call ``id`` — to treat re-emissions of + the same logical call (e.g. terminal consolidation chunks, retries) as + distinct calls. A content-derived digest stays stable across identical + re-emissions while differing across truly distinct calls. + """ + digest = hashlib.sha256( + f"{position}|{name}|{arguments}".encode("utf-8") + ).hexdigest()[:24] + return f"call_{digest}" + + def adapt_tools_to_openai_standard( tools: List[OCIToolCall], ) -> List[ChatCompletionMessageToolCall]: """Convert OCI tool-call objects in a response to the OpenAI format.""" return [ ChatCompletionMessageToolCall( - id=tool.id or f"call_{uuid.uuid4().hex[:24]}", + id=tool.id or _synthesize_oci_tool_call_id(i, tool.name, tool.arguments), type="function", function={"name": tool.name, "arguments": tool.arguments}, ) - for tool in tools + for i, tool in enumerate(tools) ] diff --git a/litellm/types/llms/oci.py b/litellm/types/llms/oci.py index c246bf7534c..c95ad1d13b6 100644 --- a/litellm/types/llms/oci.py +++ b/litellm/types/llms/oci.py @@ -389,15 +389,12 @@ class CohereChatResponse(BaseModel): # Required fields text: str apiFormat: Literal["COHERE"] = "COHERE" - finishReason: Literal[ - "COMPLETE", - "ERROR_TOXIC", - "ERROR_LIMIT", - "ERROR", - "USER_CANCEL", - "MAX_TOKENS", - "TOOL_CALL", - ] + # Accept any string (with ``None`` for absent) so unknown finish reasons + # — e.g. a value OCI adds in a future API revision — degrade gracefully + # via ``handle_cohere_response``'s ``elif oci_finish_reason is not None`` + # fallback instead of crashing Pydantic validation. Mirrors + # ``CohereStreamChunk.finishReason`` which has always been ``Optional[str]``. + finishReason: Optional[str] = None # Optional fields chatHistory: Optional[List[CohereMessage]] = None 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 3ae64c03240..c06418b0f09 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 @@ -558,6 +558,50 @@ class TestOCICohereToolCalls: assert len(result.choices[0].message.tool_calls) == 1 assert result.choices[0].message.tool_calls[0].function.name == "get_weather" + def test_cohere_response_unknown_finish_reason_degrades_to_stop(self): + """A future/unknown finishReason in non-streaming responses must + degrade to ``stop`` via ``handle_cohere_response``'s fallback + rather than crash Pydantic validation. Mirrors the streaming + handler's behavior. See bug caf74429. + """ + config = OCIChatConfig() + + mock_cohere_response = { + "modelId": "cohere.command-latest", + "modelVersion": "1.0", + "chatResponse": { + "apiFormat": "COHERE", + "text": "hello", + "finishReason": "FUTURE_REASON_NOT_YET_KNOWN", + "usage": { + "promptTokens": 1, + "completionTokens": 1, + "totalTokens": 2, + }, + }, + } + + 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 == "stop" + def test_cohere_vendor_detection(self): """Test that Cohere models are correctly identified""" assert get_vendor_from_model("cohere.command-latest") == OCIVendors.COHERE diff --git a/tests/test_litellm/llms/oci/chat/test_oci_streaming_tool_calls.py b/tests/test_litellm/llms/oci/chat/test_oci_streaming_tool_calls.py index fe8d6991a93..acad5da93e2 100644 --- a/tests/test_litellm/llms/oci/chat/test_oci_streaming_tool_calls.py +++ b/tests/test_litellm/llms/oci/chat/test_oci_streaming_tool_calls.py @@ -210,6 +210,39 @@ class TestOCIStreamingToolCalls: == '{"expression": "2+2"}' ) + def test_stream_chunk_missing_id_is_deterministic_across_chunks(self): + """ + Two chunks emitting the same logical tool call (same name + arguments + at the same position) must receive the *same* synthesized id so the + downstream stream-merger does not treat them as distinct calls. + Random uuid4 per chunk would regress this — see bug ffdef760. + """ + same_chunk_payload = lambda: { + "index": 0, + "finishReason": None, + "message": { + "role": "ASSISTANT", + "content": None, + "toolCalls": [ + { + "type": "FUNCTION", + "name": "get_weather", + "arguments": '{"location": "San Francisco"}', + } + ], + }, + } + + first = handle_generic_stream_chunk(same_chunk_payload()) + second = handle_generic_stream_chunk(same_chunk_payload()) + + assert first.choices[0].delta.tool_calls is not None + assert second.choices[0].delta.tool_calls is not None + first_id = first.choices[0].delta.tool_calls[0]["id"] + second_id = second.choices[0].delta.tool_calls[0]["id"] + assert first_id == second_id + assert first_id.startswith("call_") + def test_stream_chunk_without_tool_calls(self): """Plain text chunks (no tool calls) pass through correctly.""" chunk_data = {