Python: [DRAFT] workflow composibility design

[moonbox3]

Motivation and Context

Putting some thoughts and a POC together around workflow compatibility - being able to connect together two workflows, edges + high-level orchestration APIs, etc.

Description

• Addresses Workflow add additional steps after build #1795

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.

[markwallace-microsoft]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/core/agent_framework/_workflows
_concurrent.py	143	28	80%	50, 59–60, 68–69, 88–89, 94, 99, 124, 129, 134–135, 141, 163, 173, 180, 253–257, 259, 287, 291, 301–302, 332
_group_chat.py	414	64	84%	169–170, 301–311, 360, 374, 381, 385–387, 453, 462, 608, 619, 623, 692, 709, 714–716, 723, 889, 894, 1011, 1078–1079, 1081–1082, 1084, 1086, 1088–1089, 1157–1158, 1231, 1326, 1352, 1354, 1358–1361, 1363, 1366–1367, 1382, 1415–1418, 1428, 1439, 1448–1450
_handoff.py	482	138	71%	55, 68–70, 77–78, 80, 82, 156, 164–169, 172–173, 187, 196, 215–218, 227–229, 240, 243, 253–264, 266, 272, 278, 308, 353, 364–366, 422, 431–436, 438–439, 450, 467, 485, 509, 537, 549–551, 557, 581–583, 586–589, 591–593, 827, 833, 837, 843, 847, 859, 907, 910, 998, 1003, 1013, 1019–1022, 1030–1031, 1035–1037, 1039–1049, 1051–1052, 1054, 1056, 1071–1072, 1075–1076, 1079, 1099–1105, 1107, 1113, 1147–1148, 1201–1202, 1312–1313, 1394, 1402, 1469, 1480, 1484, 1489–1490
_magentic.py	903	258	71%	49, 54, 76–85, 90, 94–105, 329, 334, 351, 353, 368, 376–385, 463, 467, 481, 487, 502, 582, 595, 612, 621–622, 624–626, 628, 639, 781–784, 787–791, 793–795, 802, 841, 888, 924–926, 928, 936–939, 943–946, 1010, 1068–1069, 1086, 1088–1089, 1097, 1133, 1142–1144, 1164, 1217, 1237, 1240, 1269, 1272, 1280–1284, 1290, 1318–1320, 1322, 1324, 1332–1335, 1337, 1341–1342, 1345–1348, 1350–1351, 1357–1359, 1362–1363, 1368–1369, 1377, 1385, 1400, 1412, 1424–1427, 1456–1457, 1462–1464, 1495, 1521, 1536, 1552, 1569, 1636, 1643, 1646, 1648–1649, 1652–1653, 1657, 1660, 1681, 1716–1717, 1719, 1723–1725, 1740, 1751, 1761, 1805, 1810–1811, 2137–2138, 2142, 2157, 2166–2167, 2172, 2175, 2229, 2240, 2251–2253, 2266–2267, 2272, 2283–2285, 2296–2298, 2310–2317, 2319–2320, 2328, 2336–2337, 2339–2341, 2343–2346, 2350–2358, 2362–2363, 2366–2370, 2372–2373, 2375–2377, 2379–2382, 2384–2386, 2388–2389, 2391–2393, 2408–2411, 2422–2425, 2437–2440, 2444
_sequential.py	84	9	89%	71, 80–81, 91, 143, 149, 162–163, 182
_workflow.py	257	18	93%	98, 271–273, 275–276, 294, 318, 320, 413, 662, 696, 701, 704, 723–725, 790
_workflow_builder.py	338	32	90%	96, 138, 182, 185–189, 201–202, 224–225, 242, 324, 407, 409, 416–418, 421–422, 431, 434, 441, 446, 520, 853–857, 859
TOTAL	15697	2387	84%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
2224	130 💤	0 ❌	0 🔥	55.339s ⏱️

[eavanvalkenburg]

some very interesting things in here, I agree that option 2 makes the most sense, but I think the port idea might make some other scenario's easier and should be considered.

[eavanvalkenburg]

is this even needed? couldn't the next step just accept a unbuild workflowbuilder of any type? and then call as_connection itself, the function doesn't have params, so should be easy. Alternatively the prefix could be set in as_connection and then you can just connect.

[eavanvalkenburg]

or even have prefix default to the name of the workflow, making it a bit easier to keep track of it.

[eavanvalkenburg]

so while I understand the connect verbiage, why not keep it a bit simpler and just add_workflow, which allows either a WorkflowConnection or a WorkflowBuilder (and that probably means also making sure that the workflowbuilders are all based on a single baseclass with the as_connection logic.

[moonbox3]

I've now got a shared mixin-style surface (as_connection, add_workflow as the single verb) rather than forcing all high-level builders into a single inheritance base because:

• High-level builders already have distinct ctor shapes and internal state; pushing them under one base would introduce a breaking constructor contract or a leaky abstract layer we’d need to retrofit across Concurrent/Sequential/GroupChat/Magentic/Handoff.
• The behavior we need is tiny (expose .as_connection(prefix=...) and route through WorkflowBuilder), which we can provide uniformly via a mixin or thin delegations without reshaping the class hierarchy.
• add_workflow accepts WorkflowBuilder, Workflow, or WorkflowConnection; so callers get the simple API, while we keep existing builder implementations intact.
• This keeps public surface additive and avoids a sweeping inheritance refactor now; if we later find more shared behavior, we can introduce a common base with backward-compatible constructors.

[eavanvalkenburg]

This would be a interesting feature as well, making it very explicit which input and outputs there are, also allowing hosted workflows to be instantiated with different endpoints for the different input ports.

[TaoChenOSU]

I think we need to make sure that the responsibilities of our abstractions are clearly defined:

• A Workflow instance: built from a workflow builder and immutable
• A WorkflowBuilder instance: created by the users and mutable

This makes the composition story more streamlined and with less ambiguity. This also potentially aligns better with the declarative story: the declarative language is essentially a builder language.

[moonbox3]

Adding a terminology section.

[TaoChenOSU]

Have we not achieved this via subworkflows?

[TaoChenOSU]

Or you mean the workflow must not be wrapped in an executor?

[TaoChenOSU]

This is just syntactic sugar for what we already have:

workflow_executor = WorkflowExecutor(workflow, id="abc", ...)

[moonbox3]

Agreed - this is syntactic sugar. The doc is exploring different approaches; we'll select what makes sense based on the full discussion.