Skip to content

Conversation

@AirswitchAsa
Copy link

@AirswitchAsa AirswitchAsa commented Nov 18, 2025

Description

The toolUseId field is supposed to provide an unique identifier of the tool use to the user, but it was instead filled with the tool name if user is using a Gemini model. Reason behind is that Gemini requires that name be set in the equivalent FunctionResponse type. Consequently, the current code assigns tool name to toolUseId in the tool use block.

As a solution, this PR proposes the following changes:

  • create empty map _tool_use_id_to_name upon initialization
  • generate an unique id for the tool call in case "tool" and save to the map
  • look up the function name from the map when constructing FunctionResponse
  • clear the map at the beginning of each stream

Related Issues

#1200

Documentation PR

Type of Change

Bug fix

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Raises:
ModelThrottledException: If the request is throttled by Gemini.
"""
self._tool_use_id_to_name.clear()
Copy link
Member

@pgrayy pgrayy Nov 20, 2025

Choose a reason for hiding this comment

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

Let's go ahead and clear this after request transformation (right after the call to _format_request). We won't need to hold onto the mapping in between calls to streaming.

@pgrayy
Copy link
Member

pgrayy commented Nov 20, 2025

Last note, can you make sure to run hatch run prepare. I see there are some unit tests failing.

@AirswitchAsa
Copy link
Author

Last note, can you make sure to run hatch run prepare. I see there are some unit tests failing.

yes, I finally got it working :) thank you

@AirswitchAsa AirswitchAsa requested a review from pgrayy November 26, 2025 21:55
@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mkmeral
Copy link
Contributor

mkmeral commented Dec 22, 2025

@AirswitchAsa can you merge from main? There are some failing checks, and merge from main should fix those.

I'll approve the change as is. It won't get merged without passing approval workflows, and merge from main won't impact my approval

@AirswitchAsa
Copy link
Author

@AirswitchAsa can you merge from main? There are some failing checks, and merge from main should fix those.

I'll approve the change as is. It won't get merged without passing approval workflows, and merge from main won't impact my approval

Sorry that I just saw your comment - I have just merged it with latest main

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants