Skip to content

Conversation

howieleung
Copy link
Member

Description

Please add an informative description that covers that changes made by the pull request and link all relevant issues.

If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@Copilot Copilot AI review requested due to automatic review settings August 27, 2025 04:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the OpenApiTool functionality within the ToolSet to support multiple OpenAPI definitions. The main improvement allows adding multiple API definitions to a single OpenApiTool instance rather than requiring separate tools for each API specification.

Key Changes

  • Modified the add_definition method to accept optional descriptions
  • Enhanced the add method to merge new OpenApiTool definitions into existing OpenApiTool instances
  • Added overloaded remove methods to support removing specific API definitions by name or entire OpenApiTool instances

Copy link

github-actions bot commented Aug 27, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure-ai-agents



# Special handling for OpenApiTool - add definitions to existing tool instead of raising error
if isinstance(tool, OpenApiTool):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should do this. Having specific type checks for certain tools in ToolSet does not feel optimal

self._tools.append(tool)

def remove(self, tool_type: Type[Tool]) -> None:
@overload
def remove(self, tool_type: Type[OpenApiTool], *, name: str) -> None:
Copy link
Member

@jhakulin jhakulin Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment, I would not add tool specific overloads in Toolset. If Tool abstraction would contain add and remove then we would not need this.

Copy link
Member

@jhakulin jhakulin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to discuss more if we can make this more generically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants