Skip to content

bugfix: When the Tool parameter is an empty string, a type conversion error occurs #3885

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.

Sign up for GitHub

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

bugfix: When the Tool parameter is an empty string, a type conversion error occurs #3885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31515fa
bugfix: When the Tool parameter is an empty string, a type conversion…
lambochen Jul 22, 2025
0915de2
ut for JsonParser.toTypedObject when param is empty string
lambochen Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion spring-ai-model/src/main/java/org/springframework/ai/util/json/JsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

/**
* Utilities to perform parsing operations between JSON and Java.
*
* @author lambochen
*/
public final class JsonParser {

Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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:

  1. Scenarios where we should allow tool parameters to be null.
  2. 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with your point, provide users with clear feedback when there is a type mismatch.

}

if (javaType == Byte.class) {
return Byte.parseByte(value.toString());
}
else if (javaType == Integer.class) {
Expand Down
6 changes: 6 additions & 0 deletions spring-ai-model/src/test/java/org/springframework/ai/util/json/JsonParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ void doesNotDoubleSerializeValidJsonString() {
assertThat(input).isEqualTo(result);
}

@Test
void emptyString() {
var value = JsonParser.toTypedObject("", Long.class);
assertThat(value).isEqualTo(null);
}

record TestRecord(String name, Integer age) {
}

Expand Down