diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index 0003296619..e35ff3500a 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -62,7 +62,6 @@ TextSpanRegion, UsageDetails, detect_media_type_from_base64, - prepend_instructions_to_messages, validate_tool_mode, ) from agent_framework.exceptions import ( @@ -1377,7 +1376,6 @@ async def _prepare_options( "logit_bias", # not supported "seed", # not supported "stop", # not supported - "instructions", # already added as system message "response_format", # handled separately "conversation_id", # handled separately "tool_choice", # handled separately @@ -1389,15 +1387,6 @@ async def _prepare_options( raise ChatClientInvalidRequestException( "prompt_cache_options requires openai>=2.45.0; upgrade the openai package to use it." ) - - # messages - # Handle instructions by prepending to messages as system message - # Only prepend instructions for the first turn (when no conversation/response ID exists) - conversation_id = options.get("conversation_id") - if (instructions := options.get("instructions")) and not conversation_id: - # First turn: prepend instructions as system message - messages = prepend_instructions_to_messages(list(messages), instructions, role="system") - # Continuation turn: instructions already exist in conversation context, skip prepending request_uses_service_side_storage = False for key in ("conversation_id", "previous_response_id", "conversation"): value = options.get(key) diff --git a/python/packages/openai/tests/openai/test_openai_chat_client.py b/python/packages/openai/tests/openai/test_openai_chat_client.py index 80ea1f626a..f9921bcd09 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_client.py @@ -411,10 +411,14 @@ async def test_get_response_with_all_parameters() -> None: assert len(run_options["tools"]) == 1 assert run_options["tools"][0]["type"] == "function" assert run_options["tools"][0]["name"] == "get_weather" - assert run_options["input"][0]["role"] == "system" - assert run_options["input"][0]["content"][0]["text"] == "You are a helpful assistant" - assert run_options["input"][1]["role"] == "user" - assert run_options["input"][1]["content"][0]["text"] == "Test message" + + # Verify instructions are passed natively, not as a system message + assert run_options["instructions"] == "You are a helpful assistant" + + # Verify the input only contains the user message + assert len(run_options["input"]) == 1 + assert run_options["input"][0]["role"] == "user" + assert run_options["input"][0]["content"][0]["text"] == "Test message" @pytest.mark.asyncio @@ -6196,70 +6200,30 @@ def _create_mock_responses_text_response(*, response_id: str) -> MagicMock: return mock_response -async def test_instructions_sent_first_turn_then_skipped_for_continuation() -> None: - client = OpenAIChatClient(model="test-model", api_key="test-key") - mock_response = _create_mock_responses_text_response(response_id="resp_123") - - with patch.object(client.client.responses, "create", return_value=mock_response) as mock_create: - await client.get_response( - messages=[Message(role="user", contents=["Hello"])], - options={"instructions": "Reply in uppercase."}, - ) - - first_input_messages = mock_create.call_args.kwargs["input"] - assert len(first_input_messages) == 2 - assert first_input_messages[0]["role"] == "system" - assert any("Reply in uppercase" in str(c) for c in first_input_messages[0]["content"]) - assert first_input_messages[1]["role"] == "user" - - await client.get_response( - messages=[Message(role="user", contents=["Tell me a joke"])], - options={ - "instructions": "Reply in uppercase.", - "conversation_id": "resp_123", - }, - ) - - second_input_messages = mock_create.call_args.kwargs["input"] - assert len(second_input_messages) == 1 - assert second_input_messages[0]["role"] == "user" - assert not any(message["role"] == "system" for message in second_input_messages) - - -@pytest.mark.parametrize("conversation_id", ["resp_456", "conv_abc123"]) -async def test_instructions_not_repeated_for_continuation_ids( - conversation_id: str, +@pytest.mark.parametrize("conversation_id", [None, "resp_456", "conv_abc123"]) +async def test_instructions_passed_natively_not_as_system_message( + conversation_id: str | None, ) -> None: + """Test that instructions are passed to the Responses API natively and not prepended to messages.""" client = OpenAIChatClient(model="test-model", api_key="test-key") mock_response = _create_mock_responses_text_response(response_id="resp_456") with patch.object(client.client.responses, "create", return_value=mock_response) as mock_create: - await client.get_response( - messages=[Message(role="user", contents=["Continue conversation"])], - options={"instructions": "Be helpful.", "conversation_id": conversation_id}, - ) - - input_messages = mock_create.call_args.kwargs["input"] - assert len(input_messages) == 1 - assert input_messages[0]["role"] == "user" - assert not any(message["role"] == "system" for message in input_messages) + options: OpenAIChatOptions = {"instructions": "Reply in uppercase."} + if conversation_id: + options["conversation_id"] = conversation_id - -async def test_instructions_included_without_conversation_id() -> None: - client = OpenAIChatClient(model="test-model", api_key="test-key") - mock_response = _create_mock_responses_text_response(response_id="resp_new") - - with patch.object(client.client.responses, "create", return_value=mock_response) as mock_create: await client.get_response( messages=[Message(role="user", contents=["Hello"])], - options={"instructions": "You are a helpful assistant."}, + options=options, ) + assert mock_create.call_args.kwargs.get("instructions") == "Reply in uppercase." + input_messages = mock_create.call_args.kwargs["input"] - assert len(input_messages) == 2 - assert input_messages[0]["role"] == "system" - assert any("helpful assistant" in str(c) for c in input_messages[0]["content"]) - assert input_messages[1]["role"] == "user" + assert len(input_messages) == 1 + assert input_messages[0]["role"] == "user" + assert not any(message.get("role") == "system" for message in input_messages) def test_with_callable_api_key() -> None: