mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: [BREAKING] parameter naming and other fixes (#1255)
* parameter naming and other fixes * fix test * fix azure openai responses decorator ordering * fix test * fix mypy * fixes in options handling * fix tests * final fixes * exclude macos tests * fix model param
This commit is contained in:
committed by
GitHub
Unverified
parent
1a81ed202e
commit
76900f0eab
@@ -238,7 +238,7 @@ async def test_chat_client_observability(mock_chat_client, span_exporter: InMemo
|
||||
|
||||
messages = [ChatMessage(role=Role.USER, text="Test message")]
|
||||
span_exporter.clear()
|
||||
response = await client.get_response(messages=messages, model="Test")
|
||||
response = await client.get_response(messages=messages, model_id="Test")
|
||||
assert response is not None
|
||||
spans = span_exporter.get_finished_spans()
|
||||
assert len(spans) == 1
|
||||
@@ -263,7 +263,7 @@ async def test_chat_client_streaming_observability(
|
||||
span_exporter.clear()
|
||||
# Collect all yielded updates
|
||||
updates = []
|
||||
async for update in client.get_streaming_response(messages=messages, model="Test"):
|
||||
async for update in client.get_streaming_response(messages=messages, model_id="Test"):
|
||||
updates.append(update)
|
||||
|
||||
# Verify we got the expected updates, this shouldn't be dependent on otel
|
||||
|
||||
@@ -4,13 +4,12 @@ from collections.abc import AsyncIterable
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from pydantic import BaseModel, ValidationError
|
||||
from pydantic import BaseModel
|
||||
from pytest import fixture, mark, raises
|
||||
|
||||
from agent_framework import (
|
||||
AgentRunResponse,
|
||||
AgentRunResponseUpdate,
|
||||
AIFunction,
|
||||
BaseContent,
|
||||
ChatMessage,
|
||||
ChatOptions,
|
||||
@@ -37,7 +36,7 @@ from agent_framework import (
|
||||
UsageDetails,
|
||||
ai_function,
|
||||
)
|
||||
from agent_framework.exceptions import AdditionItemMismatch
|
||||
from agent_framework.exceptions import AdditionItemMismatch, ContentError
|
||||
|
||||
|
||||
@fixture
|
||||
@@ -451,7 +450,8 @@ def test_ai_content_serialization(content_type: type[BaseContent], args: dict):
|
||||
else:
|
||||
# Normal attribute checking for other content types
|
||||
for key, value in args.items():
|
||||
assert getattr(deserialized, key) == value
|
||||
if value:
|
||||
assert getattr(deserialized, key) == value
|
||||
|
||||
# For now, skip the TestModel validation since it still uses Pydantic
|
||||
# This would need to be updated when we migrate more classes
|
||||
@@ -772,53 +772,11 @@ def test_chat_options_init() -> None:
|
||||
assert options.model_id is None
|
||||
|
||||
|
||||
def test_chat_options_init_with_args(ai_function_tool, ai_tool) -> None:
|
||||
options = ChatOptions(
|
||||
model_id="gpt-4",
|
||||
max_tokens=1024,
|
||||
temperature=0.7,
|
||||
top_p=0.9,
|
||||
presence_penalty=0.0,
|
||||
frequency_penalty=0.0,
|
||||
user="user-123",
|
||||
tools=[ai_function_tool, ai_tool],
|
||||
tool_choice="required",
|
||||
additional_properties={"custom": True},
|
||||
logit_bias={"a": 1},
|
||||
metadata={"m": "v"},
|
||||
)
|
||||
assert options.model_id == "gpt-4"
|
||||
assert options.max_tokens == 1024
|
||||
assert options.temperature == 0.7
|
||||
assert options.top_p == 0.9
|
||||
assert options.presence_penalty == 0.0
|
||||
assert options.frequency_penalty == 0.0
|
||||
assert options.user == "user-123"
|
||||
for tool in options.tools:
|
||||
assert isinstance(tool, ToolProtocol)
|
||||
assert tool.name is not None
|
||||
assert tool.description is not None
|
||||
if isinstance(tool, AIFunction):
|
||||
assert tool.parameters() is not None
|
||||
|
||||
settings = options.to_provider_settings()
|
||||
assert settings["model"] == "gpt-4" # uses alias
|
||||
assert settings["tool_choice"] == "required" # serialized via model_serializer
|
||||
assert settings["custom"] is True # from additional_properties
|
||||
assert "additional_properties" not in settings
|
||||
|
||||
|
||||
def test_chat_options_tool_choice_validation_errors():
|
||||
with raises((ValidationError, TypeError)):
|
||||
with raises((ContentError, TypeError)):
|
||||
ChatOptions(tool_choice="invalid-choice")
|
||||
|
||||
|
||||
def test_chat_options_tool_choice_excluded_when_no_tools():
|
||||
options = ChatOptions(tool_choice="auto")
|
||||
settings = options.to_provider_settings()
|
||||
assert "tool_choice" not in settings
|
||||
|
||||
|
||||
def test_chat_options_and(ai_function_tool, ai_tool) -> None:
|
||||
options1 = ChatOptions(model_id="gpt-4o", tools=[ai_function_tool], logit_bias={"x": 1}, metadata={"a": "b"})
|
||||
options2 = ChatOptions(model_id="gpt-4.1", tools=[ai_tool], additional_properties={"p": 1})
|
||||
@@ -1059,69 +1017,6 @@ def test_chat_tool_mode_eq_with_string():
|
||||
assert ToolMode.AUTO == "auto"
|
||||
|
||||
|
||||
def test_chat_options_tool_choice_dict_mapping(ai_tool):
|
||||
opts = ChatOptions(tool_choice={"mode": "required", "required_function_name": "fn"}, tools=[ai_tool])
|
||||
assert isinstance(opts.tool_choice, ToolMode)
|
||||
assert opts.tool_choice.mode == "required"
|
||||
assert opts.tool_choice.required_function_name == "fn"
|
||||
# provider settings serialize to just the mode
|
||||
settings = opts.to_provider_settings()
|
||||
assert settings["tool_choice"] == "required"
|
||||
|
||||
|
||||
def test_chat_options_to_provider_settings_with_falsy_values():
|
||||
"""Test that falsy values (except None) are included in provider settings."""
|
||||
options = ChatOptions(
|
||||
temperature=0.0, # falsy but not None
|
||||
top_p=0.0, # falsy but not None
|
||||
presence_penalty=False, # falsy but not None
|
||||
frequency_penalty=None, # None - should be excluded
|
||||
additional_properties={"empty_string": "", "zero": 0, "false_flag": False, "none_value": None},
|
||||
)
|
||||
|
||||
settings = options.to_provider_settings()
|
||||
|
||||
# Falsy values that are not None should be included
|
||||
assert "temperature" in settings
|
||||
assert isinstance(settings["temperature"], float)
|
||||
assert settings["temperature"] == 0.0
|
||||
assert "top_p" in settings
|
||||
assert isinstance(settings["top_p"], float)
|
||||
assert settings["top_p"] == 0.0
|
||||
assert "presence_penalty" in settings
|
||||
assert isinstance(settings["presence_penalty"], float) # converted to float
|
||||
assert settings["presence_penalty"] == 0.0
|
||||
|
||||
# None values should be excluded
|
||||
assert "frequency_penalty" not in settings
|
||||
|
||||
# Additional properties - falsy values should always be included
|
||||
assert "empty_string" in settings
|
||||
assert settings["empty_string"] == ""
|
||||
assert "zero" in settings
|
||||
assert settings["zero"] == 0
|
||||
assert "false_flag" in settings
|
||||
assert settings["false_flag"] is False
|
||||
assert "none_value" in settings
|
||||
assert settings["none_value"] is None
|
||||
|
||||
|
||||
def test_chat_options_empty_logit_bias_and_metadata_excluded():
|
||||
"""Test that empty logit_bias and metadata are excluded from provider settings."""
|
||||
options = ChatOptions(
|
||||
model_id="gpt-4o",
|
||||
logit_bias={}, # empty dict should be excluded
|
||||
metadata={}, # empty dict should be excluded
|
||||
)
|
||||
|
||||
settings = options.to_provider_settings()
|
||||
|
||||
# Empty logit_bias and metadata should be excluded
|
||||
assert "logit_bias" not in settings
|
||||
assert "metadata" not in settings
|
||||
assert settings["model"] == "gpt-4o"
|
||||
|
||||
|
||||
# region AgentRunResponse
|
||||
|
||||
|
||||
@@ -1905,7 +1800,8 @@ def test_content_roundtrip_serialization(content_class: type[BaseContent], init_
|
||||
elif isinstance(value, dict) and hasattr(reconstructed_value, "to_dict"):
|
||||
# Compare the dict with the serialized form of the object, excluding 'type' key
|
||||
reconstructed_dict = reconstructed_value.to_dict()
|
||||
assert len(reconstructed_dict) == len(value)
|
||||
if value:
|
||||
assert len(reconstructed_dict) == len(value)
|
||||
else:
|
||||
assert reconstructed_value == value
|
||||
|
||||
|
||||
@@ -71,9 +71,7 @@ async def test_cmc(
|
||||
chat_history.append(ChatMessage(role="user", text="hello world"))
|
||||
|
||||
openai_chat_completion = OpenAIChatClient()
|
||||
await openai_chat_completion.get_response(
|
||||
messages=chat_history,
|
||||
)
|
||||
await openai_chat_completion.get_response(messages=chat_history)
|
||||
mock_create.assert_awaited_once_with(
|
||||
model=openai_unit_test_env["OPENAI_CHAT_MODEL_ID"],
|
||||
stream=False,
|
||||
@@ -189,6 +187,26 @@ async def test_cmc_general_exception(
|
||||
)
|
||||
|
||||
|
||||
@patch.object(AsyncChatCompletions, "create", new_callable=AsyncMock)
|
||||
async def test_cmc_additional_properties(
|
||||
mock_create: AsyncMock,
|
||||
chat_history: list[ChatMessage],
|
||||
mock_chat_completion_response: ChatCompletion,
|
||||
openai_unit_test_env: dict[str, str],
|
||||
):
|
||||
mock_create.return_value = mock_chat_completion_response
|
||||
chat_history.append(ChatMessage(role="user", text="hello world"))
|
||||
|
||||
openai_chat_completion = OpenAIChatClient()
|
||||
await openai_chat_completion.get_response(messages=chat_history, additional_properties={"reasoning_effort": "low"})
|
||||
mock_create.assert_awaited_once_with(
|
||||
model=openai_unit_test_env["OPENAI_CHAT_MODEL_ID"],
|
||||
stream=False,
|
||||
messages=openai_chat_completion._prepare_chat_history_for_request(chat_history), # type: ignore
|
||||
reasoning_effort="low",
|
||||
)
|
||||
|
||||
|
||||
# region Streaming
|
||||
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from openai import BadRequestError
|
||||
from openai.types.responses.response_reasoning_item import Summary
|
||||
from openai.types.responses.response_reasoning_summary_text_delta_event import ResponseReasoningSummaryTextDeltaEvent
|
||||
from openai.types.responses.response_reasoning_summary_text_done_event import ResponseReasoningSummaryTextDoneEvent
|
||||
from openai.types.responses.response_reasoning_text_delta_event import ResponseReasoningTextDeltaEvent
|
||||
@@ -209,7 +210,7 @@ def test_get_response_with_all_parameters() -> None:
|
||||
instructions="You are a helpful assistant",
|
||||
max_tokens=100,
|
||||
parallel_tool_calls=True,
|
||||
model="gpt-4",
|
||||
model_id="gpt-4",
|
||||
previous_response_id="prev-123",
|
||||
reasoning={"chain_of_thought": "enabled"},
|
||||
service_tier="auto",
|
||||
@@ -535,13 +536,13 @@ def test_response_content_creation_with_reasoning() -> None:
|
||||
mock_reasoning_item = MagicMock()
|
||||
mock_reasoning_item.type = "reasoning"
|
||||
mock_reasoning_item.content = [mock_reasoning_content]
|
||||
mock_reasoning_item.summary = ["Summary"]
|
||||
mock_reasoning_item.summary = [Summary(text="Summary", type="summary_text")]
|
||||
|
||||
mock_response.output = [mock_reasoning_item]
|
||||
|
||||
response = client._create_response_content(mock_response, chat_options=ChatOptions()) # type: ignore
|
||||
|
||||
assert len(response.messages[0].contents) == 1
|
||||
assert len(response.messages[0].contents) == 2
|
||||
assert isinstance(response.messages[0].contents[0], TextReasoningContent)
|
||||
assert response.messages[0].contents[0].text == "Reasoning step"
|
||||
|
||||
@@ -1536,11 +1537,9 @@ async def test_openai_responses_client_agent_chat_options_run_level() -> None:
|
||||
instructions="You are a helpful assistant.",
|
||||
) as agent:
|
||||
response = await agent.run(
|
||||
"Provide a brief, helpful response.",
|
||||
max_tokens=100,
|
||||
temperature=0.7,
|
||||
top_p=0.9,
|
||||
seed=123,
|
||||
"Provide a brief, helpful response about why the sky blue is.",
|
||||
max_tokens=600,
|
||||
model_id="gpt-4o",
|
||||
user="comprehensive-test-user",
|
||||
tools=[get_weather],
|
||||
tool_choice="auto",
|
||||
@@ -2077,7 +2076,6 @@ def test_prepare_options_store_parameter_handling() -> None:
|
||||
chat_options = ChatOptions(store=False, conversation_id="")
|
||||
options = client._prepare_options(messages, chat_options) # type: ignore
|
||||
assert options["store"] is False
|
||||
assert "previous_response_id" not in options
|
||||
|
||||
chat_options = ChatOptions(store=None, conversation_id=None)
|
||||
options = client._prepare_options(messages, chat_options) # type: ignore
|
||||
|
||||
Reference in New Issue
Block a user