From 6c7cc62c5281aaa768228d3c642ca0812f86ed4b Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sun, 6 Sep 2026 02:25:35 -0700 Subject: [PATCH] fix(oci): report the real finish_reason and one id per streamed tool call OCI's GENERIC protocol already sends the OpenAI reason names, so collapsing everything outside the Cohere vocabulary to "stop" turned tool_calls and length into stop on every GENERIC turn. Pass the OpenAI names through, and report tool_calls whenever the turn carries tool calls, which is what the Cohere protocol needs since it labels a tool-calling turn COMPLETE. GENERIC streaming sends the tool-call id only on the delta that opens the call, and the empty-string fallback hashed each argument fragment into an id of its own. Emit an id only on the delta that carries the function name, the way the Cohere handler already does. Resolves LIT-6826 Resolves LIT-6818 --- litellm/llms/oci/chat/cohere.py | 10 +- litellm/llms/oci/chat/generic.py | 51 +++--- .../oci/chat/test_oci_chat_transformation.py | 2 +- .../oci/chat/test_oci_cohere_tool_calls.py | 85 ++++++++++ .../llms/oci/chat/test_oci_generic_chat.py | 145 +++++++++++++++++- .../oci/chat/test_oci_streaming_tool_calls.py | 9 +- 6 files changed, 278 insertions(+), 24 deletions(-) diff --git a/litellm/llms/oci/chat/cohere.py b/litellm/llms/oci/chat/cohere.py index 6e9bb83b0a0..bfa5ae88708 100644 --- a/litellm/llms/oci/chat/cohere.py +++ b/litellm/llms/oci/chat/cohere.py @@ -235,7 +235,10 @@ def handle_cohere_response( model_response.created = int(datetime.datetime.now().timestamp()) response_text: Final = cohere_response.chatResponse.text - finish_reason: Final = _normalize_oci_finish_reason(cohere_response.chatResponse.finishReason) + finish_reason: Final = _normalize_oci_finish_reason( + cohere_response.chatResponse.finishReason, + has_tool_calls=bool(cohere_response.chatResponse.toolCalls), + ) tool_calls: list[dict[str, object]] | None = None if cohere_response.chatResponse.toolCalls: @@ -361,7 +364,10 @@ def handle_cohere_stream_chunk( for i, tc in enumerate(cohere_tool_calls) ] - finish_reason: Final = _normalize_oci_finish_reason(typed_chunk.finishReason) + finish_reason: Final = _normalize_oci_finish_reason( + typed_chunk.finishReason, + has_tool_calls=bool(typed_chunk.toolCalls) or prior_tool_calls_emitted, + ) return ModelResponseStream( choices=[ diff --git a/litellm/llms/oci/chat/generic.py b/litellm/llms/oci/chat/generic.py index 8ff8ef9abc4..38b6684374c 100644 --- a/litellm/llms/oci/chat/generic.py +++ b/litellm/llms/oci/chat/generic.py @@ -8,6 +8,7 @@ parsing, and streaming chunk parsing for models served with import datetime import hashlib +from types import MappingProxyType from typing import Any, Final import httpx @@ -240,25 +241,33 @@ def adapt_tool_definition_to_oci_standard(tools: list[dict], vendor: OCIVendors) return new_tools -def _normalize_oci_finish_reason(raw: str | None) -> str | None: - """Map an OCI-specific finish reason to its OpenAI-standard equivalent. +_OCI_FINISH_REASONS: Final = MappingProxyType( + { + "COMPLETE": "stop", + "MAX_TOKENS": "length", + "TOOL_CALL": "tool_calls", + "TOOL_CALLS": "tool_calls", + } +) - OCI emits ``COMPLETE`` / ``MAX_TOKENS`` / ``TOOL_CALL(S)`` plus a long tail - of error/cancel reasons (``ERROR``, ``ERROR_TOXIC``, ``ERROR_LIMIT``, - ``USER_CANCEL``, ``CONTENT_FILTERED``, ``CANCELLED``, ...). The OpenAI - spec only defines ``stop`` / ``length`` / ``tool_calls`` / ... — anything - else is collapsed to ``"stop"`` so downstream consumers switching on - ``finish_reason`` keep working. A ``None`` input passes through unchanged. +_OPENAI_FINISH_REASONS: Final = frozenset({"stop", "length", "tool_calls", "content_filter", "function_call"}) + + +def _normalize_oci_finish_reason(raw: str | None, *, has_tool_calls: bool = False) -> str | None: + """Map an OCI finish reason to its OpenAI-standard equivalent. + + OCI uses two vocabularies. Cohere sends ``COMPLETE`` / ``MAX_TOKENS`` / + ``TOOL_CALL(S)`` plus a long tail of error and cancel reasons, while GENERIC + already sends the OpenAI names and has to be passed through rather than + collapsed. Anything outside both sets becomes ``"stop"``, ``None`` stays + ``None`` so intermediate stream chunks stay open, and ``has_tool_calls`` + upgrades ``stop`` to ``tool_calls`` for a Cohere tool turn, which OCI still + labels ``COMPLETE``. """ if raw is None: return None - if raw == "COMPLETE": - return "stop" - if raw == "MAX_TOKENS": - return "length" - if raw in ("TOOL_CALL", "TOOL_CALLS"): - return "tool_calls" - return "stop" + normalized: Final = _OCI_FINISH_REASONS.get(raw, raw if raw in _OPENAI_FINISH_REASONS else "stop") + return "tool_calls" if has_tool_calls and normalized == "stop" else normalized def _synthesize_oci_tool_call_id(position: int, name: str, arguments: str) -> str: @@ -341,7 +350,10 @@ def handle_generic_response( if response_message.toolCalls: message.tool_calls = adapt_tools_to_openai_standard(response_message.toolCalls) - model_response.choices[0].finish_reason = _normalize_oci_finish_reason(response_choice.finishReason) + model_response.choices[0].finish_reason = _normalize_oci_finish_reason( + response_choice.finishReason, + has_tool_calls=bool(message.tool_calls), + ) oci_usage: Final = completion_response.chatResponse.usage reasoning_tokens: int | None = None @@ -408,7 +420,7 @@ def handle_generic_stream_chunk(dict_chunk: dict) -> ModelResponseStream: if typed_chunk.message and typed_chunk.message.toolCalls: tool_calls = [ { - "id": tc.id or _synthesize_oci_tool_call_id(i, tc.name, tc.arguments), + "id": tc.id or (_synthesize_oci_tool_call_id(i, tc.name, tc.arguments) if tc.name else None), "type": "function", "function": { "name": tc.name, @@ -418,7 +430,10 @@ def handle_generic_stream_chunk(dict_chunk: dict) -> ModelResponseStream: for i, tc in enumerate(typed_chunk.message.toolCalls) ] - finish_reason: Final[str | None] = _normalize_oci_finish_reason(typed_chunk.finishReason) + finish_reason: Final[str | None] = _normalize_oci_finish_reason( + typed_chunk.finishReason, + has_tool_calls=bool(tool_calls), + ) return ModelResponseStream( choices=[ diff --git a/tests/test_litellm/llms/oci/chat/test_oci_chat_transformation.py b/tests/test_litellm/llms/oci/chat/test_oci_chat_transformation.py index 86c534c73c2..ac04d0f5fab 100644 --- a/tests/test_litellm/llms/oci/chat/test_oci_chat_transformation.py +++ b/tests/test_litellm/llms/oci/chat/test_oci_chat_transformation.py @@ -721,7 +721,7 @@ class TestOCIChatConfig: choice = result.choices[0] assert isinstance(choice, litellm.Choices) - assert choice.finish_reason == "stop" + assert choice.finish_reason == "tool_calls" # Message and tool_calls assertions message = choice.message 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 5dd44d72d68..0b6b375cbf1 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 @@ -445,6 +445,91 @@ class TestOCICohereToolCalls: assert result.choices[0].index == 0 assert result.choices[0].finish_reason == "stop" # COMPLETE is mapped to stop + def test_cohere_tool_turn_reports_tool_calls_despite_complete(self): + """OCI Cohere labels a tool-calling turn ``COMPLETE`` rather than ``TOOL_CALL``. + + Passing that through as ``stop`` leaves an agent loop with nothing telling + it to run the tool it was just handed. + """ + wrapper = OCIStreamWrapper( + completion_stream=MagicMock(), model="cohere.command-a-03-2025", logging_obj=MagicMock() + ) + preamble = {"apiFormat": "COHERE", "text": "I will use the get_weather tool."} + tool_calls_event = { + "apiFormat": "COHERE", + "text": "I will use the get_weather tool.", + "toolCalls": [{"name": "get_weather", "parameters": {"city": "Paris"}}], + } + terminal = { + "apiFormat": "COHERE", + "text": "I will use the get_weather tool.", + "chatHistory": [ + {"role": "USER", "message": "What is the weather in Paris?"}, + { + "role": "CHATBOT", + "message": "I will use the get_weather tool.", + "toolCalls": [{"name": "get_weather", "parameters": {"city": "Paris"}}], + }, + ], + "finishReason": "COMPLETE", + "toolCalls": [{"name": "get_weather", "parameters": {"city": "Paris"}}], + } + + chunks = [ + wrapper.chunk_creator(f"data: {json.dumps(event)}") + for event in (preamble, tool_calls_event, terminal) + ] + + assert [chunk.choices[0].finish_reason for chunk in chunks if chunk.choices[0].finish_reason] == [ + "tool_calls" + ] + + def test_cohere_tool_turn_reports_tool_calls_when_the_last_chunk_is_bare(self): + """The finish reason can arrive on its own chunk, after the tool calls were streamed.""" + wrapper = OCIStreamWrapper( + completion_stream=MagicMock(), model="cohere.command-a-03-2025", logging_obj=MagicMock() + ) + tool_calls_event = { + "apiFormat": "COHERE", + "text": "I will use the get_weather tool.", + "toolCalls": [{"name": "get_weather", "parameters": {"city": "Paris"}}], + } + + chunks = [ + wrapper.chunk_creator(f"data: {json.dumps(event)}") + for event in (tool_calls_event, {"apiFormat": "COHERE", "finishReason": "COMPLETE"}) + ] + + assert chunks[-1].choices[0].finish_reason == "tool_calls" + + def test_cohere_non_streaming_tool_call_reports_tool_calls(self): + """Same COMPLETE-on-a-tool-turn story on the non-streaming Cohere response.""" + body = { + "modelId": "cohere.command-a-03-2025", + "modelVersion": "1.0", + "chatResponse": { + "apiFormat": "COHERE", + "text": "I will look up the weather in Paris.", + "finishReason": "COMPLETE", + "toolCalls": [{"name": "get_weather", "parameters": {"city": "Paris"}}], + "usage": {"completionTokens": 17, "promptTokens": 13, "totalTokens": 30}, + }, + } + result = OCIChatConfig().transform_response( + model="cohere.command-a-03-2025", + raw_response=httpx.Response(status_code=200, json=body), + model_response=ModelResponse(), + logging_obj={}, # type: ignore + request_data={}, + messages=[], + optional_params={}, + litellm_params={}, + encoding={}, + ) + + assert result.choices[0].finish_reason == "tool_calls" + assert len(result.choices[0].message.tool_calls) == 1 + def test_cohere_parameter_mapping_excludes_tool_choice(self): """Test that tool_choice is excluded from Cohere parameter mapping""" config = OCIChatConfig() diff --git a/tests/test_litellm/llms/oci/chat/test_oci_generic_chat.py b/tests/test_litellm/llms/oci/chat/test_oci_generic_chat.py index 0a47852d085..d81ec0c6812 100644 --- a/tests/test_litellm/llms/oci/chat/test_oci_generic_chat.py +++ b/tests/test_litellm/llms/oci/chat/test_oci_generic_chat.py @@ -2,12 +2,14 @@ Unit tests for litellm/llms/oci/chat/generic.py — error paths and stream handling. """ +import json + import pytest from unittest.mock import MagicMock import httpx -from litellm import ModelResponse +from litellm import ModelResponse, stream_chunk_builder from litellm.llms.oci.chat.generic import ( adapt_messages_to_generic_oci_standard, adapt_messages_to_generic_oci_standard_content_message, @@ -462,3 +464,144 @@ class TestGpt5MaxCompletionTokens: dumped = payload.model_dump(exclude_none=True) assert dumped["maxCompletionTokens"] == 64 assert "maxTokens" not in dumped + + +# --------------------------------------------------------------------------- +# finish_reason fidelity — replays what OCI GENERIC actually sends +# --------------------------------------------------------------------------- + + +class TestGenericFinishReasonFidelity: + """OCI GENERIC sends the OpenAI finish-reason names verbatim. + + Captured against ``oci/meta.llama-3.3-70b-instruct`` in ``us-chicago-1``: a + tool-calling turn ends with ``"finishReason": "tool_calls"`` and a + max-token cut-off with ``"finishReason": "length"``, both lowercase. + Collapsing either to ``stop`` leaves an agent loop with no signal to run + the tool and hides truncation from the caller. + """ + + _TOOL_CALL_WIRE = [ + { + "index": 0, + "message": { + "role": "ASSISTANT", + "toolCalls": [ + {"type": "FUNCTION", "id": "chatcmpl-tool-a8c3005a5731db5e", "name": "get_weather"} + ], + }, + "pad": "aaaaaaaaa", + }, + { + "index": 0, + "message": {"role": "ASSISTANT", "toolCalls": [{"type": "FUNCTION", "arguments": '{"city": "'}]}, + "pad": "aaa", + }, + { + "index": 0, + "message": {"role": "ASSISTANT", "toolCalls": [{"type": "FUNCTION", "arguments": 'Paris"}'}]}, + "pad": "aaaa", + }, + { + "message": {"role": "ASSISTANT", "toolCalls": [{"type": "FUNCTION", "arguments": ""}]}, + "finishReason": "tool_calls", + "pad": "aaaa", + }, + ] + + @staticmethod + def _drain(events): + wrapper = OCIStreamWrapper( + completion_stream=MagicMock(), model="meta.llama-3.3-70b-instruct", logging_obj=MagicMock() + ) + return [wrapper.chunk_creator(f"data: {json.dumps(event)}") for event in events] + + def test_tool_calling_stream_ends_with_tool_calls(self): + chunks = self._drain(self._TOOL_CALL_WIRE) + assert [chunk.choices[0].finish_reason for chunk in chunks] == [None, None, None, "tool_calls"] + + def test_truncated_stream_ends_with_length(self): + chunks = self._drain( + [ + { + "message": {"role": "ASSISTANT", "content": [{"type": "TEXT", "text": " conditions"}]}, + "finishReason": "length", + } + ] + ) + assert chunks[0].choices[0].finish_reason == "length" + + def test_argument_fragments_carry_no_id_of_their_own(self): + chunks = self._drain(self._TOOL_CALL_WIRE) + emitted_ids = [call["id"] for chunk in chunks for call in (chunk.choices[0].delta.tool_calls or [])] + assert emitted_ids == ["chatcmpl-tool-a8c3005a5731db5e", None, None, None] + + def test_assembled_tool_call_keeps_the_id_oci_issued(self): + assembled = stream_chunk_builder(self._drain(self._TOOL_CALL_WIRE), messages=[]) + + choice = assembled.choices[0] + assert choice.finish_reason == "tool_calls" + assert len(choice.message.tool_calls) == 1 + assert choice.message.tool_calls[0].id == "chatcmpl-tool-a8c3005a5731db5e" + assert choice.message.tool_calls[0].function.arguments == '{"city": "Paris"}' + + def test_non_streaming_tool_call_response_ends_with_tool_calls(self): + body = { + "modelId": "meta.llama-3.3-70b-instruct", + "modelVersion": "1.0.0", + "chatResponse": { + "apiFormat": "GENERIC", + "timeCreated": "2026-09-06T09:00:39.841Z", + "choices": [ + { + "index": 0, + "message": { + "role": "ASSISTANT", + "toolCalls": [ + { + "type": "FUNCTION", + "id": "chatcmpl-tool-a6e053a520f1cfe9", + "name": "get_weather", + "arguments": '{"city": "Paris"}', + } + ], + }, + "finishReason": "tool_calls", + } + ], + "usage": {"completionTokens": 23, "promptTokens": 238, "totalTokens": 261}, + }, + } + result = handle_generic_response( + body, "meta.llama-3.3-70b-instruct", ModelResponse(), httpx.Response(status_code=200, json=body) + ) + + assert result.choices[0].finish_reason == "tool_calls" + assert result.choices[0].message.tool_calls[0].id == "chatcmpl-tool-a6e053a520f1cfe9" + + def test_non_streaming_truncated_response_ends_with_length(self): + body = { + "modelId": "meta.llama-3.3-70b-instruct", + "modelVersion": "1.0.0", + "chatResponse": { + "apiFormat": "GENERIC", + "timeCreated": "2026-09-06T09:00:40.889Z", + "choices": [ + { + "index": 0, + "message": { + "role": "ASSISTANT", + "content": [{"type": "TEXT", "text": "I'm a large language model, I don't"}], + "toolCalls": [], + }, + "finishReason": "length", + } + ], + "usage": {"completionTokens": 20, "promptTokens": 42, "totalTokens": 62}, + }, + } + result = handle_generic_response( + body, "meta.llama-3.3-70b-instruct", ModelResponse(), httpx.Response(status_code=200, json=body) + ) + + assert result.choices[0].finish_reason == "length" 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 002def9196d..524cdc9258a 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 @@ -99,7 +99,12 @@ class TestOCIStreamingToolCalls: assert result.choices[0].delta.tool_calls[0]["function"]["name"] == "" def test_stream_chunk_with_all_missing_fields(self): - """All optional fields missing — all default gracefully.""" + """All optional fields missing — all default gracefully. + + A delta carrying neither an id nor a function name is an argument + fragment of a call that was already opened, so it carries no id of its + own and the consumer merges it onto the open call by index. + """ chunk_data = { "index": 0, "finishReason": None, @@ -119,7 +124,7 @@ class TestOCIStreamingToolCalls: assert isinstance(result, ModelResponseStream) assert result.choices[0].delta.tool_calls is not None - assert result.choices[0].delta.tool_calls[0]["id"].startswith("call_") + assert result.choices[0].delta.tool_calls[0]["id"] is None assert result.choices[0].delta.tool_calls[0]["function"]["name"] == "" assert result.choices[0].delta.tool_calls[0]["function"]["arguments"] == ""