mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
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 <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
fe08574a7c
commit
4268080c20
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
+4
-3
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user