From 4268080c20f382655d9c25c8209839fa3f1d26c9 Mon Sep 17 00:00:00 2001 From: Evan Mattson <35585003+moonbox3@users.noreply.github.com> Date: Thu, 4 Jun 2026 17:59:04 +0900 Subject: [PATCH] Python: Fix spurious Magentic custom manager warning (#6261) * Fix magentic manager warning * Use typing_extensions.Sentinel for _MISSING sentinel value Replace the bare object() sentinel with typing_extensions.Sentinel per PEP 661 (now final). Sentinel provides a proper name and repr ('<_MISSING>') and is the idiomatic approach going forward. Refs #4306 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct Sentinel type annotation for max_stall_count param (#6261) Use int | Sentinel for max_stall_count parameter type annotation instead of int with cast(Any, _MISSING) to properly express that the parameter can hold either an int or the _MISSING sentinel value. This fixes the pyright reportUnnecessaryComparison errors caused by the types int and Sentinel having no overlap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename _MISSING sentinel to UNSET in orchestrations The sentinel is user-visible as a default in public init signatures, so use UNSET (no leading underscore) instead of the private _MISSING name. Drop the now-unnecessary reportPrivateUsage ignores on the UNSET imports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agent_framework_bedrock/_chat_client.py | 13 ++---- .../tests/test_bedrock_structured_output.py | 1 + .../foundry_hosting/tests/test_responses.py | 8 +--- .../_concurrent.py | 4 +- .../_group_chat.py | 4 +- .../_handoff.py | 4 +- .../_magentic.py | 19 +++++---- .../_participant_output_config.py | 7 ++-- .../_sequential.py | 4 +- .../orchestrations/tests/test_magentic.py | 42 +++++++++++++++++++ 10 files changed, 70 insertions(+), 36 deletions(-) diff --git a/python/packages/bedrock/agent_framework_bedrock/_chat_client.py b/python/packages/bedrock/agent_framework_bedrock/_chat_client.py index cb8545f9a3..2fd7887721 100644 --- a/python/packages/bedrock/agent_framework_bedrock/_chat_client.py +++ b/python/packages/bedrock/agent_framework_bedrock/_chat_client.py @@ -795,10 +795,7 @@ class BedrockChatClient( schema = copy.deepcopy(schema_src) else: if not isinstance(response_format, type) or not issubclass(response_format, BaseModel): - raise TypeError( - "response_format must be None, a dict JSON schema, " - "or a Pydantic BaseModel subclass." - ) + raise TypeError("response_format must be None, a dict JSON schema, or a Pydantic BaseModel subclass.") # response_format is a Pydantic model class schema = response_format.model_json_schema() name = response_format.__name__ @@ -817,9 +814,7 @@ class BedrockChatClient( return { "textFormat": { "type": "json_schema", - "structure": { - "jsonSchema": json_schema - }, + "structure": {"jsonSchema": json_schema}, } } @@ -840,9 +835,7 @@ class BedrockChatClient( if node_id in visited: return visited.add(node_id) - if node.get("type") == "object" or ( - "properties" in node and "type" not in node - ): + if node.get("type") == "object" or ("properties" in node and "type" not in node): existing = node.get("additionalProperties") if existing is None or existing is True: node["additionalProperties"] = False diff --git a/python/packages/bedrock/tests/test_bedrock_structured_output.py b/python/packages/bedrock/tests/test_bedrock_structured_output.py index 8df04b5e75..7b39f67d69 100644 --- a/python/packages/bedrock/tests/test_bedrock_structured_output.py +++ b/python/packages/bedrock/tests/test_bedrock_structured_output.py @@ -238,6 +238,7 @@ async def test_chat_response_value_populated_streaming() -> None: async def test_unsupported_model_validation_exception() -> None: """When a model doesn't support outputConfig, a clear error should be raised.""" + class _FailingStubBedrockRuntime: def converse(self, **kwargs: Any) -> dict[str, Any]: # Simulate botocore ClientError for ValidationException diff --git a/python/packages/foundry_hosting/tests/test_responses.py b/python/packages/foundry_hosting/tests/test_responses.py index 0bfff345a7..9c65a9ea42 100644 --- a/python/packages/foundry_hosting/tests/test_responses.py +++ b/python/packages/foundry_hosting/tests/test_responses.py @@ -2118,15 +2118,11 @@ class TestMultiTurnMixedContent: assert resp2.json()["status"] == "completed" second_call_messages = agent.run.call_args_list[1].kwargs["messages"] - mcp_call_contents = [ - c for m in second_call_messages for c in m.contents if c.type == "mcp_server_tool_call" - ] + mcp_call_contents = [c for m in second_call_messages for c in m.contents if c.type == "mcp_server_tool_call"] mcp_result_contents = [ c for m in second_call_messages for c in m.contents if c.type == "mcp_server_tool_result" ] - function_result_contents = [ - c for m in second_call_messages for c in m.contents if c.type == "function_result" - ] + function_result_contents = [c for m in second_call_messages for c in m.contents if c.type == "function_result"] assert len(mcp_call_contents) >= 1 assert len(mcp_result_contents) >= 1 diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_concurrent.py b/python/packages/orchestrations/agent_framework_orchestrations/_concurrent.py index 6fc29c79b3..9db8878ef4 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_concurrent.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_concurrent.py @@ -19,7 +19,7 @@ from typing_extensions import Never from ._orchestration_request_info import AgentApprovalExecutor from ._participant_output_config import ( - _MISSING, # pyright: ignore[reportPrivateUsage] + UNSET, _coalesce_output_from, # pyright: ignore[reportPrivateUsage] _coerce_intermediate_output_from, # pyright: ignore[reportPrivateUsage] _ParticipantIntermediateOutputSelection, # pyright: ignore[reportPrivateUsage] @@ -213,7 +213,7 @@ class ConcurrentBuilder: *, participants: Sequence[SupportsAgentRun | Executor], checkpoint_storage: CheckpointStorage | None = None, - output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, _MISSING), + output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, UNSET), intermediate_output_from: _ParticipantIntermediateOutputSelection = None, ) -> None: """Initialize the ConcurrentBuilder. diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_group_chat.py b/python/packages/orchestrations/agent_framework_orchestrations/_group_chat.py index 3778e5d110..728f3e388c 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_group_chat.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_group_chat.py @@ -52,7 +52,7 @@ from ._base_group_chat_orchestrator import ( from ._orchestration_request_info import AgentApprovalExecutor from ._orchestrator_helpers import clean_conversation_for_handoff from ._participant_output_config import ( - _MISSING, # pyright: ignore[reportPrivateUsage] + UNSET, _coalesce_output_from, # pyright: ignore[reportPrivateUsage] _coerce_intermediate_output_from, # pyright: ignore[reportPrivateUsage] _ParticipantIntermediateOutputSelection, # pyright: ignore[reportPrivateUsage] @@ -626,7 +626,7 @@ class GroupChatBuilder: termination_condition: TerminationCondition | None = None, max_rounds: int | None = None, checkpoint_storage: CheckpointStorage | None = None, - output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, _MISSING), + output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, UNSET), intermediate_output_from: _ParticipantIntermediateOutputSelection = None, ) -> None: """Initialize the GroupChatBuilder. diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py b/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py index 70f28e7f04..65da3b8709 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py @@ -54,7 +54,7 @@ from agent_framework._workflows._workflow_context import WorkflowContext from ._base_group_chat_orchestrator import TerminationCondition from ._orchestrator_helpers import clean_conversation_for_handoff from ._participant_output_config import ( - _MISSING, # pyright: ignore[reportPrivateUsage] + UNSET, _coalesce_output_from, # pyright: ignore[reportPrivateUsage] _coerce_intermediate_output_from, # pyright: ignore[reportPrivateUsage] _ParticipantIntermediateOutputSelection, # pyright: ignore[reportPrivateUsage] @@ -597,7 +597,7 @@ class HandoffBuilder: description: str | None = None, checkpoint_storage: CheckpointStorage | None = None, termination_condition: TerminationCondition | None = None, - output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, _MISSING), + output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, UNSET), intermediate_output_from: _ParticipantIntermediateOutputSelection = None, ) -> None: r"""Initialize a HandoffBuilder for creating conversational handoff workflows. diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py b/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py index 53ca4052ff..f8cbf88fd7 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py @@ -28,7 +28,7 @@ from agent_framework._workflows._request_info_mixin import response_handler from agent_framework._workflows._workflow import Workflow from agent_framework._workflows._workflow_builder import WorkflowBuilder from agent_framework._workflows._workflow_context import WorkflowContext -from typing_extensions import Never +from typing_extensions import Never, Sentinel from ._base_group_chat_orchestrator import ( BaseGroupChatOrchestrator, @@ -39,7 +39,7 @@ from ._base_group_chat_orchestrator import ( ParticipantRegistry, ) from ._participant_output_config import ( - _MISSING, # pyright: ignore[reportPrivateUsage] + UNSET, _coalesce_output_from, # pyright: ignore[reportPrivateUsage] _coerce_intermediate_output_from, # pyright: ignore[reportPrivateUsage] _ParticipantIntermediateOutputSelection, # pyright: ignore[reportPrivateUsage] @@ -1411,13 +1411,13 @@ class MagenticBuilder: task_ledger_plan_update_prompt: str | None = None, progress_ledger_prompt: str | None = None, final_answer_prompt: str | None = None, - max_stall_count: int = 3, + max_stall_count: int | Sentinel = UNSET, max_reset_count: int | None = None, max_round_count: int | None = None, # Existing params enable_plan_review: bool = False, checkpoint_storage: CheckpointStorage | None = None, - output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, _MISSING), + output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, UNSET), intermediate_output_from: _ParticipantIntermediateOutputSelection = None, ) -> None: """Initialize the Magentic workflow builder. @@ -1621,7 +1621,7 @@ class MagenticBuilder: progress_ledger_prompt: str | None = None, final_answer_prompt: str | None = None, # Limits - max_stall_count: int = 3, + max_stall_count: int | Sentinel = UNSET, max_reset_count: int | None = None, max_round_count: int | None = None, ) -> None: @@ -1656,8 +1656,10 @@ class MagenticBuilder: "Exactly one of manager, manager_agent, manager_factory, or manager_agent_factory must be provided." ) + resolved_max_stall_count: int = 3 if max_stall_count is UNSET else cast(int, max_stall_count) + def _log_warning_if_constructor_args_provided() -> None: - if any( + if max_stall_count is not UNSET or any( arg is not None for arg in [ task_ledger, @@ -1668,7 +1670,6 @@ class MagenticBuilder: task_ledger_plan_update_prompt, progress_ledger_prompt, final_answer_prompt, - max_stall_count, max_reset_count, max_round_count, ] @@ -1689,7 +1690,7 @@ class MagenticBuilder: task_ledger_plan_update_prompt=task_ledger_plan_update_prompt, progress_ledger_prompt=progress_ledger_prompt, final_answer_prompt=final_answer_prompt, - max_stall_count=max_stall_count, + max_stall_count=resolved_max_stall_count, max_reset_count=max_reset_count, max_round_count=max_round_count, ) @@ -1707,7 +1708,7 @@ class MagenticBuilder: "task_ledger_plan_update_prompt": task_ledger_plan_update_prompt, "progress_ledger_prompt": progress_ledger_prompt, "final_answer_prompt": final_answer_prompt, - "max_stall_count": max_stall_count, + "max_stall_count": resolved_max_stall_count, "max_reset_count": max_reset_count, "max_round_count": max_round_count, } diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_participant_output_config.py b/python/packages/orchestrations/agent_framework_orchestrations/_participant_output_config.py index 49138b7d0d..dfb22fc85a 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_participant_output_config.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_participant_output_config.py @@ -8,8 +8,9 @@ from typing import Any, Literal from agent_framework import SupportsAgentRun from agent_framework._workflows._agent_utils import resolve_agent_id from agent_framework._workflows._executor import Executor +from typing_extensions import Sentinel -_MISSING = object() +UNSET = Sentinel("UNSET") _ALL_OUTPUTS: Literal["all"] = "all" _ALL_OTHER_OUTPUTS: Literal["all_other"] = "all_other" _ParticipantOutputSpecifier = str | SupportsAgentRun | Executor @@ -20,10 +21,10 @@ _WorkflowExecutorSpecifier = Executor | SupportsAgentRun def _coalesce_output_from( # pyright: ignore[reportUnusedFunction] *, - output_from: Any = _MISSING, + output_from: Any = UNSET, ) -> _ParticipantOutputSelection: """Resolve orchestration output selection to ``output_from``.""" - if output_from is not _MISSING: + if output_from is not UNSET: return _coerce_output_from(output_from) return None diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_sequential.py b/python/packages/orchestrations/agent_framework_orchestrations/_sequential.py index 70796d5e26..4f8720b0bf 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_sequential.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_sequential.py @@ -33,7 +33,7 @@ from agent_framework._workflows._workflow_context import WorkflowContext from ._orchestration_request_info import AgentApprovalExecutor from ._participant_output_config import ( - _MISSING, # pyright: ignore[reportPrivateUsage] + UNSET, _coalesce_output_from, # pyright: ignore[reportPrivateUsage] _coerce_intermediate_output_from, # pyright: ignore[reportPrivateUsage] _ParticipantIntermediateOutputSelection, # pyright: ignore[reportPrivateUsage] @@ -99,7 +99,7 @@ class SequentialBuilder: participants: Sequence[SupportsAgentRun | Executor], checkpoint_storage: CheckpointStorage | None = None, chain_only_agent_responses: bool = False, - output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, _MISSING), + output_from: Sequence[_ParticipantOutputSpecifier] | Literal["all"] | None = cast(Any, UNSET), intermediate_output_from: _ParticipantIntermediateOutputSelection = None, ) -> None: """Initialize the SequentialBuilder. diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index 5c94d2fb14..615ba998bc 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import logging import sys from collections.abc import AsyncIterable, Awaitable, Sequence from dataclasses import dataclass @@ -987,6 +988,33 @@ def test_magentic_builder_requires_exactly_one_manager_option(): MagenticBuilder(participants=[agent], manager=manager, manager_factory=manager_factory) +def test_magentic_with_custom_manager_does_not_warn_without_standard_manager_options(caplog: Any) -> None: + caplog.set_level(logging.WARNING, logger="agent_framework_orchestrations._magentic") + + MagenticBuilder(participants=[StubAgent("agentA", "reply")], manager=FakeManager()) + + assert "Custom manager provided; all other manager arguments will be ignored." not in caplog.text + + +def test_magentic_with_custom_manager_factory_does_not_warn_without_standard_manager_options(caplog: Any) -> None: + caplog.set_level(logging.WARNING, logger="agent_framework_orchestrations._magentic") + + def manager_factory() -> MagenticManagerBase: + return FakeManager() + + MagenticBuilder(participants=[StubAgent("agentA", "reply")], manager_factory=manager_factory) + + assert "Custom manager provided; all other manager arguments will be ignored." not in caplog.text + + +def test_magentic_with_custom_manager_warns_when_standard_manager_option_is_provided(caplog: Any) -> None: + caplog.set_level(logging.WARNING, logger="agent_framework_orchestrations._magentic") + + MagenticBuilder(participants=[StubAgent("agentA", "reply")], manager=FakeManager(), max_stall_count=3) + + assert "Custom manager provided; all other manager arguments will be ignored." in caplog.text + + async def test_magentic_with_manager_factory(): """Test workflow creation using manager_factory.""" factory_call_count = 0 @@ -1037,6 +1065,20 @@ async def test_magentic_with_agent_factory(): assert event_count > 0 +def test_magentic_agent_factory_uses_default_max_stall_count() -> None: + def agent_factory() -> SupportsAgentRun: + return cast(SupportsAgentRun, StubManagerAgent()) + + participant = StubAgent("agentA", "reply from agentA") + workflow = MagenticBuilder(participants=[participant], manager_agent_factory=agent_factory).build() + + orchestrator = next(e for e in workflow.executors.values() if isinstance(e, MagenticOrchestrator)) + manager = orchestrator._manager # type: ignore[reportPrivateUsage] + + assert isinstance(manager, StandardMagenticManager) + assert manager.max_stall_count == 3 + + async def test_magentic_manager_factory_reusable_builder(): """Test that the builder can be reused to build multiple workflows with manager factory.""" factory_call_count = 0