mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: extend HITL support for all orchestration patterns (#2620)
* Support HITL for orchestration patterns * Cleanup around naming * Fix typing issues * Clean up * Naming clean up * Updates to HITL to make it cleaner * Rename human input hook to orchestration request info * Clean up per PR feedback
This commit is contained in:
committed by
GitHub
Unverified
parent
0d9ae1920d
commit
b378ca75d1
@@ -1082,3 +1082,106 @@ def test_set_manager_builds_with_agent_manager() -> None:
|
||||
|
||||
assert isinstance(orchestrator, GroupChatOrchestratorExecutor)
|
||||
assert orchestrator._is_manager_agent()
|
||||
|
||||
|
||||
async def test_group_chat_with_request_info_filtering():
|
||||
"""Test that with_request_info(agents=[...]) only pauses before specified agents run."""
|
||||
from agent_framework import AgentInputRequest, RequestInfoEvent
|
||||
|
||||
# Create agents - we want to verify only beta triggers pause
|
||||
alpha = StubAgent("alpha", "response from alpha")
|
||||
beta = StubAgent("beta", "response from beta")
|
||||
|
||||
# Manager that selects alpha first, then beta, then finishes
|
||||
call_count = 0
|
||||
|
||||
async def selector(state: GroupChatStateSnapshot) -> str | None:
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
return "alpha"
|
||||
if call_count == 2:
|
||||
return "beta"
|
||||
return None
|
||||
|
||||
workflow = (
|
||||
GroupChatBuilder()
|
||||
.set_select_speakers_func(selector, display_name="manager", final_message="done")
|
||||
.participants(alpha=alpha, beta=beta)
|
||||
.with_request_info(agents=["beta"]) # Only pause before beta runs
|
||||
.build()
|
||||
)
|
||||
|
||||
# Run until we get a request info event (should be before beta, not alpha)
|
||||
request_events: list[RequestInfoEvent] = []
|
||||
async for event in workflow.run_stream("test task"):
|
||||
if isinstance(event, RequestInfoEvent) and isinstance(event.data, AgentInputRequest):
|
||||
request_events.append(event)
|
||||
# Don't break - let stream complete naturally when paused
|
||||
|
||||
# Should have exactly one request event before beta
|
||||
assert len(request_events) == 1
|
||||
request_event = request_events[0]
|
||||
|
||||
# The target agent should be beta's executor ID (groupchat_agent:beta)
|
||||
assert request_event.data.target_agent_id is not None
|
||||
assert "beta" in request_event.data.target_agent_id
|
||||
|
||||
# Continue the workflow with a response
|
||||
outputs: list[WorkflowOutputEvent] = []
|
||||
async for event in workflow.send_responses_streaming({request_event.request_id: "continue please"}):
|
||||
if isinstance(event, WorkflowOutputEvent):
|
||||
outputs.append(event)
|
||||
|
||||
# Workflow should complete
|
||||
assert len(outputs) == 1
|
||||
|
||||
|
||||
async def test_group_chat_with_request_info_no_filter_pauses_all():
|
||||
"""Test that with_request_info() without agents pauses before all participants."""
|
||||
from agent_framework import AgentInputRequest, RequestInfoEvent
|
||||
|
||||
# Create agents
|
||||
alpha = StubAgent("alpha", "response from alpha")
|
||||
|
||||
# Manager selects alpha then finishes
|
||||
call_count = 0
|
||||
|
||||
async def selector(state: GroupChatStateSnapshot) -> str | None:
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
return "alpha"
|
||||
return None
|
||||
|
||||
workflow = (
|
||||
GroupChatBuilder()
|
||||
.set_select_speakers_func(selector, display_name="manager", final_message="done")
|
||||
.participants(alpha=alpha)
|
||||
.with_request_info() # No filter - pause for all
|
||||
.build()
|
||||
)
|
||||
|
||||
# Run until we get a request info event
|
||||
request_events: list[RequestInfoEvent] = []
|
||||
async for event in workflow.run_stream("test task"):
|
||||
if isinstance(event, RequestInfoEvent) and isinstance(event.data, AgentInputRequest):
|
||||
request_events.append(event)
|
||||
break
|
||||
|
||||
# Should pause before alpha
|
||||
assert len(request_events) == 1
|
||||
assert request_events[0].data.target_agent_id is not None
|
||||
assert "alpha" in request_events[0].data.target_agent_id
|
||||
|
||||
|
||||
def test_group_chat_builder_with_request_info_returns_self():
|
||||
"""Test that with_request_info() returns self for method chaining."""
|
||||
builder = GroupChatBuilder()
|
||||
result = builder.with_request_info()
|
||||
assert result is builder
|
||||
|
||||
# Also test with agents parameter
|
||||
builder2 = GroupChatBuilder()
|
||||
result2 = builder2.with_request_info(agents=["test"])
|
||||
assert result2 is builder2
|
||||
|
||||
@@ -687,6 +687,54 @@ async def test_tool_choice_preserved_from_agent_config():
|
||||
assert str(last_tool_choice) == "required", f"Expected 'required', got {last_tool_choice}"
|
||||
|
||||
|
||||
async def test_handoff_builder_with_request_info():
|
||||
"""Test that HandoffBuilder supports request info via with_request_info()."""
|
||||
from agent_framework import AgentInputRequest, RequestInfoEvent
|
||||
|
||||
# Create test agents
|
||||
coordinator = _RecordingAgent(name="coordinator")
|
||||
specialist = _RecordingAgent(name="specialist")
|
||||
|
||||
# Build workflow with request info enabled
|
||||
workflow = (
|
||||
HandoffBuilder(participants=[coordinator, specialist])
|
||||
.set_coordinator("coordinator")
|
||||
.with_termination_condition(lambda conv: len([m for m in conv if m.role == Role.USER]) >= 1)
|
||||
.with_request_info()
|
||||
.build()
|
||||
)
|
||||
|
||||
# Run workflow until it pauses for request info
|
||||
request_event: RequestInfoEvent | None = None
|
||||
async for event in workflow.run_stream("Hello"):
|
||||
if isinstance(event, RequestInfoEvent) and isinstance(event.data, AgentInputRequest):
|
||||
request_event = event
|
||||
|
||||
# Verify request info was emitted
|
||||
assert request_event is not None, "Request info should have been emitted"
|
||||
assert isinstance(request_event.data, AgentInputRequest)
|
||||
|
||||
# Provide response and continue
|
||||
output_events: list[WorkflowOutputEvent] = []
|
||||
async for event in workflow.send_responses_streaming({request_event.request_id: "approved"}):
|
||||
if isinstance(event, WorkflowOutputEvent):
|
||||
output_events.append(event)
|
||||
|
||||
# Verify we got output events
|
||||
assert len(output_events) > 0, "Should produce output events after response"
|
||||
|
||||
|
||||
async def test_handoff_builder_with_request_info_method_chaining():
|
||||
"""Test that with_request_info returns self for method chaining."""
|
||||
coordinator = _RecordingAgent(name="coordinator")
|
||||
|
||||
builder = HandoffBuilder(participants=[coordinator])
|
||||
result = builder.with_request_info()
|
||||
|
||||
assert result is builder, "with_request_info should return self for chaining"
|
||||
assert builder._request_info_enabled is True # type: ignore
|
||||
|
||||
|
||||
async def test_return_to_previous_state_serialization():
|
||||
"""Test that return_to_previous state is properly serialized/deserialized for checkpointing."""
|
||||
from agent_framework._workflows._handoff import _HandoffCoordinator # type: ignore[reportPrivateUsage]
|
||||
|
||||
@@ -857,3 +857,22 @@ async def test_magentic_checkpoint_runtime_overrides_buildtime() -> None:
|
||||
|
||||
assert len(runtime_checkpoints) > 0, "Runtime storage should have checkpoints"
|
||||
assert len(buildtime_checkpoints) == 0, "Build-time storage should have no checkpoints when overridden"
|
||||
|
||||
|
||||
def test_magentic_builder_does_not_have_human_input_hook():
|
||||
"""Test that MagenticBuilder does not expose with_human_input_hook (uses specialized HITL instead).
|
||||
|
||||
Magentic uses specialized human intervention mechanisms:
|
||||
- with_plan_review() for plan approval
|
||||
- with_human_input_on_stall() for stall intervention
|
||||
- Tool approval via FunctionApprovalRequestContent
|
||||
|
||||
These emit MagenticHumanInterventionRequest events with structured decision options.
|
||||
"""
|
||||
builder = MagenticBuilder()
|
||||
|
||||
# MagenticBuilder should NOT have the generic human input hook mixin
|
||||
assert not hasattr(builder, "with_human_input_hook"), (
|
||||
"MagenticBuilder should not have with_human_input_hook - "
|
||||
"use with_plan_review() or with_human_input_on_stall() instead"
|
||||
)
|
||||
|
||||
@@ -0,0 +1,168 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
"""Unit tests for request info support in high-level builders."""
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from agent_framework import (
|
||||
AgentInputRequest,
|
||||
AgentProtocol,
|
||||
AgentResponseReviewRequest,
|
||||
ChatMessage,
|
||||
RequestInfoInterceptor,
|
||||
Role,
|
||||
)
|
||||
from agent_framework._workflows._executor import Executor, handler
|
||||
from agent_framework._workflows._orchestration_request_info import resolve_request_info_filter
|
||||
from agent_framework._workflows._workflow_context import WorkflowContext
|
||||
|
||||
|
||||
class DummyExecutor(Executor):
|
||||
"""Dummy executor with a handler for testing."""
|
||||
|
||||
@handler
|
||||
async def handle(self, data: str, ctx: WorkflowContext[Any, Any]) -> None:
|
||||
pass
|
||||
|
||||
|
||||
class TestResolveRequestInfoFilter:
|
||||
"""Tests for resolve_request_info_filter function."""
|
||||
|
||||
def test_returns_none_for_none_input(self):
|
||||
"""Test that None input returns None (no filtering)."""
|
||||
result = resolve_request_info_filter(None)
|
||||
assert result is None
|
||||
|
||||
def test_returns_none_for_empty_list(self):
|
||||
"""Test that empty list returns None."""
|
||||
result = resolve_request_info_filter([])
|
||||
assert result is None
|
||||
|
||||
def test_resolves_string_names(self):
|
||||
"""Test resolving string agent names."""
|
||||
result = resolve_request_info_filter(["agent1", "agent2"])
|
||||
assert result == {"agent1", "agent2"}
|
||||
|
||||
def test_resolves_executor_ids(self):
|
||||
"""Test resolving Executor instances by ID."""
|
||||
exec1 = DummyExecutor(id="executor1")
|
||||
exec2 = DummyExecutor(id="executor2")
|
||||
|
||||
result = resolve_request_info_filter([exec1, exec2])
|
||||
assert result == {"executor1", "executor2"}
|
||||
|
||||
def test_resolves_agent_names(self):
|
||||
"""Test resolving AgentProtocol-like objects by name attribute."""
|
||||
agent1 = MagicMock(spec=AgentProtocol)
|
||||
agent1.name = "writer"
|
||||
agent2 = MagicMock(spec=AgentProtocol)
|
||||
agent2.name = "reviewer"
|
||||
|
||||
result = resolve_request_info_filter([agent1, agent2])
|
||||
assert result == {"writer", "reviewer"}
|
||||
|
||||
def test_mixed_types(self):
|
||||
"""Test resolving a mix of strings, agents, and executors."""
|
||||
agent = MagicMock(spec=AgentProtocol)
|
||||
agent.name = "writer"
|
||||
executor = DummyExecutor(id="custom_exec")
|
||||
|
||||
result = resolve_request_info_filter(["manual_name", agent, executor])
|
||||
assert result == {"manual_name", "writer", "custom_exec"}
|
||||
|
||||
def test_skips_agent_without_name(self):
|
||||
"""Test that agents without names are skipped."""
|
||||
agent_with_name = MagicMock(spec=AgentProtocol)
|
||||
agent_with_name.name = "valid"
|
||||
agent_without_name = MagicMock(spec=AgentProtocol)
|
||||
agent_without_name.name = None
|
||||
|
||||
result = resolve_request_info_filter([agent_with_name, agent_without_name])
|
||||
assert result == {"valid"}
|
||||
|
||||
|
||||
class TestAgentInputRequest:
|
||||
"""Tests for AgentInputRequest dataclass (formerly AgentResponseReviewRequest)."""
|
||||
|
||||
def test_create_request(self):
|
||||
"""Test creating an AgentInputRequest with all fields."""
|
||||
conversation = [ChatMessage(role=Role.USER, text="Hello")]
|
||||
request = AgentInputRequest(
|
||||
target_agent_id="test_agent",
|
||||
conversation=conversation,
|
||||
instruction="Review this",
|
||||
metadata={"key": "value"},
|
||||
)
|
||||
|
||||
assert request.target_agent_id == "test_agent"
|
||||
assert request.conversation == conversation
|
||||
assert request.instruction == "Review this"
|
||||
assert request.metadata == {"key": "value"}
|
||||
|
||||
def test_create_request_defaults(self):
|
||||
"""Test creating an AgentInputRequest with default values."""
|
||||
request = AgentInputRequest(target_agent_id="test_agent")
|
||||
|
||||
assert request.target_agent_id == "test_agent"
|
||||
assert request.conversation == []
|
||||
assert request.instruction is None
|
||||
assert request.metadata == {}
|
||||
|
||||
def test_backward_compatibility_alias(self):
|
||||
"""Test that AgentResponseReviewRequest is an alias for AgentInputRequest."""
|
||||
assert AgentResponseReviewRequest is AgentInputRequest
|
||||
|
||||
|
||||
class TestRequestInfoInterceptor:
|
||||
"""Tests for RequestInfoInterceptor executor."""
|
||||
|
||||
def test_interceptor_creation_generates_unique_id(self):
|
||||
"""Test creating a RequestInfoInterceptor generates unique IDs."""
|
||||
interceptor1 = RequestInfoInterceptor()
|
||||
interceptor2 = RequestInfoInterceptor()
|
||||
assert interceptor1.id.startswith("request_info_interceptor-")
|
||||
assert interceptor2.id.startswith("request_info_interceptor-")
|
||||
assert interceptor1.id != interceptor2.id
|
||||
|
||||
def test_interceptor_with_custom_id(self):
|
||||
"""Test creating a RequestInfoInterceptor with custom ID."""
|
||||
interceptor = RequestInfoInterceptor(executor_id="custom_review")
|
||||
assert interceptor.id == "custom_review"
|
||||
|
||||
def test_interceptor_with_agent_filter(self):
|
||||
"""Test creating a RequestInfoInterceptor with agent filter."""
|
||||
agent_filter = {"agent1", "agent2"}
|
||||
interceptor = RequestInfoInterceptor(
|
||||
executor_id="filtered_review",
|
||||
agent_filter=agent_filter,
|
||||
)
|
||||
assert interceptor.id == "filtered_review"
|
||||
assert interceptor._agent_filter == agent_filter
|
||||
|
||||
def test_should_pause_for_agent_no_filter(self):
|
||||
"""Test that interceptor pauses for all agents when no filter is set."""
|
||||
interceptor = RequestInfoInterceptor()
|
||||
assert interceptor._should_pause_for_agent("any_agent") is True
|
||||
assert interceptor._should_pause_for_agent("another_agent") is True
|
||||
assert interceptor._should_pause_for_agent(None) is True
|
||||
|
||||
def test_should_pause_for_agent_with_filter(self):
|
||||
"""Test that interceptor only pauses for agents in the filter."""
|
||||
agent_filter = {"writer", "reviewer"}
|
||||
interceptor = RequestInfoInterceptor(agent_filter=agent_filter)
|
||||
|
||||
assert interceptor._should_pause_for_agent("writer") is True
|
||||
assert interceptor._should_pause_for_agent("reviewer") is True
|
||||
assert interceptor._should_pause_for_agent("drafter") is False
|
||||
assert interceptor._should_pause_for_agent(None) is False
|
||||
|
||||
def test_should_pause_for_agent_with_prefixed_id(self):
|
||||
"""Test that filter matches agent names in prefixed executor IDs."""
|
||||
agent_filter = {"writer"}
|
||||
interceptor = RequestInfoInterceptor(agent_filter=agent_filter)
|
||||
|
||||
# Should match the name portion after the colon
|
||||
assert interceptor._should_pause_for_agent("groupchat_agent:writer") is True
|
||||
assert interceptor._should_pause_for_agent("request_info:writer") is True
|
||||
assert interceptor._should_pause_for_agent("groupchat_agent:editor") is False
|
||||
@@ -111,7 +111,8 @@ def test_add_agent_with_custom_parameters():
|
||||
builder = WorkflowBuilder()
|
||||
|
||||
# Add agent with custom parameters
|
||||
result = builder.add_agent(agent, output_response=True, id="my_custom_id")
|
||||
with pytest.deprecated_call():
|
||||
result = builder.add_agent(agent, output_response=True, id="my_custom_id")
|
||||
|
||||
# Verify that add_agent returns the builder for chaining
|
||||
assert result is builder
|
||||
@@ -133,7 +134,8 @@ def test_add_agent_reuses_same_wrapper():
|
||||
builder = WorkflowBuilder()
|
||||
|
||||
# Add agent with specific parameters
|
||||
builder.add_agent(agent, output_response=True, id="agent_exec")
|
||||
with pytest.deprecated_call():
|
||||
builder.add_agent(agent, output_response=True, id="agent_exec")
|
||||
|
||||
# Use the same agent instance in add_edge - should reuse the same wrapper
|
||||
builder.set_start_executor(agent)
|
||||
@@ -158,8 +160,9 @@ def test_add_agent_then_use_in_edges():
|
||||
builder = WorkflowBuilder()
|
||||
|
||||
# Add agents with specific settings
|
||||
builder.add_agent(agent1, output_response=False, id="exec1")
|
||||
builder.add_agent(agent2, output_response=True, id="exec2")
|
||||
with pytest.deprecated_call():
|
||||
builder.add_agent(agent1, output_response=False, id="exec1")
|
||||
builder.add_agent(agent2, output_response=True, id="exec2")
|
||||
|
||||
# Use the same agent instances to create edges
|
||||
workflow = builder.set_start_executor(agent1).add_edge(agent1, agent2).build()
|
||||
@@ -183,7 +186,8 @@ def test_add_agent_without_explicit_id_uses_agent_name():
|
||||
agent = DummyAgent(id="agent_x", name="named_agent")
|
||||
builder = WorkflowBuilder()
|
||||
|
||||
result = builder.add_agent(agent)
|
||||
with pytest.deprecated_call():
|
||||
result = builder.add_agent(agent)
|
||||
|
||||
# Verify that add_agent returns the builder for chaining
|
||||
assert result is builder
|
||||
@@ -203,10 +207,11 @@ def test_add_agent_duplicate_id_raises_error():
|
||||
builder = WorkflowBuilder()
|
||||
|
||||
# Add first agent
|
||||
builder.add_agent(agent1)
|
||||
with pytest.deprecated_call():
|
||||
builder.add_agent(agent1)
|
||||
|
||||
# Adding second agent with same name should raise ValueError
|
||||
with pytest.raises(ValueError, match="Duplicate executor ID"):
|
||||
with pytest.deprecated_call(), pytest.raises(ValueError, match="Duplicate executor ID"):
|
||||
builder.add_agent(agent2)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user