Python: Fix conversation-id propagation when chat_options is a dict

[moonbox3]

Motivation and Context

When chat_options was passed as a plain dict (or TypedDict) in kwargs, _update_conversation_id crashed with an AttributeError because it unconditionally used attribute assignment (chat_opts.conversation_id = ...) instead of key-based access.

Fixes #4305

Description

The root cause was that _update_conversation_id assumed chat_options was always an object supporting attribute assignment, but callers could pass it as a dict or TypedDict. The fix adds an isinstance(chat_opts, dict) check and uses dictionary key assignment for dict-like values while preserving attribute assignment for object-style options. Unit tests covering dict, TypedDict, object, fallback, and no-op cases were added to prevent regression.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the Contribution Guidelines
• All unit tests pass, and I have added new tests where possible
• Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

---

Note: PR autogenerated by moonbox3's agent

[moonbox3]

Automated Code Review

Reviewers: 3 | Confidence: 92%

✓ Correctness

The change correctly extends _update_conversation_id to handle chat_options passed as a plain dict (or TypedDict) in addition to an object with attribute access. The isinstance(chat_opts, dict) check is correct since TypedDict instances are dicts at runtime, and the fallback to attribute-based access for non-dict objects preserves existing behavior. Tests cover the key scenarios adequately. No correctness issues found.

✓ Security Reliability

This diff adds a type check so that _update_conversation_id handles chat_options as either a dict (key access) or an object (attribute access), along with thorough unit tests. The change is straightforward and low-risk from a security and reliability perspective. No injection risks, secrets, resource leaks, or unsafe deserialization are introduced. The isinstance check is a reasonable way to distinguish the two cases.

✓ Test Coverage

The new tests in TestUpdateConversationId provide solid coverage of the dict-vs-object branching logic added to _update_conversation_id. All key code paths are exercised: dict chat_options, TypedDict chat_options, object chat_options, absent chat_options, None conversation_id early return, and the optional options parameter. Assertions are meaningful and verify correct values rather than just absence of exceptions. No blocking issues found.

Suggestions

• Minor: consider whether a Mapping ABC check (isinstance(chat_opts, collections.abc.Mapping)) would be more robust than isinstance(chat_opts, dict), in case a non-dict mapping type is ever passed. Not blocking since current callers only use dict/TypedDict/objects.
• Consider whether a TypedDict subclass could bypass the isinstance(chat_opts, dict) check in edge cases (it shouldn't, since TypedDict instances are plain dicts at runtime, but worth a mental note).
• Consider adding a test that passes an object-style chat_options together with the optional options dict to verify both are updated correctly in the attribute-access branch (currently test_options_dict_also_updated only exercises the dict branch).
• Consider a test where the dict already contains a previous conversation_id to verify it gets overwritten, confirming idempotent behavior.

---

Automated review by moonbox3's agents

[Copilot]

Pull request overview

Fixes conversation-id propagation during function invocation when chat_options is provided via kwargs as a plain dict/TypedDict, avoiding an AttributeError and ensuring the conversation id is correctly carried forward across tool iterations.

Changes:

• Update _update_conversation_id to use key assignment when kwargs["chat_options"] is a dict.
• Add unit tests covering dict, TypedDict, object-style options, kwargs fallback behavior, no-op on None, and updating the optional options dict.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
python/packages/core/agent_framework/_tools.py	Fixes _update_conversation_id to support dict-based chat_options by using key assignment.
python/packages/core/tests/core/test_function_invocation_logic.py	Adds regression tests for _update_conversation_id across dict/object/fallback/no-op cases.

[markwallace-microsoft]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/core/agent_framework
_tools.py	879	89	89%	166–167, 322, 324, 342–344, 351, 369, 383, 390, 397, 413, 415, 422, 459, 484, 488, 505–507, 554–556, 578, 632, 654, 717–723, 759, 770–781, 803–805, 810, 814, 828–830, 869, 938, 948, 958, 1014, 1045, 1064, 1342, 1399, 1419, 1490–1494, 1616, 1620, 1644, 1692, 1694, 1779, 1809, 1829, 1831, 1884, 1947, 2138–2139, 2187, 2255–2256, 2314, 2319, 2326
TOTAL	22225	2756	87%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
4720	247 💤	0 ❌	0 🔥	1m 19s ⏱️

[eavanvalkenburg]

Chat options should always be a dict, afaik