ToolCallingChatOptions support internalToolExecutionMaxAttempts, to limit the maximum number of tool calls and prevent infinite recursive calls to LLM in special cases

[lambochen]

This PR primarily addresses the issue of infinite recursive calls to LLM under certain special conditions, with internalToolExecutionEnabled=true in place, ref issue: #3333.

By introducing support for internalToolExecutionMaxAttempts in ToolCallingChatOptions and performing call count checks in each ChatModel, interrupt the process when the maximum call count is reached.

[ x ] Sign the contributor license agreement
[ x ] Rebase your changes on the latest main branch and squash your commits
[ x ] Add/Update unit tests as needed
[ x ] Run a build and make sure all tests pass prior to submission

[markpollack]

thanks, one comment is that we need to preserve api compatability and

public ChatResponse internalCall(Prompt prompt, ChatResponse previousChatResponse, int attempts) {

would need to be an overload vs adding a new method parameter.

[lambochen]

thanks, one comment is that we need to preserve api compatability and

public ChatResponse internalCall(Prompt prompt, ChatResponse previousChatResponse, int attempts) {

would need to be an overload vs adding a new method parameter.

Your suggestion is great, I'll optimize it, thanks

[lambochen]

Hi, @markpollack @tzolov , the PR is ready, please review it, thank you.

If there are any issues or if additional content is needed, please to comment, and I will promptly correct or supplement it. thank you

[ThomasVitale]

Consider renaming this to something like toolExecutionMaxIterations.

I'm suggesting dropping the internal prefix since this option can also be used as part of the user-controller tool execution, which is executed externally from the ChatModel.

Also, I'm suggesting replacing attempts with iterations since attempts would make seem related to the concept of retrying tool calls, whereas here we want to limit the number of iterations in the loop supporting the tool call requests from the model.

[lambochen]

Hi, ThomasVitale, thank you for your attention and comments.

Regarding removing the internal prefix: I agree. In the past，considering consistency with internalToolExecutionEnabled, I think we should reconsider this.

Regarding replacing attempts with iterations: Yes, I accept your suggestion, actually, I initially considered iterations as well.

[lambochen]

Of course, I also agree with the naming of the previous developer. The internal can express that it is executed internally by the Spring AI framework, which is a preset behavior.

Regarding the internal prefix, I support not using the prefix, as I believe this is a capability of the framework, a feature. The internal prefix makes users think this is a field only for internal use of the framework, and using it may come with some "concerns."

Discussion：If the internal prefix is removed, should the original internalToolExecutionEnabled field also be deprecated and a new toolExecutionEnabled created to replace it?

[ThomasVitale]

I would say it's a mistake that no limit was set in the original implementation. I would consider having a lower value as the default limit. We should think of a good default value here that avoids infinite loops.

[lambochen]

Yes, I agree with your point. To ensure downward compatibility, I have used Integer.MAX_VALUE (a very large number) to indicate no limit.

I tend to prefer setting a smaller value to limit the number of executions and the scope of impact on the program, but I don't have specific comparative data. If you have suggestions, please feel free to share them. Thank you

[ThomasVitale]

In general, I would suggest replacing attempts with iterations. In this case, I would make it more specific and name the argument toolExecutionIterations or toolExecutionCompletedIterations or something like that.

[lambochen]

Thank you for your suggestion, I will rename attempts to use iterations.