Skip to content

Conversation

tzolov
Copy link
Contributor

@tzolov tzolov commented Jul 24, 2024

  • extend the OllamaApi with Tool, ToolCalls and Function and add Tool Role.
  • make OllamaChatModel extend the AbstractToolCallSupport.
  • extend the OllamaChatModel to convert the Spring AI messages to OllamaApi messages, including Tools.
  • OllamaOptions implements FunctionCallingOptions.
  • add OllamaApiToolFunctionCallIT for testing function calling.
  • patch the AbstractToolCallSupport#isToolCall to take set of stop resons.
  • add FunctionCallbackInPromptIT and FunctionCallbackWrapperIT function calling auto-configuration tests.
  • extend OllamAutoConfiguration to support function calling registration.
  • add function call tests to OllamChatAutoConfigurationIT.
  • add OllamaWithOpenAiChatModelIT to OpenAi that uses the OpenAI API to call Ollama.
  • move buildToolCallConversation and handleToolCalls to parent AbstractToolCallSupport.
  • update Docs. add Ollama function call docs.
  • update Ollama diagrams.

Resolves #720

- extend the OllamaApi with Tool, ToolCalls and Function and add Tool Role.
- make OllamaChatModel extend the AbstractToolCallSupport.
- extend the OllamaChatModel to convert the Spring AI messages to OllamaApi messages, including Tools.
- OllamaOptions implements FunctionCallingOptions.
- add OllamaApiToolFunctionCallIT for testing function calling.
- patch the AbstractToolCallSupport#isToolCall to take set of stop resons.
- add FunctionCallbackInPromptIT and FunctionCallbackWrapperIT function calling auto-configuration tests.
- extend OllamAutoConfiguration to support function calling registration.
- add function call tests to OllamChatAutoConfigurationIT.
- add OllamaWithOpenAiChatModelIT to OpenAi that uses the OpenAI API to call Ollama.
- move buildToolCallConversation and handleToolCalls to parent AbstractToolCallSupport.
- update Docs. add Ollama function call docs.
- update Ollama diagrams.

Resolves spring-projects#720
* @param parameters The parameters the functions accepts, described as a JSON Schema object. To describe a
* function that accepts no parameters, provide the value {"type": "object", "properties": {}}.
*/
public record Function(
Copy link
Contributor

Choose a reason for hiding this comment

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

For parity with the Ollama types, this might be named ToolFunction

Copy link
Member

Choose a reason for hiding this comment

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

It is already nested inside the record Tool, so new ChatRequest.Tool.ToolFunction would read awkardly.

assertThat(responseMessage.toolCalls()).isNotNull();

// Check if the model wanted to call a function
if (responseMessage.toolCalls() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check shouldn't be needed since the previous assertion makes sure the tool calls are not null.

Copy link
Member

Choose a reason for hiding this comment

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

removing

@@ -175,7 +175,7 @@ public ChatResponse call(Prompt prompt) {

ChatResponse chatResponse = new ChatResponse(generations, from(completionEntity.getBody(), rateLimit));

if (isToolCall(chatResponse, OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name())) {
if (isToolCall(chatResponse, Set.of(OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name(), "stop"))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be Set.of(OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name(), OpenAiApi.ChatCompletionFinishReason.STOP.name())?

Also, consider adding a comment about why we need the "stop" reason as well.

Copy link
Member

Choose a reason for hiding this comment

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

OpenAiApi.ChatCompletionFinishReason.STOP.name()) seems better

@Disabled("For manual smoke testing only.")
@Testcontainers
@SpringBootTest(classes = OllamaWithOpenAiChatModelIT.Config.class)
class OllamaWithOpenAiChatModelIT {
Copy link
Contributor

Choose a reason for hiding this comment

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

Really nice!

@@ -95,6 +97,20 @@ protected Set<String> handleFunctionCallbackConfigurations(FunctionCallingOption
return functionToCall;
}

protected List<Message> handleToolCalls(Prompt prompt, ChatResponse response) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's really nice we could abstract this. It might help in the future if we allow "deconstructing" the function calling workflow and have users calling the functions explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed I was thinking the same. Maybe the AbstractToolCallSuppor is not the perfect abstraction for direct usage but we will figure it out.

@Testcontainers
public class OllamaApiToolFunctionCallIT {

private static String MODEL = "mistral";
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be

private static final String MODEL = OllamaModel.MISTRAL.getName();

Copy link
Member

Choose a reason for hiding this comment

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

sounds good

@@ -65,7 +64,7 @@ class OllamaChatModelIT {
@Container
static OllamaContainer ollamaContainer = new OllamaContainer("ollama/ollama:0.1.32");
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider updating this image to version "0.2.8" as well

@@ -53,7 +53,7 @@ class OllamaChatModelMultimodalIT {
@Container
static OllamaContainer ollamaContainer = new OllamaContainer("ollama/ollama:0.1.32");
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider updating this image to version "0.2.8" as well

Copy link
Member

Choose a reason for hiding this comment

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

I updated and it seems there is a regression, the description of the image returned now is totally wrong. I'll merge the PR with the updated versions but we need to circle back and manually test.


private static final Logger logger = LoggerFactory.getLogger(OllamaChatModelFunctionCallingIT.class);

private static String MODEL = "mistral";
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be

private static final String MODEL = OllamaModel.MISTRAL.getName();

@ThomasVitale
Copy link
Contributor

Super!

@markpollack markpollack self-assigned this Jul 24, 2024
@markpollack
Copy link
Member

addressed comments and made other minor cosmetic changes, merged in d53876a

@markpollack markpollack added this to the 1.0.0-M2 milestone Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support invoking functions in models that support it via Ollama
3 participants