Skip to content

Fix optional args toolcalling - #1538

Merged
albertvillanova merged 8 commits into
mainfrom
fix-optional-args-toolcalling
Jul 10, 2025
Merged

albertvillanova merged 8 commits into
mainfrom
fix-optional-args-toolcalling

Conversation

@aymeric-roucher

Copy link
Copy Markdown
Collaborator

No description provided.

@aymeric-roucher
aymeric-roucher force-pushed the fix-optional-args-toolcalling branch from 1a4461b to 702199d Compare July 9, 2025 16:46

@albertvillanova albertvillanova left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there might be some confusion around the term "nullable" in this context.

We should clarify terminology: "nullable" can be interpreted in two ways:

  • either that the argument can explicitly be set to None,
  • or that it's optional in the sense that it has a default value and doesn't need to be passed at all.

I think that distinction is important when reasoning about how LLMs should construct tool arguments.

@aymeric-roucher

aymeric-roucher commented Jul 10, 2025 •

Copy link
Copy Markdown
Collaborator Author

"nullable": True should be equivalent to "required": False, I agree with refactoring in a follow up PR to switch to using required instead of nullable. Do you see any inconsistency with this? Else we should go ahead and merge this IMO, it's a required fix else validate_tool_arguments is a breaking change.

@albertvillanova albertvillanova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. I just added a complete test suite (some of them to be addressed in a subsequent PR).

@albertvillanova
albertvillanova merged commit b1c72ff into main Jul 10, 2025
maxwellfet928 pushed a commit to maxwellfet928/smolagents that referenced this pull request Jun 8, 2026
Co-authored-by: Albert Villanova del Moral <8515462+albertvillanova@users.noreply.github.com>
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