mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fixed instructions duplication in model clients (#1332)
* Small fix * Updated instruction handling in chat clients
This commit is contained in:
committed by
GitHub
Unverified
parent
8967269d3e
commit
a02b82f022
@@ -830,7 +830,11 @@ class AzureAIAgentClient(BaseChatClient):
|
||||
run_options["additional_messages"] = additional_messages
|
||||
|
||||
# Add instruction from existing agent at the beginning
|
||||
if agent_definition is not None and agent_definition.instructions:
|
||||
if (
|
||||
agent_definition is not None
|
||||
and agent_definition.instructions
|
||||
and agent_definition.instructions not in instructions
|
||||
):
|
||||
instructions.insert(0, agent_definition.instructions)
|
||||
|
||||
if len(instructions) > 0:
|
||||
|
||||
@@ -554,6 +554,19 @@ async def test_azure_ai_chat_client_create_run_options_with_messages(mock_ai_pro
|
||||
assert len(run_options["additional_messages"]) == 1 # Only user message
|
||||
|
||||
|
||||
async def test_azure_ai_chat_client_instructions_sent_once(mock_ai_project_client: MagicMock) -> None:
|
||||
"""Ensure instructions are only sent once for AzureAIAgentClient."""
|
||||
chat_client = create_test_azure_ai_chat_client(mock_ai_project_client)
|
||||
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
messages = chat_client.prepare_messages([ChatMessage(role=Role.USER, text="Hello")], chat_options)
|
||||
|
||||
run_options, _ = await chat_client._create_run_options(messages, chat_options) # type: ignore
|
||||
|
||||
assert run_options.get("instructions") == instructions
|
||||
|
||||
|
||||
async def test_azure_ai_chat_client_inner_get_response(mock_ai_project_client: MagicMock) -> None:
|
||||
"""Test _inner_get_response method."""
|
||||
chat_client = create_test_azure_ai_chat_client(mock_ai_project_client, agent_id="test-agent")
|
||||
|
||||
@@ -425,7 +425,7 @@ class OpenAIAssistantsClient(OpenAIConfigMixin, BaseChatClient):
|
||||
"json_schema": chat_options.response_format.model_json_schema(),
|
||||
}
|
||||
|
||||
instructions: list[str] = [chat_options.instructions] if chat_options and chat_options.instructions else []
|
||||
instructions: list[str] = []
|
||||
tool_results: list[FunctionResultContent] | None = None
|
||||
|
||||
additional_messages: list[AdditionalMessage] | None = None
|
||||
|
||||
@@ -154,10 +154,13 @@ class OpenAIBaseChatClient(OpenAIBase, BaseChatClient):
|
||||
|
||||
def _prepare_options(self, messages: MutableSequence[ChatMessage], chat_options: ChatOptions) -> dict[str, Any]:
|
||||
# Preprocess web search tool if it exists
|
||||
options_dict = chat_options.to_dict(exclude={"type"})
|
||||
instructions = options_dict.pop("instructions", None)
|
||||
if instructions:
|
||||
messages = [ChatMessage(role="system", text=instructions), *messages]
|
||||
options_dict = chat_options.to_dict(
|
||||
exclude={
|
||||
"type",
|
||||
"instructions", # included as system message
|
||||
}
|
||||
)
|
||||
|
||||
if messages and "messages" not in options_dict:
|
||||
options_dict["messages"] = self._prepare_chat_history_for_request(messages)
|
||||
if "messages" not in options_dict:
|
||||
|
||||
@@ -309,6 +309,7 @@ class OpenAIBaseResponsesClient(OpenAIBase, BaseChatClient):
|
||||
"logit_bias", # not supported
|
||||
"seed", # not supported
|
||||
"stop", # not supported
|
||||
"instructions", # already added as system message
|
||||
}
|
||||
)
|
||||
translations = {
|
||||
|
||||
@@ -15,6 +15,7 @@ from agent_framework import (
|
||||
ChatAgent,
|
||||
ChatClientProtocol,
|
||||
ChatMessage,
|
||||
ChatOptions,
|
||||
ChatResponse,
|
||||
ChatResponseUpdate,
|
||||
HostedCodeInterpreterTool,
|
||||
@@ -154,6 +155,18 @@ def test_azure_assistants_client_init_with_default_headers(azure_openai_unit_tes
|
||||
assert chat_client.client.default_headers[key] == value
|
||||
|
||||
|
||||
def test_azure_assistants_client_instructions_sent_once(mock_async_azure_openai: MagicMock) -> None:
|
||||
"""Ensure instructions are only included once for Azure OpenAI Assistants requests."""
|
||||
chat_client = create_test_azure_assistants_client(mock_async_azure_openai)
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
|
||||
prepared_messages = chat_client.prepare_messages([ChatMessage(role="user", text="Hello")], chat_options)
|
||||
run_options, _ = chat_client._prepare_options(prepared_messages, chat_options) # type: ignore[reportPrivateUsage]
|
||||
|
||||
assert run_options.get("instructions") == instructions
|
||||
|
||||
|
||||
async def test_azure_assistants_client_get_assistant_id_or_create_existing_assistant(
|
||||
mock_async_azure_openai: MagicMock,
|
||||
) -> None:
|
||||
|
||||
@@ -23,6 +23,7 @@ from agent_framework import (
|
||||
ChatAgent,
|
||||
ChatClientProtocol,
|
||||
ChatMessage,
|
||||
ChatOptions,
|
||||
ChatResponse,
|
||||
ChatResponseUpdate,
|
||||
TextContent,
|
||||
@@ -83,6 +84,18 @@ def test_init_base_url(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
assert azure_chat_client.client.default_headers[key] == value
|
||||
|
||||
|
||||
def test_azure_openai_chat_client_instructions_sent_once(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Ensure instructions are only included once when preparing Azure OpenAI chat requests."""
|
||||
client = AzureOpenAIChatClient()
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
|
||||
prepared_messages = client.prepare_messages([ChatMessage(role="user", text="Hello")], chat_options)
|
||||
request_options = client._prepare_options(prepared_messages, chat_options) # type: ignore[reportPrivateUsage]
|
||||
|
||||
assert json.dumps(request_options).count(instructions) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("exclude_list", [["AZURE_OPENAI_BASE_URL"]], indirect=True)
|
||||
def test_init_endpoint(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
azure_chat_client = AzureOpenAIChatClient()
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
import json
|
||||
import os
|
||||
from typing import Annotated
|
||||
|
||||
@@ -14,6 +15,7 @@ from agent_framework import (
|
||||
ChatAgent,
|
||||
ChatClientProtocol,
|
||||
ChatMessage,
|
||||
ChatOptions,
|
||||
ChatResponse,
|
||||
ChatResponseUpdate,
|
||||
HostedCodeInterpreterTool,
|
||||
@@ -112,6 +114,18 @@ def test_init_with_default_header(azure_openai_unit_test_env: dict[str, str]) ->
|
||||
assert azure_responses_client.client.default_headers[key] == value
|
||||
|
||||
|
||||
def test_azure_responses_client_instructions_sent_once(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Ensure instructions are only included once for Azure OpenAI Responses requests."""
|
||||
client = AzureOpenAIResponsesClient()
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
|
||||
prepared_messages = client.prepare_messages([ChatMessage(role="user", text="Hello")], chat_options)
|
||||
request_options = client._prepare_options(prepared_messages, chat_options) # type: ignore[reportPrivateUsage]
|
||||
|
||||
assert json.dumps(request_options).count(instructions) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("exclude_list", [["AZURE_OPENAI_RESPONSES_DEPLOYMENT_NAME"]], indirect=True)
|
||||
def test_init_with_empty_model_id(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
|
||||
@@ -193,6 +193,18 @@ def test_openai_assistants_client_init_with_default_headers(openai_unit_test_env
|
||||
assert chat_client.client.default_headers[key] == value
|
||||
|
||||
|
||||
def test_openai_assistants_client_instructions_sent_once(mock_async_openai: MagicMock) -> None:
|
||||
"""Ensure instructions are only included once for OpenAI Assistants requests."""
|
||||
chat_client = create_test_openai_assistants_client(mock_async_openai)
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
|
||||
prepared_messages = chat_client.prepare_messages([ChatMessage(role=Role.USER, text="Hello")], chat_options)
|
||||
run_options, _ = chat_client._prepare_options(prepared_messages, chat_options) # type: ignore[reportPrivateUsage]
|
||||
|
||||
assert run_options.get("instructions") == instructions
|
||||
|
||||
|
||||
async def test_openai_assistants_client_get_assistant_id_or_create_existing_assistant(
|
||||
mock_async_openai: MagicMock,
|
||||
) -> None:
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
import json
|
||||
import os
|
||||
from typing import Annotated
|
||||
from unittest.mock import MagicMock, patch
|
||||
@@ -99,6 +100,18 @@ def test_init_base_url_from_settings_env() -> None:
|
||||
assert str(client.client.base_url) == "https://custom-openai-endpoint.com/v1/"
|
||||
|
||||
|
||||
def test_openai_chat_client_instructions_sent_once(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Ensure instructions are only included once for OpenAI chat requests."""
|
||||
client = OpenAIChatClient()
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
|
||||
prepared_messages = client.prepare_messages([ChatMessage(role="user", text="Hello")], chat_options)
|
||||
request_options = client._prepare_options(prepared_messages, chat_options) # type: ignore[reportPrivateUsage]
|
||||
|
||||
assert json.dumps(request_options).count(instructions) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("exclude_list", [["OPENAI_CHAT_MODEL_ID"]], indirect=True)
|
||||
def test_init_with_empty_model_id(openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import json
|
||||
import os
|
||||
from typing import Annotated
|
||||
from unittest.mock import MagicMock, patch
|
||||
@@ -133,6 +134,18 @@ def test_init_with_default_header(openai_unit_test_env: dict[str, str]) -> None:
|
||||
assert openai_responses_client.client.default_headers[key] == value
|
||||
|
||||
|
||||
def test_openai_responses_client_instructions_sent_once(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Ensure instructions are only included once for OpenAI Responses requests."""
|
||||
client = OpenAIResponsesClient()
|
||||
instructions = "You are a helpful assistant."
|
||||
chat_options = ChatOptions(instructions=instructions)
|
||||
|
||||
prepared_messages = client.prepare_messages([ChatMessage(role="user", text="Hello")], chat_options)
|
||||
request_options = client._prepare_options(prepared_messages, chat_options) # type: ignore[reportPrivateUsage]
|
||||
|
||||
assert json.dumps(request_options).count(instructions) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("exclude_list", [["OPENAI_RESPONSES_MODEL_ID"]], indirect=True)
|
||||
def test_init_with_empty_model_id(openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
|
||||
Reference in New Issue
Block a user