Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] The WorkflowStepFactory is not actually a factory #241

Closed
owaiskazi19 opened this issue Dec 2, 2023 · 1 comment · Fixed by #243
Closed

[BUG] The WorkflowStepFactory is not actually a factory #241

owaiskazi19 opened this issue Dec 2, 2023 · 1 comment · Fixed by #243
Assignees
Labels
bug Something isn't working

Comments

@owaiskazi19
Copy link
Member

What is the bug?

{
                    "id": "workflow_step_3",
                    "type": "register_agent",
                    "previous_node_inputs": {
                        "workflow_step_1": "tools",
                        "workflow_step_2": "tools"
                    },
                    "user_inputs": {
                        "name": "TOOLS-TOOLS-TOOLS",
                        "type": "cot",
                        "description": "this is a test agent",
                        "parameters": {"hello": "world"},
                        "llm.model_id": "ldzS04kBxRPZ5cnWrqpd",
                        "llm.parameters": {

For a case like the above, where a workflow step depends on 2 different workflow steps. It requires the outputs of

public CompletableFuture<WorkflowData> execute(
        String currentNodeId,
        WorkflowData currentNodeInputs,
        Map<String, WorkflowData> outputs,
        Map<String, String> previousNodeInputs
    )

to consist of the responses received from workflow_step_1 and workflow_step_2. Currently, the outputs just have the response of only one previous nodes.

This issue can also hamper when a WorkflowSteps requires multiple modelIds from different DeployModelSteps.

How can one reproduce the bug?

Use the below sample template to create an agent which requires multiple tools

{
    "name": "tool-register-agent",
    "description": "test case",
    "use_case": "REGISTER_AGENT",
    "version": {
        "template": "1.0.0",
        "compatibility": [
            "2.12.0",
            "3.0.0"
        ]
    },
    "workflows": {
        "provision": {
            "nodes": [
                {
                    "id": "workflow_step_1",
                    "type": "create_tool",
                    "user_inputs": {
                        "name": "MLModelTool",
                        "type": "MLModelTool",
                        "alias": "language_model_tool",
                        "description": "A general tool to answer any question. But it can't handle math problem",
                        "parameters": {
                            "model_id": "ldzS04kBxRPZ5cnWrqpd",
                            "prompt": "Answer the question as best you can.",
                            "response_filter": "choices[0].message.content"
                        }
                    }
                },
                {
                    "id": "workflow_step_2",
                    "type": "create_tool",
                    "user_inputs": {
                        "name": "MathTool",
                        "type": "MathTool",
                        "description": "A general tool to calculate any math problem. The action input must be valid math expression, like 2+3",
                        "parameters": {
                            "max_iteration": 5
                        }
                    }
                },
                {
                    "id": "workflow_step_3",
                    "type": "register_agent",
                    "previous_node_inputs": {
                        "workflow_step_1": "tools",
                        "workflow_step_2": "tools"
                    },
                    "user_inputs": {
                        "name": "TOOLS-TOOLS-TOOLS",
                        "type": "cot",
                        "description": "this is a test agent",
                        "parameters": {"hello": "world"},
                        "llm.model_id": "ldzS04kBxRPZ5cnWrqpd",
                        "llm.parameters": {
                                "max_iteration": "5",
                                "stop_when_no_tool_found": "true"
                        },
                        
                        "memory": {
                            "type": "conversation_buffer_window"
                        },
                        "app_type": "chatbot",
                        "created_time": 1689793598499,
                        "last_updated_time": 1689793598530
                    }
                }
            ],
            "edges": [
                {
                    "source": "workflow_step_1",
                    "dest": "workflow_step_3"
                },
                {
                    "source": "workflow_step_2",
                    "dest": "workflow_step_3"
                }
            ]
        }
    }
}

Response after logging the output:

[2023-12-02T23:14:34,901][INFO ][o.o.f.w.RegisterAgentStep] [ip-172-31-56-214] TOOL ADDED MLModelTool
[2023-12-02T23:14:34,901][INFO ][o.o.f.w.RegisterAgentStep] [ip-172-31-56-214] TOOLS LIST [org.opensearch.ml.common.agent.MLToolSpec@46b3a260]

What is the expected behavior?

The outputs field should have all the responses of the previous workflow steps.

What is your host/environment?

Ubuntu

Do you have any screenshots?

If applicable, add screenshots to help explain your problem.

Do you have any additional context?

Add any other context about the problem.

@owaiskazi19 owaiskazi19 added bug Something isn't working untriaged labels Dec 2, 2023
@dbwiddis dbwiddis changed the title [BUG] outputs of the execute interface doesn't contain all workflow steps responses [BUG] The WorkflowStepFactory is not actually a factory Dec 3, 2023
@dbwiddis
Copy link
Member

dbwiddis commented Dec 3, 2023

The problem is that the WorkflowStepFactory isn't actually a factory. It's just storing a map of instances, so if the same workflow type is called a second time, it returns the same instance. If the future is already completed, then future.complete() becomes a no-op and the second "instance" retains the value of the completed future from the first instance.

I'll fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants