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

tools: prioritizes explicitly passed parameters over generated ones #394

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

teocns
Copy link
Contributor

@teocns teocns commented Dec 7, 2024

Problem
weather_tool = cf.tool(
    name="get_current_weather",
    description="Get the current weather at a location",
    parameters={
        "type": "object",
        "properties": {
            "latitude": {
                "type": "number",
                "description": "The latitude of the location",
            },
            "longitude": {
                "type": "number",
                "description": "The longitude of the location",
            },
        },
        "required": ["latitude", "longitude"],
    },
    fn=get_current_weather,
)

TypeError: controlflow.tools.tools.Tool() got multiple values for keyword argument 'parameters'

Key changes
  1. We now handle custom parameters first, before any description processing
  2. Added a safety check if param.name not in parameters.get("properties", {}) to ensure we only try to add descriptions for parameters that exist in the schema
  3. The rest of the function (return value handling, description length check, etc.) remains unchanged.

This way:

  • Custom parameters are respected
  • Parameter descriptions from annotations are still added when available
  • The function remains backward compatible with all existing tests
  • We avoid trying to add descriptions to parameters that don't exist in the schema
Tests
  1. Basic custom parameters override - ensuring that provided parameters completely replace the generated ones
  2. Annotation handling with custom parameters - verifying that parameter descriptions from annotations are still added when the parameter names match
  3. Disabling parameter descriptions - confirming that include_param_descriptions=False works correctly with custom parameters

@teocns teocns changed the title tools: prioritizes explicitly passed parameters over automatically ge… tools: prioritizes explicitly passed parameters over generated ones Dec 7, 2024
@github-actions github-actions bot added the tests Adds or improves unit tests label Dec 7, 2024
@teocns teocns force-pushed the fix/tool-parameters-parsing branch from 73f1e18 to 2c544c1 Compare December 7, 2024 20:32
…nerated ones

Key changes:
1. We now handle custom parameters first, before any description
processing
2. Added a safety check `if param.name not in
parameters.get("properties", {})` to ensure we only try to add
descriptions for parameters that exist in the schema3. The rest of the
function (return value handling, description length check, etc.) remains
unchanged. This way:
- Custom parameters are respected
- Parameter descriptions from annotations are still added when available
- The function remains backward compatible with all existing tests
- We avoid trying to add descriptions to parameters that don't exist in
the schema

Signed-off-by: Teo <[email protected]>
@teocns teocns force-pushed the fix/tool-parameters-parsing branch from 2c544c1 to 06967d0 Compare December 7, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Adds or improves unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant