Skip to content

Update tools.adoc About toolDefinition parameter transfer error #3468

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

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

RobinElysia
Copy link
Contributor

Method method = ReflectionUtils.findMethod(DateTimeTools.class, "getCurrentDateTime");
ToolCallback toolCallback = MethodToolCallback.builder()
.toolDefinition(ToolDefinition.builder() // Shouldn't method be passed here?
.description("Get the current date and time in the user's timezone")
.build())
.toolMethod(method)
.toolObject(new DateTimeTools())
.build();

There shouldn't be a method here. This is obviously wrong.

Signed-off-by: qwp_p <[email protected]>
Copy link
Contributor

@sunyuhan1998 sunyuhan1998 left a comment

Choose a reason for hiding this comment

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

Please refer to the following source code. I don't think there is any error in the document itself. Could you please double-check it?

public static DefaultToolDefinition.Builder builder(Method method) {

@RobinElysia
Copy link
Contributor Author

请参考以下来源代码。我认为文档本身没有任何错误。您可以再检查一下吗?

public static DefaultToolDefinition.Builder builder(Method method) {

Ok, maybe it's my version problem.😂

@RobinElysia
Copy link
Contributor Author

Please refer to the following source code. I don't think there is any error in the document itself. Could you please double-check it?

public static DefaultToolDefinition.Builder builder(Method method) {

No, no, no, that's not right. The version is fine. Sorry, it seems to be the fault of the document sample.😦

@RobinElysia
Copy link
Contributor Author

请参考以下来源代码。我认为文档本身没有任何错误。您可以再检查一下吗?

public static DefaultToolDefinition.Builder builder(Method method) {

This should be the interface:
spring-ai-model/src/main/java/org/springframework/ai/tool/definition/ToolDefinition.java

@sunyuhan1998
Copy link
Contributor

sunyuhan1998 commented Jun 9, 2025

I'm a bit confused about what you mean, the sample code in the current sample documentation is as follows:

ToolCallback toolCallback = MethodToolCallback.builder()
.toolDefinition(ToolDefinitions.builder(method)
.description("Get the current date and time in the user's timezone")
.build())
.toolMethod(method)
.toolObject(new DateTimeTools())
.build();

According to the interface definition, the org.springframework.ai.tool.method.MethodToolCallback.Builder#toolDefinition method receives a parameter of type ToolDefinition:

public Builder toolDefinition(ToolDefinition toolDefinition) {
this.toolDefinition = toolDefinition;
return this;
}

and ToolDefinitions.builder(method).build() returns a ToolDefinition:

which I don't seem to see a problem with.

@RobinElysia
Copy link
Contributor Author

I'm a bit confused about what you mean, the sample code in the current sample documentation is as follows:

ToolCallback toolCallback = MethodToolCallback.builder()
.toolDefinition(ToolDefinitions.builder(method)
.description("Get the current date and time in the user's timezone")
.build())
.toolMethod(method)
.toolObject(new DateTimeTools())
.build();

According to the interface definition, the org.springframework.ai.tool.method.MethodToolCallback.Builder#toolDefinition method receives a parameter of type ToolDefinition:

public Builder toolDefinition(ToolDefinition toolDefinition) {
this.toolDefinition = toolDefinition;
return this;
}

and ToolDefinitions.builder(method).build() returns a ToolDefinition:

which I don't seem to see a problem with.

ok,I think i get it

@ilayaperumalg
Copy link
Member

@RobinElysia Thanks for the PR fixing the doc.

@ilayaperumalg ilayaperumalg self-assigned this Jun 18, 2025
@ilayaperumalg ilayaperumalg added the documentation Improvements or additions to documentation label Jun 18, 2025
@ilayaperumalg ilayaperumalg modified the milestone: 1.1.x Jun 18, 2025
@ilayaperumalg ilayaperumalg merged commit 23c38df into spring-projects:main Jun 18, 2025
2 checks passed
spring-builds pushed a commit that referenced this pull request Jun 18, 2025
There shouldn't be a method here.

Fixes #3468

Signed-off-by: qwp_p <[email protected]>
(cherry picked from commit 23c38df)
@sunyuhan1998
Copy link
Contributor

@RobinElysia Thanks for the PR fixing the doc.

Hi @ilayaperumalg , I think this merge might be a mistake, because the original document was correct — ToolDefinitions does not have a builder() method that accepts no parameters.

public final class ToolDefinitions {
private ToolDefinitions() {
// prevents instantiation.
}
/**
* Create a default {@link ToolDefinition} builder from a {@link Method}.
*/
public static DefaultToolDefinition.Builder builder(Method method) {
Assert.notNull(method, "method cannot be null");
return DefaultToolDefinition.builder()
.name(ToolUtils.getToolName(method))
.description(ToolUtils.getToolDescription(method))
.inputSchema(JsonSchemaGenerator.generateForMethodInput(method));
}
/**
* Create a default {@link ToolDefinition} instance from a {@link Method}.
*/
public static ToolDefinition from(Method method) {
return builder(method).build();
}
}

@ilayaperumalg
Copy link
Member

@sunyuhan1998 yes, sorry about that. fixing the doc now

ilayaperumalg added a commit that referenced this pull request Jun 18, 2025
Auto-cherry-pick to 1.0.x

This reverts commit 23c38df.
spring-builds pushed a commit that referenced this pull request Jun 18, 2025
This reverts commit 23c38df.

(cherry picked from commit 5036ed3)
@ilayaperumalg
Copy link
Member

Reverted the change. @sunyuhan1998 Thanks for pointing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation status: backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants