mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
[BREAKING] Python: Remove workflow register factory methods. Update tests and samples (#3781)
* Remove workflow register factory methods. Update tests and samples * Address Copilot feedback
This commit is contained in:
Evan Mattson
committed by
GitHub
Unverified
GitHub
parent
f407f726a7
commit
a4c9e43afb
@@ -670,16 +670,20 @@ class TestWorkflowAgent:
|
||||
return ResponseStream(_iter(), finalizer=AgentResponse.from_updates)
|
||||
|
||||
@executor
|
||||
async def start_executor(messages: list[ChatMessage], ctx: WorkflowContext[AgentExecutorRequest, str]) -> None:
|
||||
async def start_exec(messages: list[ChatMessage], ctx: WorkflowContext[AgentExecutorRequest, str]) -> None:
|
||||
await ctx.yield_output("Start output")
|
||||
await ctx.send_message(AgentExecutorRequest(messages=messages, should_respond=True))
|
||||
|
||||
# Build workflow: start -> agent1 (no output) -> agent2 (output_response=True)
|
||||
builder = WorkflowBuilder(start_executor="start", output_executors=["start", "agent2"])
|
||||
builder.register_executor(lambda: start_executor, "start")
|
||||
builder.register_agent(lambda: MockAgent("agent1", "Agent1 output - should NOT appear"), "agent1")
|
||||
builder.register_agent(lambda: MockAgent("agent2", "Agent2 output - SHOULD appear"), "agent2")
|
||||
workflow = builder.add_edge("start", "agent1").add_edge("agent1", "agent2").build()
|
||||
agent1 = MockAgent("agent1", "Agent1 output - should NOT appear")
|
||||
agent2 = MockAgent("agent2", "Agent2 output - SHOULD appear")
|
||||
|
||||
# Build workflow: start -> agent1 (no output) -> agent2 (output visible)
|
||||
workflow = (
|
||||
WorkflowBuilder(start_executor=start_exec, output_executors=[start_exec, agent2])
|
||||
.add_edge(start_exec, agent1)
|
||||
.add_edge(agent1, agent2)
|
||||
.build()
|
||||
)
|
||||
|
||||
agent = WorkflowAgent(workflow=workflow, name="Test Agent")
|
||||
result = await agent.run("Test input")
|
||||
@@ -754,17 +758,13 @@ class TestWorkflowAgent:
|
||||
return ResponseStream(_iter(), finalizer=AgentResponse.from_updates)
|
||||
|
||||
@executor
|
||||
async def start_executor(messages: list[ChatMessage], ctx: WorkflowContext[AgentExecutorRequest]) -> None:
|
||||
async def start_exec(messages: list[ChatMessage], ctx: WorkflowContext[AgentExecutorRequest]) -> None:
|
||||
await ctx.send_message(AgentExecutorRequest(messages=messages, should_respond=True))
|
||||
|
||||
mock_agent = MockAgent("agent", "Unique response text")
|
||||
|
||||
# Build workflow with single agent
|
||||
workflow = (
|
||||
WorkflowBuilder(start_executor="start")
|
||||
.register_executor(lambda: start_executor, "start")
|
||||
.register_agent(lambda: MockAgent("agent", "Unique response text"), "agent")
|
||||
.add_edge("start", "agent")
|
||||
.build()
|
||||
)
|
||||
workflow = WorkflowBuilder(start_executor=start_exec).add_edge(start_exec, mock_agent).build()
|
||||
|
||||
agent = WorkflowAgent(workflow=workflow, name="Test Agent")
|
||||
result = await agent.run("Test input")
|
||||
|
||||
@@ -134,322 +134,65 @@ def test_add_agent_duplicate_id_raises_error():
|
||||
builder.add_edge(agent1, agent2).build()
|
||||
|
||||
|
||||
# Tests for new executor registration patterns
|
||||
def test_fan_out_edges_with_direct_instances():
|
||||
"""Test fan-out edges with direct executor instances."""
|
||||
source = MockExecutor(id="Source")
|
||||
target1 = MockExecutor(id="Target1")
|
||||
target2 = MockExecutor(id="Target2")
|
||||
|
||||
workflow = WorkflowBuilder(start_executor=source).add_fan_out_edges(source, [target1, target2]).build()
|
||||
|
||||
def test_register_executor_basic():
|
||||
"""Test basic executor registration with lazy initialization."""
|
||||
builder = WorkflowBuilder(start_executor="TestExecutor")
|
||||
|
||||
# Register an executor factory - ID must match the registered name
|
||||
result = builder.register_executor(lambda: MockExecutor(id="TestExecutor"), name="TestExecutor")
|
||||
|
||||
# Verify that register returns the builder for chaining
|
||||
assert result is builder
|
||||
|
||||
# Build workflow and verify executor is instantiated
|
||||
workflow = builder.build()
|
||||
assert "TestExecutor" in workflow.executors
|
||||
assert isinstance(workflow.executors["TestExecutor"], MockExecutor)
|
||||
|
||||
|
||||
def test_register_multiple_executors():
|
||||
"""Test registering multiple executors and connecting them with edges."""
|
||||
builder = WorkflowBuilder(start_executor="ExecutorA")
|
||||
|
||||
# Register multiple executors - IDs must match registered names
|
||||
builder.register_executor(lambda: MockExecutor(id="ExecutorA"), name="ExecutorA")
|
||||
builder.register_executor(lambda: MockExecutor(id="ExecutorB"), name="ExecutorB")
|
||||
builder.register_executor(lambda: MockExecutor(id="ExecutorC"), name="ExecutorC")
|
||||
|
||||
# Build workflow with edges using registered names
|
||||
workflow = builder.add_edge("ExecutorA", "ExecutorB").add_edge("ExecutorB", "ExecutorC").build()
|
||||
|
||||
# Verify all executors are present
|
||||
assert "ExecutorA" in workflow.executors
|
||||
assert "ExecutorB" in workflow.executors
|
||||
assert "ExecutorC" in workflow.executors
|
||||
assert workflow.start_executor_id == "ExecutorA"
|
||||
|
||||
|
||||
def test_register_with_multiple_names():
|
||||
"""Test registering the same factory function under multiple names."""
|
||||
builder = WorkflowBuilder(start_executor="ExecutorA")
|
||||
|
||||
# Register same executor factory under multiple names
|
||||
# Note: Each call creates a new instance, so IDs won't conflict
|
||||
counter = {"val": 0}
|
||||
|
||||
def make_executor():
|
||||
counter["val"] += 1
|
||||
return MockExecutor(id="ExecutorA" if counter["val"] == 1 else "ExecutorB")
|
||||
|
||||
builder.register_executor(make_executor, name=["ExecutorA", "ExecutorB"])
|
||||
|
||||
# Set up workflow
|
||||
workflow = builder.add_edge("ExecutorA", "ExecutorB").build()
|
||||
|
||||
# Verify both executors are present
|
||||
assert "ExecutorA" in workflow.executors
|
||||
assert "ExecutorB" in workflow.executors
|
||||
assert workflow.start_executor_id == "ExecutorA"
|
||||
|
||||
|
||||
def test_register_duplicate_name_raises_error():
|
||||
"""Test that registering duplicate names raises an error."""
|
||||
builder = WorkflowBuilder(start_executor="MyExecutor")
|
||||
|
||||
# Register first executor
|
||||
builder.register_executor(lambda: MockExecutor(id="executor_1"), name="MyExecutor")
|
||||
|
||||
# Registering second executor with same name should raise ValueError
|
||||
with pytest.raises(ValueError, match="already registered"):
|
||||
builder.register_executor(lambda: MockExecutor(id="executor_2"), name="MyExecutor")
|
||||
|
||||
|
||||
def test_register_duplicate_id_raises_error():
|
||||
"""Test that registering duplicate id raises an error."""
|
||||
builder = WorkflowBuilder(start_executor="MyExecutor1")
|
||||
|
||||
# Register first executor
|
||||
builder.register_executor(lambda: MockExecutor(id="executor"), name="MyExecutor1")
|
||||
builder.register_executor(lambda: MockExecutor(id="executor"), name="MyExecutor2")
|
||||
|
||||
# Registering second executor with same ID should raise ValueError
|
||||
with pytest.raises(ValueError, match="Executor with ID 'executor' has already been registered."):
|
||||
builder.build()
|
||||
|
||||
|
||||
def test_register_agent_basic():
|
||||
"""Test basic agent registration with lazy initialization."""
|
||||
builder = WorkflowBuilder(start_executor="TestAgent")
|
||||
|
||||
# Register an agent factory
|
||||
result = builder.register_agent(lambda: DummyAgent(id="agent_test", name="test_agent"), name="TestAgent")
|
||||
|
||||
# Verify that register_agent returns the builder for chaining
|
||||
assert result is builder
|
||||
|
||||
# Build workflow and verify agent is wrapped in AgentExecutor
|
||||
workflow = builder.build()
|
||||
assert "test_agent" in workflow.executors
|
||||
assert isinstance(workflow.executors["test_agent"], AgentExecutor)
|
||||
|
||||
|
||||
def test_register_agent_with_thread():
|
||||
"""Test registering an agent with a custom thread."""
|
||||
builder = WorkflowBuilder(start_executor="ThreadedAgent")
|
||||
custom_thread = AgentThread()
|
||||
|
||||
# Register agent with custom thread
|
||||
builder.register_agent(
|
||||
lambda: DummyAgent(id="agent_with_thread", name="threaded_agent"),
|
||||
name="ThreadedAgent",
|
||||
agent_thread=custom_thread,
|
||||
)
|
||||
|
||||
# Build workflow and verify agent executor configuration
|
||||
workflow = builder.build()
|
||||
executor = workflow.executors["threaded_agent"]
|
||||
|
||||
assert isinstance(executor, AgentExecutor)
|
||||
assert executor.id == "threaded_agent"
|
||||
assert executor._agent_thread is custom_thread # type: ignore
|
||||
|
||||
|
||||
def test_register_agent_duplicate_name_raises_error():
|
||||
"""Test that registering agents with duplicate names raises an error."""
|
||||
builder = WorkflowBuilder(start_executor="MyAgent")
|
||||
|
||||
# Register first agent
|
||||
builder.register_agent(lambda: DummyAgent(id="agent1", name="first"), name="MyAgent")
|
||||
|
||||
# Registering second agent with same name should raise ValueError
|
||||
with pytest.raises(ValueError, match="already registered"):
|
||||
builder.register_agent(lambda: DummyAgent(id="agent2", name="second"), name="MyAgent")
|
||||
|
||||
|
||||
def test_register_and_add_edge_with_strings():
|
||||
"""Test that registered executors can be connected using string names."""
|
||||
builder = WorkflowBuilder(start_executor="Source")
|
||||
|
||||
# Register executors
|
||||
builder.register_executor(lambda: MockExecutor(id="source"), name="Source")
|
||||
builder.register_executor(lambda: MockExecutor(id="target"), name="Target")
|
||||
|
||||
# Add edge using string names
|
||||
workflow = builder.add_edge("Source", "Target").build()
|
||||
|
||||
# Verify edge is created correctly
|
||||
assert workflow.start_executor_id == "source"
|
||||
assert "source" in workflow.executors
|
||||
assert "target" in workflow.executors
|
||||
|
||||
|
||||
def test_register_agent_and_add_edge_with_strings():
|
||||
"""Test that registered agents can be connected using string names."""
|
||||
builder = WorkflowBuilder(start_executor="Writer")
|
||||
|
||||
# Register agents
|
||||
builder.register_agent(lambda: DummyAgent(id="writer_id", name="writer"), name="Writer")
|
||||
builder.register_agent(lambda: DummyAgent(id="reviewer_id", name="reviewer"), name="Reviewer")
|
||||
|
||||
# Add edge using string names
|
||||
workflow = builder.add_edge("Writer", "Reviewer").build()
|
||||
|
||||
# Verify edge is created correctly
|
||||
assert workflow.start_executor_id == "writer"
|
||||
assert "writer" in workflow.executors
|
||||
assert "reviewer" in workflow.executors
|
||||
assert all(isinstance(e, AgentExecutor) for e in workflow.executors.values())
|
||||
|
||||
|
||||
def test_register_with_fan_out_edges():
|
||||
"""Test using registered names with fan-out edge groups."""
|
||||
builder = WorkflowBuilder(start_executor="Source")
|
||||
|
||||
# Register executors - IDs must match registered names
|
||||
builder.register_executor(lambda: MockExecutor(id="Source"), name="Source")
|
||||
builder.register_executor(lambda: MockExecutor(id="Target1"), name="Target1")
|
||||
builder.register_executor(lambda: MockExecutor(id="Target2"), name="Target2")
|
||||
|
||||
# Add fan-out edges using registered names
|
||||
workflow = builder.add_fan_out_edges("Source", ["Target1", "Target2"]).build()
|
||||
|
||||
# Verify all executors are present
|
||||
assert "Source" in workflow.executors
|
||||
assert "Target1" in workflow.executors
|
||||
assert "Target2" in workflow.executors
|
||||
|
||||
|
||||
def test_register_with_fan_in_edges():
|
||||
"""Test using registered names with fan-in edge groups."""
|
||||
builder = WorkflowBuilder(start_executor="Source1")
|
||||
def test_fan_in_edges_with_direct_instances():
|
||||
"""Test fan-in edges with direct executor instances."""
|
||||
source1 = MockExecutor(id="Source1")
|
||||
source2 = MockExecutor(id="Source2")
|
||||
aggregator = MockAggregator(id="Aggregator")
|
||||
|
||||
# Register executors - IDs must match registered names
|
||||
builder.register_executor(lambda: MockExecutor(id="Source1"), name="Source1")
|
||||
builder.register_executor(lambda: MockExecutor(id="Source2"), name="Source2")
|
||||
builder.register_executor(lambda: MockAggregator(id="Aggregator"), name="Aggregator")
|
||||
workflow = (
|
||||
WorkflowBuilder(start_executor=source1)
|
||||
.add_edge(source1, source2)
|
||||
.add_fan_in_edges([source1, source2], aggregator)
|
||||
.build()
|
||||
)
|
||||
|
||||
# Add fan-in edges using registered names
|
||||
# Both Source1 and Source2 need to be reachable, so connect Source1 to Source2
|
||||
workflow = builder.add_edge("Source1", "Source2").add_fan_in_edges(["Source1", "Source2"], "Aggregator").build()
|
||||
|
||||
# Verify all executors are present
|
||||
assert "Source1" in workflow.executors
|
||||
assert "Source2" in workflow.executors
|
||||
assert "Aggregator" in workflow.executors
|
||||
|
||||
|
||||
def test_register_with_chain():
|
||||
"""Test using registered names with add_chain."""
|
||||
builder = WorkflowBuilder(start_executor="Step1")
|
||||
def test_chain_with_direct_instances():
|
||||
"""Test add_chain with direct executor instances."""
|
||||
step1 = MockExecutor(id="Step1")
|
||||
step2 = MockExecutor(id="Step2")
|
||||
step3 = MockExecutor(id="Step3")
|
||||
|
||||
# Register executors - IDs must match registered names
|
||||
builder.register_executor(lambda: MockExecutor(id="Step1"), name="Step1")
|
||||
builder.register_executor(lambda: MockExecutor(id="Step2"), name="Step2")
|
||||
builder.register_executor(lambda: MockExecutor(id="Step3"), name="Step3")
|
||||
workflow = WorkflowBuilder(start_executor=step1).add_chain([step1, step2, step3]).build()
|
||||
|
||||
# Add chain using registered names
|
||||
workflow = builder.add_chain(["Step1", "Step2", "Step3"]).build()
|
||||
|
||||
# Verify all executors are present
|
||||
assert "Step1" in workflow.executors
|
||||
assert "Step2" in workflow.executors
|
||||
assert "Step3" in workflow.executors
|
||||
assert workflow.start_executor_id == "Step1"
|
||||
|
||||
|
||||
def test_register_factory_called_only_once():
|
||||
"""Test that registered factory functions are called only during build."""
|
||||
call_count = 0
|
||||
|
||||
def factory():
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
return MockExecutor(id="Test")
|
||||
|
||||
builder = WorkflowBuilder(start_executor="Test")
|
||||
builder.register_executor(factory, name="Test")
|
||||
|
||||
# Factory should not be called yet
|
||||
assert call_count == 0
|
||||
|
||||
# Factory should still not be called
|
||||
assert call_count == 0
|
||||
|
||||
# Build workflow
|
||||
workflow = builder.build()
|
||||
|
||||
# Factory should now be called exactly once
|
||||
assert call_count == 1
|
||||
assert "Test" in workflow.executors
|
||||
|
||||
|
||||
def test_mixing_eager_and_lazy_initialization_error():
|
||||
"""Test that mixing eager executor instances with lazy string names raises appropriate error."""
|
||||
builder = WorkflowBuilder(start_executor="Lazy")
|
||||
|
||||
# Create an eager executor instance
|
||||
eager_executor = MockExecutor(id="eager")
|
||||
|
||||
# Register a lazy executor
|
||||
builder.register_executor(lambda: MockExecutor(id="Lazy"), name="Lazy")
|
||||
|
||||
# Mixing eager and lazy should raise an error during add_edge
|
||||
with pytest.raises(
|
||||
ValueError,
|
||||
match=(
|
||||
r"Both source and target must be either registered factory names \(str\) "
|
||||
r"or Executor/SupportsAgentRun instances\."
|
||||
),
|
||||
):
|
||||
builder.add_edge(eager_executor, "Lazy")
|
||||
|
||||
|
||||
def test_register_with_condition():
|
||||
"""Test adding edges with conditions using registered names."""
|
||||
builder = WorkflowBuilder(start_executor="Source")
|
||||
def test_add_edge_with_condition():
|
||||
"""Test adding edges with conditions using direct executor instances."""
|
||||
source = MockExecutor(id="Source")
|
||||
target = MockExecutor(id="Target")
|
||||
|
||||
def condition_func(msg: MockMessage) -> bool:
|
||||
return msg.data > 0
|
||||
|
||||
# Register executors - IDs must match registered names
|
||||
builder.register_executor(lambda: MockExecutor(id="Source"), name="Source")
|
||||
builder.register_executor(lambda: MockExecutor(id="Target"), name="Target")
|
||||
workflow = WorkflowBuilder(start_executor=source).add_edge(source, target, condition=condition_func).build()
|
||||
|
||||
# Add edge with condition
|
||||
workflow = builder.add_edge("Source", "Target", condition=condition_func).build()
|
||||
|
||||
# Verify workflow is built correctly
|
||||
assert "Source" in workflow.executors
|
||||
assert "Target" in workflow.executors
|
||||
|
||||
|
||||
def test_register_agent_creates_unique_instances():
|
||||
"""Test that registered agent factories create new instances on each build."""
|
||||
instance_ids: list[int] = []
|
||||
|
||||
def agent_factory() -> DummyAgent:
|
||||
agent = DummyAgent(id=f"agent_{len(instance_ids)}", name="test")
|
||||
instance_ids.append(id(agent))
|
||||
return agent
|
||||
|
||||
# Build first workflow
|
||||
builder1 = WorkflowBuilder(start_executor="Agent")
|
||||
builder1.register_agent(agent_factory, name="Agent")
|
||||
_ = builder1.build()
|
||||
|
||||
# Build second workflow
|
||||
builder2 = WorkflowBuilder(start_executor="Agent")
|
||||
builder2.register_agent(agent_factory, name="Agent")
|
||||
_ = builder2.build()
|
||||
|
||||
# Verify that two different agent instances were created
|
||||
assert len(instance_ids) == 2
|
||||
assert instance_ids[0] != instance_ids[1]
|
||||
|
||||
|
||||
# region with_output_from tests
|
||||
|
||||
|
||||
@@ -488,14 +231,17 @@ def test_with_output_from_with_agent_instances():
|
||||
assert workflow._output_executors == ["reviewer"] # type: ignore
|
||||
|
||||
|
||||
def test_with_output_from_with_registered_names():
|
||||
"""Test with_output_from with registered factory names (strings)."""
|
||||
builder = WorkflowBuilder(start_executor="ExecutorAFactory", output_executors=["ExecutorBFactory"])
|
||||
builder.register_executor(lambda: MockExecutor(id="ExecutorA"), name="ExecutorAFactory")
|
||||
builder.register_executor(lambda: MockExecutor(id="ExecutorB"), name="ExecutorBFactory")
|
||||
workflow = builder.add_edge("ExecutorAFactory", "ExecutorBFactory").build()
|
||||
def test_with_output_from_with_executor_instances_by_id():
|
||||
"""Test with_output_from with direct executor instances resolves to executor IDs."""
|
||||
executor_a = MockExecutor(id="ExecutorA")
|
||||
executor_b = MockExecutor(id="ExecutorB")
|
||||
|
||||
workflow = (
|
||||
WorkflowBuilder(start_executor=executor_a, output_executors=[executor_b])
|
||||
.add_edge(executor_a, executor_b)
|
||||
.build()
|
||||
)
|
||||
|
||||
# Verify that the workflow was built with the correct output executors
|
||||
assert workflow._output_executors == ["ExecutorB"] # type: ignore
|
||||
|
||||
|
||||
@@ -531,14 +277,17 @@ def test_with_output_from_can_be_set_to_different_value():
|
||||
assert workflow._output_executors == ["executor_b"] # type: ignore
|
||||
|
||||
|
||||
def test_with_output_from_with_registered_agents():
|
||||
"""Test with_output_from with registered agent factory names."""
|
||||
builder = WorkflowBuilder(start_executor="WriterAgent", output_executors=["ReviewerAgent"])
|
||||
builder.register_agent(lambda: DummyAgent(id="agent1", name="writer"), name="WriterAgent")
|
||||
builder.register_agent(lambda: DummyAgent(id="agent2", name="reviewer"), name="ReviewerAgent")
|
||||
workflow = builder.add_edge("WriterAgent", "ReviewerAgent").build()
|
||||
def test_with_output_from_with_agent_instances_resolves_name():
|
||||
"""Test with_output_from with agent instances resolves to agent names."""
|
||||
agent_writer = DummyAgent(id="agent1", name="writer")
|
||||
agent_reviewer = DummyAgent(id="agent2", name="reviewer")
|
||||
|
||||
workflow = (
|
||||
WorkflowBuilder(start_executor=agent_writer, output_executors=[agent_reviewer])
|
||||
.add_edge(agent_writer, agent_reviewer)
|
||||
.build()
|
||||
)
|
||||
|
||||
# Verify that the workflow was built with the agent's resolved name
|
||||
assert workflow._output_executors == ["reviewer"] # type: ignore
|
||||
|
||||
|
||||
|
||||
@@ -474,8 +474,9 @@ async def test_message_trace_context_serialization(span_exporter: InMemorySpanEx
|
||||
async def test_workflow_build_error_tracing(span_exporter: InMemorySpanExporter) -> None:
|
||||
"""Test that build errors are properly recorded in build spans."""
|
||||
|
||||
# Test validation error by referencing a non-existent start executor
|
||||
builder = WorkflowBuilder(start_executor="NonExistent")
|
||||
# Create a valid builder, then clear the start executor to trigger a build-time ValueError
|
||||
builder = WorkflowBuilder(start_executor=MockExecutor(id="mock"))
|
||||
builder._start_executor = None # type: ignore[assignment]
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
builder.build()
|
||||
|
||||
Reference in New Issue
Block a user