-
Notifications
You must be signed in to change notification settings - Fork 10
fix: add external tools #319
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
Conversation
ai21/models/maestro/run.py
Outdated
type: Literal["object"] | ||
properties: Dict[str, HTTPToolFunctionParamProperties] | ||
required: List[str] | ||
additionalProperties: Optional[bool] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make it snake_case and then serialize it so it aligns with python's conventions?
ai21/models/maestro/run.py
Outdated
Role = Literal["user", "assistant"] | ||
RunStatus = Literal["completed", "failed", "in_progress", "requires_action"] | ||
ToolType = Literal["file_search", "web_search"] | ||
ToolType = Literal["file_search", "web_search", "http", "mcp"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we need to update it as each Tool schema has the type on its own
ai21/models/maestro/run.py
Outdated
|
||
|
||
class McpTool(TypedDict, total=False): | ||
type: Required[Literal["mcp"]] = "mcp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the type is required and there is only one option, is it really necessary to make it required?
Based on the external tool use PRD
we shall expose external tools in the Maestro API. external tool can be HTTP or MCP. They are both added to the tools list in the maestro run.