bugfix: When the Tool parameter is an empty string, a type conversion error occurs #3885
Conversation
… error occurs, reporting java.lang.NumberFormatException Signed-off-by: lambochen <lambochen@yeah.net>
Signed-off-by: lambochen <lambochen@yeah.net>
| @@ -141,7 +143,11 @@ public static Object toTypedObject(Object value, Class<?> type) { | |||
| if (javaType == String.class) { | |||
| return value.toString(); | |||
| } | |||
| else if (javaType == Byte.class) { | |||
| if (value.toString().isEmpty()) { | |||
| return null; | |||
There was a problem hiding this comment.
The package-info for the package containing JsonParser has already been declared as @NonNullApi, so I think we should never return null under any circumstances.
I believe the current issue is essentially caused by a type mismatch between the parameters provided by the model when calling the tool and the actual parameter types expected by the tool. Therefore, I suggest we directly throw a clear exception here, including in the exception message the parameter types expected by the tool, as well as the parameter types (and values) actually provided by the model.
This would also help users improve their prompts and tool descriptions.
There was a problem hiding this comment.
Hi, @sunyuhan1998
Thank you for your comment.
throw a clear exception
Regarding this point, I have a different opinion.
From the perspective of this package, it indeed should not return null, as @NonNullApi has been declared. Excuse me for overlooking this when coding; I will make the adjustment.
But from the perspective of Tool Calling, parameter conversion can allow null values because the tool parameter may be optional.
In summary, I think solving this issue might be better addressed by the caller of JsonParser, i.e., the Tool Calling parameter parsing layer.
There was a problem hiding this comment.
parameter conversion can allow null values because the tool parameter may be optional.
I agree with your point that tool parameters should indeed allow null values.
I think perhaps we are currently facing two distinct issues:
- Scenarios where we should allow tool parameters to be null.
- Scenarios where the parameters provided by the model do not match the required parameter types of the tool.
I’m trying to express mainly relates to the second scenario. In your example, the tool expects a parameter of type Long, but the model outputs an empty string. If we extend this discussion further, suppose the tool expects a Boolean parameter, but the model provides the number 5—how should we handle such cases? Therefore, I believe it's necessary to provide users with clear feedback when such mismatches occur.
There was a problem hiding this comment.
I agree with your point, provide users with clear feedback when there is a type mismatch.
ref issue: #3884
Thank you for taking time to contribute this pull request!
You might have already read the contributor guide, but as a reminder, please make sure to:
git commit -s) per the DCOmainbranch and squash your commitsFor more details, please check the contributor guide.
Thank you upfront!