mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Feat: Add finish_reason support to AgentResponse and AgentResponseUpdate (#5211)
* feat: add finish_reason support to AgentResponse and AgentResponseUpdate Add finish_reason field to AgentResponse and AgentResponseUpdate classes, propagate it through _process_update() and map_chat_to_agent_update(), and add comprehensive unit tests. Fixes #4622 * feat: add finish_reason to AgentResponse and AgentResponseUpdate * style: add copyright header to test_finish_reason.py * docs: add finish_reason to AgentResponse and AgentResponseUpdate docstrings * refactor: move finish_reason tests into test_types.py per review feedback Move all finish_reason test cases from the separate test_finish_reason.py file into test_types.py as requested by eavanvalkenburg. Tests are placed in a new '# region finish_reason' section at the end of the file. * fix: use model instead of model_id in _process_update Address PR review feedback from @eavanvalkenburg — ChatResponse and ChatResponseUpdate both use 'model', not 'model_id'. * fix: resolve SIM102 lint error in _process_update Combine nested if statements for AgentResponse finish_reason check to satisfy ruff SIM102 rule, with line wrapping to stay under 120 chars. * fix: resolve pyright reportArgumentType in map_chat_to_agent_update Add type: ignore[arg-type] for FinishReason NewType widening when passing ChatResponseUpdate.finish_reason to AgentResponseUpdate. Matches existing patterns in the codebase (40+ similar ignores).
This commit is contained in:
committed by
GitHub
Unverified
parent
90a633967c
commit
91e34358eb
@@ -1879,6 +1879,12 @@ def _process_update(response: ChatResponse | AgentResponse, update: ChatResponse
|
||||
response.finish_reason = update.finish_reason
|
||||
if update.model is not None:
|
||||
response.model = update.model
|
||||
if (
|
||||
isinstance(response, AgentResponse)
|
||||
and isinstance(update, AgentResponseUpdate)
|
||||
and update.finish_reason is not None
|
||||
):
|
||||
response.finish_reason = update.finish_reason
|
||||
response.continuation_token = update.continuation_token
|
||||
|
||||
|
||||
@@ -2435,6 +2441,7 @@ class AgentResponse(SerializationMixin, Generic[ResponseModelT]):
|
||||
response_id: str | None = None,
|
||||
agent_id: str | None = None,
|
||||
created_at: CreatedAtT | None = None,
|
||||
finish_reason: FinishReasonLiteral | FinishReason | None = None,
|
||||
usage_details: UsageDetails | None = None,
|
||||
value: ResponseModelT | None = None,
|
||||
response_format: StructuredResponseFormat = None,
|
||||
@@ -2450,6 +2457,9 @@ class AgentResponse(SerializationMixin, Generic[ResponseModelT]):
|
||||
agent_id: The identifier of the agent that produced this response. Useful in multi-agent
|
||||
scenarios to track which agent generated the response.
|
||||
created_at: A timestamp for the chat response.
|
||||
finish_reason: The reason the model stopped generating. Common values include
|
||||
``"stop"`` (natural completion), ``"length"`` (token limit), and
|
||||
``"tool_calls"`` (the model invoked a tool).
|
||||
usage_details: The usage details for the chat response.
|
||||
value: The structured output of the agent run response, if applicable.
|
||||
response_format: Optional response format for the agent response.
|
||||
@@ -2476,6 +2486,7 @@ class AgentResponse(SerializationMixin, Generic[ResponseModelT]):
|
||||
self.response_id = response_id
|
||||
self.agent_id = agent_id
|
||||
self.created_at = created_at
|
||||
self.finish_reason = finish_reason
|
||||
self.usage_details = usage_details
|
||||
self._value: ResponseModelT | None = value
|
||||
self._response_format: type[BaseModel] | Mapping[str, Any] | None = response_format
|
||||
@@ -2688,6 +2699,7 @@ class AgentResponseUpdate(SerializationMixin):
|
||||
response_id: str | None = None,
|
||||
message_id: str | None = None,
|
||||
created_at: CreatedAtT | None = None,
|
||||
finish_reason: FinishReasonLiteral | FinishReason | None = None,
|
||||
continuation_token: ContinuationToken | None = None,
|
||||
additional_properties: dict[str, Any] | None = None,
|
||||
raw_representation: Any | None = None,
|
||||
@@ -2703,6 +2715,9 @@ class AgentResponseUpdate(SerializationMixin):
|
||||
response_id: Optional ID of the response of which this update is a part.
|
||||
message_id: Optional ID of the message of which this update is a part.
|
||||
created_at: Optional timestamp for the chat response update.
|
||||
finish_reason: The reason the model stopped generating. Common values include
|
||||
``"stop"`` (natural completion), ``"length"`` (token limit), and
|
||||
``"tool_calls"`` (the model invoked a tool).
|
||||
continuation_token: Optional token for resuming a long-running background operation.
|
||||
When present, indicates the operation is still in progress.
|
||||
additional_properties: Optional additional properties associated with the chat response update.
|
||||
@@ -2729,6 +2744,7 @@ class AgentResponseUpdate(SerializationMixin):
|
||||
self.response_id = response_id
|
||||
self.message_id = message_id
|
||||
self.created_at = created_at
|
||||
self.finish_reason = finish_reason
|
||||
self.continuation_token = continuation_token
|
||||
self.additional_properties = _restore_compaction_annotation_in_additional_properties(
|
||||
additional_properties,
|
||||
@@ -2761,6 +2777,7 @@ def map_chat_to_agent_update(update: ChatResponseUpdate, agent_name: str | None)
|
||||
response_id=update.response_id,
|
||||
message_id=update.message_id,
|
||||
created_at=update.created_at,
|
||||
finish_reason=update.finish_reason, # type: ignore[arg-type]
|
||||
continuation_token=update.continuation_token,
|
||||
additional_properties=update.additional_properties,
|
||||
raw_representation=update,
|
||||
|
||||
@@ -40,8 +40,10 @@ from agent_framework._types import (
|
||||
_get_data_bytes_as_str,
|
||||
_parse_content_list,
|
||||
_parse_structured_response_value,
|
||||
_process_update,
|
||||
_validate_uri,
|
||||
add_usage_details,
|
||||
map_chat_to_agent_update,
|
||||
validate_tool_mode,
|
||||
)
|
||||
from agent_framework.exceptions import AdditionItemMismatch, ContentError
|
||||
@@ -4179,3 +4181,101 @@ def test_prepend_instructions_custom_role():
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
|
||||
# region finish_reason
|
||||
|
||||
|
||||
def test_agent_response_init_with_finish_reason() -> None:
|
||||
"""Test that AgentResponse correctly initializes and stores finish_reason."""
|
||||
response = AgentResponse(
|
||||
messages=[Message("assistant", [Content.from_text("test")])],
|
||||
finish_reason="stop",
|
||||
)
|
||||
assert response.finish_reason == "stop"
|
||||
|
||||
|
||||
def test_agent_response_update_init_with_finish_reason() -> None:
|
||||
"""Test that AgentResponseUpdate correctly initializes and stores finish_reason."""
|
||||
update = AgentResponseUpdate(
|
||||
contents=[Content.from_text("test")],
|
||||
role="assistant",
|
||||
finish_reason="stop",
|
||||
)
|
||||
assert update.finish_reason == "stop"
|
||||
|
||||
|
||||
def test_map_chat_to_agent_update_forwards_finish_reason() -> None:
|
||||
"""Test that mapping a ChatResponseUpdate with finish_reason forwards it."""
|
||||
chat_update = ChatResponseUpdate(
|
||||
contents=[Content.from_text("test")],
|
||||
finish_reason="length",
|
||||
)
|
||||
agent_update = map_chat_to_agent_update(chat_update, agent_name="test_agent")
|
||||
|
||||
assert agent_update.finish_reason == "length"
|
||||
assert agent_update.author_name == "test_agent"
|
||||
|
||||
|
||||
def test_process_update_propagates_finish_reason_to_agent_response() -> None:
|
||||
"""Test that _process_update correctly updates an AgentResponse from an AgentResponseUpdate."""
|
||||
response = AgentResponse(messages=[Message("assistant", [Content.from_text("test")])])
|
||||
update = AgentResponseUpdate(
|
||||
contents=[Content.from_text("more text")],
|
||||
role="assistant",
|
||||
finish_reason="stop",
|
||||
)
|
||||
|
||||
# Process the update
|
||||
_process_update(response, update)
|
||||
|
||||
assert response.finish_reason == "stop"
|
||||
|
||||
|
||||
def test_process_update_does_not_overwrite_with_none() -> None:
|
||||
"""Test that _process_update does not overwrite an existing finish_reason with None."""
|
||||
response = AgentResponse(
|
||||
messages=[Message("assistant", [Content.from_text("test")])],
|
||||
finish_reason="length",
|
||||
)
|
||||
update = AgentResponseUpdate(
|
||||
contents=[Content.from_text("more text")],
|
||||
role="assistant",
|
||||
finish_reason=None,
|
||||
)
|
||||
|
||||
# Process the update
|
||||
_process_update(response, update)
|
||||
|
||||
assert response.finish_reason == "length"
|
||||
|
||||
|
||||
def test_agent_response_serialization_includes_finish_reason() -> None:
|
||||
"""Test that AgentResponse serializes correctly, including finish_reason."""
|
||||
response = AgentResponse(
|
||||
messages=[Message("assistant", [Content.from_text("test")])],
|
||||
response_id="test_123",
|
||||
finish_reason="stop",
|
||||
)
|
||||
|
||||
# Serialize using the framework's API and verify finish_reason is included.
|
||||
data = response.to_dict()
|
||||
assert "finish_reason" in data
|
||||
assert data["finish_reason"] == "stop"
|
||||
|
||||
|
||||
def test_agent_response_update_serialization_includes_finish_reason() -> None:
|
||||
"""Test that AgentResponseUpdate serializes correctly, including finish_reason."""
|
||||
update = AgentResponseUpdate(
|
||||
contents=[Content.from_text("test")],
|
||||
role="assistant",
|
||||
response_id="test_456",
|
||||
finish_reason="tool_calls",
|
||||
)
|
||||
|
||||
data = update.to_dict()
|
||||
assert "finish_reason" in data
|
||||
assert data["finish_reason"] == "tool_calls"
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
Reference in New Issue
Block a user