Skip to content

Conversation

@ThomasVitale
Copy link
Contributor

  • Update AnthropicChatModel to use the new ToolCallingManager API, while ensuring full API backward compatibility.
  • Introduce Builder to instantiate a new AnthropicChatModel since the number of overloaded constructors is growing too big.
  • Update documentation about tool calling and Anthropic support for that.

Relates to gh-2207

- Update AnthropicChatModel to use the new ToolCallingManager API, while ensuring full API backward compatibility.
- Introduce Builder to instantiate a new AnthropicChatModel since the number of overloaded constructors is growing too big.
- Update documentation about tool calling and Anthropic support for that.

Signed-off-by: Thomas Vitale <[email protected]>
@ThomasVitale ThomasVitale marked this pull request as ready for review February 12, 2025 23:31
if (!isProxyToolCalls(prompt, this.defaultOptions) && this.isToolCall(chatResponse, Set.of("tool_use"))) {
var toolCallConversation = handleToolCalls(prompt, chatResponse);
return this.internalStream(new Prompt(toolCallConversation, prompt.getOptions()), chatResponse);
if (ToolCallingChatOptions.isInternalToolExecutionEnabled(prompt.getOptions()) && chatResponse.hasToolCalls() && chatResponse.hasFinishReasons(Set.of("tool_use"))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For Anthropic in streaming mode, checking for tool calls is not enough. We also need to check for the finish reason.

That was not necessary for OpenAI, Ollama and Mistral, for which it's enough to check for tool calls.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeh, I remember that there were a cases requiring the finish reason. we have to review the bedrock-converse and the vertex gemini refactoring again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think we can add some auto-tests in those special cases, also as a way to document the behavior (ensuring tools are only called once).

I thought about adding some additional tests here for Anthropic, but we first need this defect fixed: #1370

@tzolov
Copy link
Contributor

tzolov commented Feb 13, 2025

Thanks @ThomasVitale
Added minor doc improvements. Rebased and merged at b7dcfc1

@tzolov tzolov closed this Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants