Skip to content

feat(toolbox-core): Add support for optional parameters #290

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

Merged
merged 30 commits into from
Jul 2, 2025

Conversation

anubhav756
Copy link
Contributor

@anubhav756 anubhav756 commented Jun 19, 2025

Background

The previous implementation of toolbox-core treated all tool parameters as implicitly required. This limited the flexibility of tool definitions and did not align with common use cases where tools have optional arguments with default behaviors.

This PR introduces full support for optional parameters, corresponding to the feature added to the Toolbox server (#617), enabling developers to create more versatile and powerful tools. The changes ensure that optionality is correctly reflected and handled at every stage: from the function signature and type hinting down to the final payload sent to the server.

Proposed Changes

This PR introduces a series of coordinated changes across the parameter handling and tool invocation pipeline:

1. ParameterSchema Enhancement

  • A new required: bool = True field has been added to the ParameterSchema of the Toolbox protocol, allowing tool manifest authors to explicitly mark parameters as optional.
  • The internal __get_type() method now correctly wraps the type in typing.Optional when a parameter has required=False.
  • The to_param() method now constructs an inspect.Parameter with default=None for optional parameters.

2. Correct Signature Generation

  • In the ToolboxTool.__init__ method, the list of tool parameters is now sorted to place all required parameters before optional ones.
    • This robustly prevents a ValueError: non-default argument follows default argument that could occur if a manifest defined an optional parameter before a required one.

3. Aligned Pydantic Validation

The params_to_pydantic_model utility function has been updated to be aware of the required flag. It now generates Pydantic Field definitions with default=None for optional fields, ensuring runtime validation is consistent with the function signature.

4. Cleaner API Payloads

  • The payload now only includes parameters that were explicitly provided by the user in the function call.
  • Optional parameters that are omitted will be completely left out of the JSON request body, rather than being sent with a null value.
    • The Toolbox server fills such a field with the user-provided default value.

Impact

  • For Tool Authors: You can now define optional parameters in your tool manifests by setting `default: "Some Value".
  • For Tool Users: Tools with optional parameters will now have correct, user-friendly Python signatures (e.g., my_tool(required_arg: str, optional_arg: Optional[str] = "Some Value")). Standard tools like help()` and editor autocompletion will work as expected.
  • For API Integrity: Requests sent to the Toolbox server are cleaner and more concise, reducing ambiguity by omitting keys for unprovided optional values.

@anubhav756 anubhav756 self-assigned this Jun 19, 2025
@anubhav756 anubhav756 requested review from kurtisvg and Yuan325 June 19, 2025 07:41
@anubhav756 anubhav756 marked this pull request as ready for review June 19, 2025 08:08
@anubhav756 anubhav756 requested a review from a team as a code owner June 19, 2025 08:08
@anubhav756
Copy link
Contributor Author

/gcbrun

kurtisvg
kurtisvg previously approved these changes Jun 19, 2025
kurtisvg
kurtisvg previously approved these changes Jun 20, 2025
kurtisvg
kurtisvg previously approved these changes Jun 24, 2025
@anubhav756 anubhav756 force-pushed the anubhav-optional-params branch from 57aeeb2 to d6a153e Compare July 2, 2025 13:44
@anubhav756 anubhav756 requested a review from kurtisvg July 2, 2025 13:50
@anubhav756 anubhav756 force-pushed the anubhav-optional-params branch 3 times, most recently from 1c598a4 to 90f7472 Compare July 2, 2025 16:17
@anubhav756 anubhav756 force-pushed the anubhav-optional-params branch from 90f7472 to 7e38892 Compare July 2, 2025 17:08
@anubhav756 anubhav756 merged commit 3c8360c into main Jul 2, 2025
22 checks passed
@anubhav756 anubhav756 deleted the anubhav-optional-params branch July 2, 2025 17:46
@release-please release-please bot mentioned this pull request Jul 1, 2025
anubhav756 added a commit to GoogleCloudPlatform/genai-databases-retrieval-app that referenced this pull request Jul 3, 2025
* Remove client session management from the orchestration class
  * This is now managed by Toolbox SDK internally
* Simplify prompt creation
  * This is handled by the respective tools
* Tools descriptions, params, annotations, etc. are loaded from Toolbox
  * These are added through `bind_tools` from LangGraph
* This enables removal of the custom response message creation (which
was added as a `TODO`)
* Add logged in user's token to the `RunnableConfig`
* This is necessary so that the tools that require authentication can
read the user's token if available
* Simplify tools helper file by removing tools and helpers since those
are now handled by Toolbox SDK internally
* Add a Toolbox URL to connect to through integration tests
* Removes `ToolMessage` while inserting ticket.
  * This was causing an issue with `langchain-google-vertexai`

  ```
google.api_core.exceptions.InvalidArgument: 400 Please ensure that the
number of function response parts should be equal to number of function
call parts of the function call turn.
  ```
* Remove unused human and AI messages post book ticket flow.

## Diagram

![image](https://github.com/user-attachments/assets/46af0a74-3395-45a8-8ff4-f5466b034f17)

> [!IMPORTANT]
> This PR depends on a couple of features from Toolbox SDK:
> * Support for optional parameters
([#290](googleapis/mcp-toolbox-sdk-python#290))
> * Self-authenticated tools via `RunnableConfig`
([#291](googleapis/mcp-toolbox-sdk-python#291))

> [!NOTE]
> The failure in the integration test is expected. This PR is part of a
series of changes, and the corresponding fix for this test is in a
subsequent PR.
> ### Reasoning
> We've intentionally split the work into smaller, focused PRs to make
the review process more manageable and efficient.
> ### Merge Plan
> All related PRs will be merged into the `toolbox-main` branch first.
We will ensure all tests are passing on `toolbox-main` before merging
the entire feature set into `main`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants