mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix conversation-id propagation when chat_options is a dict (#4340)
* Fix #4305: Handle dict chat_options in _update_conversation_id _update_conversation_id assumed chat_options had attribute access, but ChatOptions is a TypedDict (dict). When a dict was passed, setting .conversation_id raised AttributeError. Now checks isinstance(dict) and uses key access for dicts, falling back to attribute access for objects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR feedback: use Mapping ABC and add missing tests (#4305) - Use collections.abc.Mapping instead of dict for isinstance check in _update_conversation_id, making it more robust for non-dict mapping types. - Add test for object-style chat_options with optional options dict parameter. - Add test verifying existing conversation_id gets overwritten (idempotent). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove unnecessary Mapping check in _update_conversation_id (#4305) chat_options is always a dict, so the isinstance(chat_opts, Mapping) check and the else branch for attribute-style access are dead code. Simplify to direct dict key assignment and remove object-style tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
23d6d91c8f
commit
f74bda5a83
@@ -1458,7 +1458,7 @@ def _update_conversation_id(
|
||||
if conversation_id is None:
|
||||
return
|
||||
if "chat_options" in kwargs:
|
||||
kwargs["chat_options"].conversation_id = conversation_id
|
||||
kwargs["chat_options"]["conversation_id"] = conversation_id
|
||||
else:
|
||||
kwargs["conversation_id"] = conversation_id
|
||||
|
||||
|
||||
@@ -3449,3 +3449,66 @@ async def test_streaming_function_calling_response_includes_reasoning_and_tool_r
|
||||
reasoning_contents = [c for msg in response.messages for c in msg.contents if c.type == "text_reasoning"]
|
||||
assert len(reasoning_contents) >= 1
|
||||
assert reasoning_contents[0].id == "rs_test123"
|
||||
|
||||
|
||||
# region _update_conversation_id unit tests
|
||||
|
||||
|
||||
class TestUpdateConversationId:
|
||||
"""Tests for _update_conversation_id handling dict chat_options."""
|
||||
|
||||
def test_chat_options_as_dict(self):
|
||||
"""When chat_options is a plain dict, conversation_id should be set via key access."""
|
||||
from agent_framework._tools import _update_conversation_id
|
||||
|
||||
kwargs: dict[str, Any] = {"chat_options": {}}
|
||||
_update_conversation_id(kwargs, "conv_1")
|
||||
assert kwargs["chat_options"]["conversation_id"] == "conv_1"
|
||||
|
||||
def test_chat_options_as_typed_dict(self):
|
||||
"""When chat_options is a ChatOptions TypedDict, conversation_id should be set via key access."""
|
||||
from agent_framework import ChatOptions
|
||||
from agent_framework._tools import _update_conversation_id
|
||||
|
||||
opts: ChatOptions = {"temperature": 0.5}
|
||||
kwargs: dict[str, Any] = {"chat_options": opts}
|
||||
_update_conversation_id(kwargs, "conv_2")
|
||||
assert kwargs["chat_options"]["conversation_id"] == "conv_2"
|
||||
|
||||
def test_no_chat_options_falls_back_to_kwargs(self):
|
||||
"""When chat_options is absent, conversation_id should be set directly on kwargs."""
|
||||
from agent_framework._tools import _update_conversation_id
|
||||
|
||||
kwargs: dict[str, Any] = {}
|
||||
_update_conversation_id(kwargs, "conv_4")
|
||||
assert kwargs["conversation_id"] == "conv_4"
|
||||
|
||||
def test_none_conversation_id_is_noop(self):
|
||||
"""When conversation_id is None, kwargs should not be modified."""
|
||||
from agent_framework._tools import _update_conversation_id
|
||||
|
||||
kwargs: dict[str, Any] = {"chat_options": {}}
|
||||
_update_conversation_id(kwargs, None)
|
||||
assert "conversation_id" not in kwargs["chat_options"]
|
||||
assert "conversation_id" not in kwargs
|
||||
|
||||
def test_options_dict_also_updated(self):
|
||||
"""The optional options dict should also receive conversation_id."""
|
||||
from agent_framework._tools import _update_conversation_id
|
||||
|
||||
kwargs: dict[str, Any] = {"chat_options": {}}
|
||||
options: dict[str, Any] = {}
|
||||
_update_conversation_id(kwargs, "conv_5", options)
|
||||
assert kwargs["chat_options"]["conversation_id"] == "conv_5"
|
||||
assert options["conversation_id"] == "conv_5"
|
||||
|
||||
def test_dict_overwrites_existing_conversation_id(self):
|
||||
"""When a dict already has a conversation_id, it should be overwritten."""
|
||||
from agent_framework._tools import _update_conversation_id
|
||||
|
||||
kwargs: dict[str, Any] = {"chat_options": {"conversation_id": "old_id"}}
|
||||
_update_conversation_id(kwargs, "new_id")
|
||||
assert kwargs["chat_options"]["conversation_id"] == "new_id"
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
Reference in New Issue
Block a user