Add tool parameter type validation

[nathaliellenaa]

Description

Add tool parameter type validation.

• Add new parameter max_question_length/max_input_length to control max size for question/input field. Default is 10000. Users can change the value by passing this parameter when register an agent or executing tool

// Register agent
POST /_plugins/_ml/agents/_register
{
  "name": "Test agent tool",
  "type": "flow",
  "description": "this is a test agent",
  "tools": [
    {
      "type": "AgentTool",
      "description": "A general agent to answer any question",
      "parameters": {
        "agent_id": "9X7xWI0Bpc3sThaJdY9i",
        "max_question_length": 10
      }
    }
  ]
}

// Execute agent
POST /_plugins/_ml/tools/_execute/AgentTool
{
    "parameters": {
        "agent_id": "g-CELJoBkoz7WY_qrUkJ",
        "question": "how many indices in my cluster?"
        "max_question_length": 20000
      }
}

// Sample error response
{
    "status": 400,
    "error": {
        "type": "IllegalArgumentException",
        "reason": "Invalid Request",
        "details": "question length cannot exceed 10 characters"
    }
}

• Add validations for malformed input

Examples:

POST /_plugins/_ml/tools/_execute/ListIndexTool
{
  "parameters": {
    "question": ["invalid"]
  }
}

// Error response
{
    "status": 400,
    "error": {
        "type": "OpenSearchParseException",
        "reason": "Invalid Request",
        "details": "[question] property isn't a string, but of type [java.util.ArrayList]"
    }
}

More examples in this doc:
Tool Validation Test.pdf

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[nathaliellenaa]

Working on adding tests

[rithin-pullela-aws]

Thanks for this PR Nathalie, these validations have been long time pending. Added 2 small comments, other than that LGTM!

[rithin-pullela-aws]

Why would the index mapping tool, need a "question" attribute?
Looks like we are not using it anywhere

[nathaliellenaa]

We use it during tool execution. Also, in the doc, it listed question as a required parameter.

POST /_plugins/_ml/agents/9X7xWI0Bpc3sThaJdY9i/_execute
{
  "parameters": {
    "index": [ "sample-ecommerce" ],
    "question": "What fields are in the sample-ecommerce index?"
  }
}

I tested and we can run this tool without the question parameter. I'll update the doc to make it optional.
We can keep the question length validation here since users can still pass the question parameter, but I'll modify this to optional

ml-commons/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/IndexMappingTool.java

Line 229 in 8932e94

ConfigurationUtils.readStringProperty(TYPE, null, params, "question");

[zane-neo]

Although the question is shown in the parameter map passed to tools, but question is an agent level parameter, let's say an agent runs 10 tools, we only need to validate the question once when agent runs, instead of validating it 10 times when each tool runs.

[rithin-pullela-aws]

Same as above, we don't need the question parameter. The tool just ignores the question param

[nathaliellenaa]

Same thing here, we use it during tool execution. Doc marked it as a required field. But looking at the code, we can actually run this tool without question parameter. I'll update the doc to make this parameter optional.

We can keep the question length validation here since users can still pass the question parameter, but I'll modify this to optional

ml-commons/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java

Line 471 in 8932e94

ConfigurationUtils.readStringProperty(TYPE, null, params, "question");

[nathaliellenaa]

Some outdated tool information I found during testing, will open doc PR to update these

• ListIndexTool
  • indices should be array type
  • question parameter should be optional
• IndexMappingTool
  • input and question parameter should be optional
• SearchIndexTool
  • Mark input parameter as required
• ReadFromScratchPadTool
  • persistent_notes parameter is optional when executing tool directly

[zane-neo]

Although the question is shown in the parameter map passed to tools, but question is an agent level parameter, let's say an agent runs 10 tools, we only need to validate the question once when agent runs, instead of validating it 10 times when each tool runs.

[zane-neo]

I don't see the benefit of parsing these values, if a tool requires different types like map or double/float, then they're still being passed to tool in String format.

[zane-neo]

High level comment:

1. Can we add an interface method in org.opensearch.ml.common.spi.tools.Tool like getCreateToolParamsDefinition so that every tool returns either a json string like attributes or a Map<String, Class> to indicate what type the parameter should be. And validate the parameters when creating the agent?
2. question is an agent level parameter and should be validated in agent instead of tools.