diff --git a/core/providers/openai/openai.go b/core/providers/openai/openai.go index 04dd4fb680c..a7842a2fb5d 100644 --- a/core/providers/openai/openai.go +++ b/core/providers/openai/openai.go @@ -1296,14 +1296,12 @@ func HandleOpenAIChatCompletionStreaming( var serviceTier *schemas.BifrostServiceTier forwardedTerminalFinishReason := false // Upstream frames read but not yet handed to a chunk. Raw capture must not - // depend on whether a frame becomes a forwarded chunk: role-only, - // finish-only and usage-only frames are documented parts of an OpenAI - // stream - and the usage frame is the one Bifrost bills from - yet none of - // them reaches the content-forwarding branch that used to be the only place - // RawResponse was set. Buffering here and draining on the next forwarded + // depend on whether a frame becomes a forwarded chunk: finish-only and + // usage-only frames are documented parts of an OpenAI stream - and the usage + // frame is the one Bifrost bills from - yet neither reaches the semantic + // chunk-forwarding branch. Buffering here and draining on the next forwarded // chunk (or the synthetic terminal chunk) keeps every frame in upstream - // order, which appending them all to the final chunk would not: the - // role-only frame arrives before the content. See + // order. See // https://github.com/maximhq/bifrost/issues/7144. // // Only populated when sendBackRawResponse is set, and drained on every chunk @@ -1611,7 +1609,9 @@ func HandleOpenAIChatCompletionStreaming( created = response.Created } - // Handle regular content chunks, including reasoning + // Handle regular content chunks, including the initial role-only delta. + // OpenAI commonly sends role:"assistant" with empty content first; dropping + // it leaves strict streaming clients unable to reconstruct a valid message. // Refusal and Annotations are answer-bearing delta fields just like // Content: a refusal IS the model's reply, and annotations carry the // URL citations behind a web-search answer. Omitting them here dropped @@ -1621,7 +1621,8 @@ func HandleOpenAIChatCompletionStreaming( // OpenAI-compatible provider. if choice.ChatStreamResponseChoice != nil && choice.ChatStreamResponseChoice.Delta != nil && - ((choice.ChatStreamResponseChoice.Delta.Content != nil && *choice.ChatStreamResponseChoice.Delta.Content != "") || + (choice.ChatStreamResponseChoice.Delta.Role != nil || + (choice.ChatStreamResponseChoice.Delta.Content != nil && *choice.ChatStreamResponseChoice.Delta.Content != "") || (choice.ChatStreamResponseChoice.Delta.Refusal != nil && *choice.ChatStreamResponseChoice.Delta.Refusal != "") || len(choice.ChatStreamResponseChoice.Delta.Annotations) > 0 || choice.ChatStreamResponseChoice.Delta.Reasoning != nil || @@ -8122,4 +8123,4 @@ func (provider *OpenAIProvider) PassthroughStream( }, }, ), nil -} \ No newline at end of file +} diff --git a/core/providers/openai/streamtruncation_test.go b/core/providers/openai/streamtruncation_test.go index 2c525a8905d..958466f32d7 100644 --- a/core/providers/openai/streamtruncation_test.go +++ b/core/providers/openai/streamtruncation_test.go @@ -887,12 +887,11 @@ func TestResponsesStreamFallbackSilentParkAfterFinishReasonEndsCleanlyOnIdleTime // Raw-response capture must be independent of semantic chunk forwarding. // // The OpenAI chat streaming loop only attaches ExtraFields.RawResponse inside the -// branch that forwards a chunk carrying content/reasoning/audio/tool calls. Three -// documented, perfectly normal frame shapes never enter that branch and so their -// bytes are discarded before the framework's accumulator (which reconstructs -// raw_response purely by concatenating chunk.RawResponse) can ever see them: +// branch that forwards a semantic chunk. Some documented, perfectly normal frame +// shapes never enter that branch and so their bytes would be discarded before the +// framework's accumulator (which reconstructs raw_response purely by concatenating +// chunk.RawResponse) could ever see them: // -// - role-only delta {"role":"assistant"}, no content // - finish-only delta {} with finish_reason set // - usage-only choices: [] with the authoritative token counts // @@ -949,7 +948,7 @@ func reconstructRawResponse(t *testing.T, chunks []*schemas.BifrostStreamChunk) // fullShapeSSEBody is the frame sequence from issue #7144: role-only, content, // finish-only, usage-only, [DONE]. const ( - rawRoleOnlyFrame = `{"id":"chatcmpl-repro","object":"chat.completion.chunk","created":1,"model":"repro-model","choices":[{"index":0,"delta":{"role":"assistant"},"finish_reason":null}],"usage":null}` + rawRoleOnlyFrame = `{"id":"chatcmpl-repro","object":"chat.completion.chunk","created":1,"model":"repro-model","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}],"usage":null}` rawContentFrame = `{"id":"chatcmpl-repro","object":"chat.completion.chunk","created":1,"model":"repro-model","choices":[{"index":0,"delta":{"content":"hello"},"finish_reason":null}],"usage":null}` rawFinishOnlyFrame = `{"id":"chatcmpl-repro","object":"chat.completion.chunk","created":1,"model":"repro-model","choices":[{"index":0,"delta":{},"finish_reason":"stop"}],"usage":null}` rawUsageOnlyFrame = `{"id":"chatcmpl-repro","object":"chat.completion.chunk","created":1,"model":"repro-model","choices":[],"usage":{"prompt_tokens":1000,"completion_tokens":100,"total_tokens":1100}}` @@ -1014,11 +1013,9 @@ func TestChatStreamRawResponseKeepsFinishOnlyFrame(t *testing.T) { } } -// The opening role-only frame is dropped by the same branch. It is listed -// separately because it is the one dropped frame that arrives *before* the content -// frames: a fix that simply appends the missing bytes to the final synthetic chunk -// would satisfy the two tests above while silently reordering this one, so the -// ordering assertion below is the real contract. +// The opening role-only frame arrives before the content frames. Keeping it in this +// ordering assertion prevents raw-response capture from silently moving it to the +// final synthetic chunk. func TestChatStreamRawResponseKeepsEveryFrameInUpstreamOrder(t *testing.T) { server := completeSSEServer(t, fullShapeSSEBody()) defer server.Close() @@ -1098,6 +1095,34 @@ func collectChatDeltas(chunks []*schemas.BifrostStreamChunk) []*schemas.ChatStre return deltas } +// OpenAI begins many streams with role:"assistant" and empty content. Strict +// clients use that delta to assign the role of the accumulated message, so it must +// reach the client even though it carries no text. +func TestChatStreamForwardsRoleOnlyDelta(t *testing.T) { + server := completeSSEServer(t, fullShapeSSEBody()) + defer server.Close() + + provider := newStreamTestProvider(server.URL) + stream, bifrostErr := provider.ChatCompletionStream(newStreamTestContext(), passthroughPostHook, nil, testKey(), basicChatRequest()) + if bifrostErr != nil { + t.Fatalf("stream setup failed: %v", bifrostErr) + } + + deltas := collectChatDeltas(collectChunks(t, stream)) + if len(deltas) < 2 { + t.Fatalf("expected role and content deltas, got %d", len(deltas)) + } + if deltas[0].Role == nil || *deltas[0].Role != "assistant" { + t.Fatalf("expected first delta role assistant, got %+v", deltas[0].Role) + } + if deltas[0].Content == nil || *deltas[0].Content != "" { + t.Fatalf("expected empty content on first delta, got %+v", deltas[0].Content) + } + if deltas[1].Content == nil || *deltas[1].Content != "hello" { + t.Fatalf("expected content delta after role, got %+v", deltas[1].Content) + } +} + // A refusal is the model's answer. Dropping it hands the client an empty stream // that looks like a successful, content-free completion. func TestChatStreamForwardsRefusalOnlyDelta(t *testing.T) { diff --git a/tests/integrations/python/config.yml b/tests/integrations/python/config.yml index be91318364e..8c9358fe372 100644 --- a/tests/integrations/python/config.yml +++ b/tests/integrations/python/config.yml @@ -76,7 +76,7 @@ providers: transcription: "whisper" embeddings: "text-embedding-3-small" image_generation: "gpt-image-1" - thinking: "o1" + thinking: "gpt-5.5" batch_file_upload: "gpt-4o-2" batch_list: "gpt-4o-2" batch_retrieve: "gpt-4o-2" diff --git a/tests/integrations/python/pyproject.toml b/tests/integrations/python/pyproject.toml index a37fd9c483c..64c115b7a74 100644 --- a/tests/integrations/python/pyproject.toml +++ b/tests/integrations/python/pyproject.toml @@ -73,6 +73,9 @@ dev = [ ] [tool.pytest.ini_options] +# LangChain's standard integration suite includes unmarked async tests. Run +# those tests through pytest-asyncio instead of rejecting them in strict mode. +asyncio_mode = "auto" # Test discovery testpaths = ["."] python_files = "test_*.py" diff --git a/tests/integrations/python/tests/test_langchain.py b/tests/integrations/python/tests/test_langchain.py index bb621f2e18f..80ddf5a6d75 100644 --- a/tests/integrations/python/tests/test_langchain.py +++ b/tests/integrations/python/tests/test_langchain.py @@ -36,7 +36,6 @@ import logging import os from typing import Any, Dict, List, Type -from unittest.mock import patch import boto3 import pytest @@ -131,7 +130,7 @@ class EmbeddingsIntegrationTests: get_content_string_with_summary, mock_tool_response, ) -from .utils.config_loader import get_config, get_integration_url, get_model +from .utils.config_loader import get_config, get_integration_url, get_model, get_provider_model from .utils.parametrize import format_provider_model, get_cross_provider_params_for_scenario @@ -885,93 +884,94 @@ def test_18_multi_provider_langchain_comparison(self, test_config): """Test Case 18: Compare responses across multiple LangChain providers""" providers_tested = [] responses = {} + provider_errors = {} + base_url = get_integration_url("langchain") + message = [HumanMessage(content="What is the future of AI? Answer in one sentence.")] # Test OpenAI try: openai_chat = ChatOpenAI( - model="gpt-3.5-turbo", + model=format_provider_model("openai", get_provider_model("openai", "chat")), temperature=0.5, max_tokens=50, - base_url=( - get_integration_url("langchain") if get_integration_url("langchain") else None - ), + base_url=base_url, ) - message = [HumanMessage(content="What is the future of AI? Answer in one sentence.")] responses["openai"] = openai_chat.invoke(message) + print(f"OpenAI response: {responses['openai']}") providers_tested.append("OpenAI") - except Exception: - pass + except Exception as e: + provider_errors["OpenAI"] = str(e) # Test Anthropic try: anthropic_chat = ChatAnthropic( - model="claude-3-haiku-20240307", + model=format_provider_model( + "anthropic", get_provider_model("anthropic", "chat") + ), temperature=0.5, max_tokens=50, - base_url=( - get_integration_url("langchain") if get_integration_url("langchain") else None - ), + base_url=base_url, ) responses["anthropic"] = anthropic_chat.invoke(message) + print(f"Anthropic response: {responses['anthropic']}") providers_tested.append("Anthropic") - except Exception: - pass + except Exception as e: + provider_errors["Anthropic"] = str(e) # Test Gemini (if available) try: gemini_chat = ChatGoogleGenerativeAI( - model="gemini-1.5-flash", + model=format_provider_model("gemini", get_provider_model("gemini", "chat")), google_api_key="dummy-google-api-key-bifrost-handles-auth", temperature=0.5, max_tokens=50, + base_url=base_url, ) - base_url = get_integration_url("langchain") - if base_url: - with patch.object(gemini_chat, "_client") as mock_client: - mock_client.base_url = f"{base_url}/v1beta" - responses["gemini"] = gemini_chat.invoke(message) - providers_tested.append("Gemini") + responses["gemini"] = gemini_chat.invoke(message) + print(f"Gemini response: {responses['gemini']}") + providers_tested.append("Gemini") - except Exception: - pass + except Exception as e: + provider_errors["Gemini"] = str(e) # Test Mistral (if available) if MISTRAL_AI_AVAILABLE: try: - base_url = get_integration_url("langchain") - if base_url: - mistral_chat = ChatMistralAI( - model="mistral-7b-instruct", - mistral_api_key="dummy-mistral-api-key-bifrost-handles-auth", - endpoint=f"{base_url}/v1", - temperature=0.5, - max_tokens=50, - ) + mistral_chat = ChatMistralAI( + model=format_provider_model("mistral", "mistral-medium-3-5"), + mistral_api_key="dummy-mistral-api-key-bifrost-handles-auth", + endpoint=f"{base_url}/v1", + temperature=0.5, + max_tokens=50, + ) - responses["mistral"] = mistral_chat.invoke(message) - providers_tested.append("Mistral") + responses["mistral"] = mistral_chat.invoke(message) + print(f"Mistral response: {responses['mistral']}") + providers_tested.append("Mistral") - except Exception: - pass + except Exception as e: + provider_errors["Mistral"] = str(e) # Verify we tested at least 2 providers assert ( len(providers_tested) >= 2 - ), f"Should test at least 2 providers, got: {providers_tested}" + ), f"Should test at least 2 providers, got: {providers_tested}; errors: {provider_errors}" # Verify all responses are valid + response_contents = [] for provider, response in responses.items(): assert isinstance(response, AIMessage), f"{provider} should return AIMessage" assert response.content is not None, f"{provider} should have content" - assert len(response.content) > 0, f"{provider} should have non-empty content" + content = get_content_string(response.content) + assert content, f"{provider} should have non-empty content" + response_contents.append(content) # Verify responses are different (providers should give unique answers) - response_contents = [resp.content for resp in responses.values()] unique_responses = set(response_contents) assert len(unique_responses) > 1, "Different providers should give different responses"