Python: Feat: Add finish_reason support to AgentResponse and AgentResponseUpdate#5211
Python: Feat: Add finish_reason support to AgentResponse and AgentResponseUpdate#5211LEDazzio01 wants to merge 4 commits intomicrosoft:mainfrom
Conversation
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 microsoft#4622
There was a problem hiding this comment.
Pull request overview
Adds finish_reason support to agent-level response types in the Python SDK, aligning AgentResponse/AgentResponseUpdate with the existing chat response types and improving parity with the .NET SDK.
Changes:
- Added
finish_reasonfields toAgentResponseandAgentResponseUpdate. - Propagated
finish_reasonduring streaming update accumulation (_process_update) and when mappingChatResponseUpdate -> AgentResponseUpdate. - Added unit tests validating initialization, mapping, propagation behavior, and serialization.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_types.py | Adds finish_reason to agent response types and ensures it is propagated during update processing and chat→agent update mapping. |
| python/packages/core/tests/core/test_finish_reason.py | New tests covering finish_reason initialization, propagation, and serialization for agent response types. |
| from agent_framework import ( | ||
| AgentResponse, | ||
| AgentResponseUpdate, |
There was a problem hiding this comment.
Add the standard copyright header at the top of this new test file to match the rest of python/packages/core/tests/core (nearly all existing test modules start with # Copyright (c) Microsoft. All rights reserved.).
| self, | ||
| *, | ||
| messages: Message | Sequence[Message] | None = None, | ||
| 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, |
There was a problem hiding this comment.
finish_reason was added to AgentResponse.__init__, but the Keyword Args: docstring section below doesn’t document it. Please add a short description for finish_reason so the constructor docs stay in sync with the signature.
| author_name: str | None = None, | ||
| agent_id: str | None = None, | ||
| 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, |
There was a problem hiding this comment.
finish_reason was added to AgentResponseUpdate.__init__, but the constructor docstring doesn’t mention it in Keyword Args:. Please document finish_reason (and what values mean, if applicable) to keep docs consistent with the new parameter.
Add
finish_reasontoAgentResponseandAgentResponseUpdateMotivation
Resolves the gap between
ChatResponse/ChatResponseUpdate(which already havefinish_reason) and the agent-level response types. This brings the Python SDK into parity with the .NET SDK'sAgentResponseItem.FinishReasonfield.\n\nRelated: Supersedes #4958 (rebased cleanly from currentmainto resolve merge conflicts).\n\n## Changes\n\n###python/packages/core/agent_framework/_types.py\n\n1.AgentResponse.__init__— Addedfinish_reason: FinishReasonLiteral | FinishReason | None = Noneparameter andself.finish_reasoninitialization.\n2.AgentResponseUpdate.__init__— Same addition.\n3._process_update()— Added anAgentResponse/AgentResponseUpdateisinstance block to propagatefinish_reasonfrom updates to the accumulated response (mirrors the existingChatResponseblock).\n4.map_chat_to_agent_update()— Forwardsfinish_reasonfromChatResponseUpdatetoAgentResponseUpdate.\n\n###python/packages/core/tests/core/test_finish_reason.py(NEW)\n\nComprehensive unit tests covering:\n-AgentResponseandAgentResponseUpdateinitialization withfinish_reason\n-map_chat_to_agent_updateforwarding\n-_process_updatepropagation (including None-safety)\n- Serialization round-trip via.to_dict()\n\n## Testing\n\n- [x] All new tests pass locally\n- [x] No changes to existing public API signatures (additive only)\n- [x] Backward compatible —finish_reasondefaults toNone\n\n## Checklist\n\n- [x] Followed Contribution Guidelines\n- [x] Unit tests added\n- [x] No breaking changes"