From c418266ae302c7917dc39d6096c1e50ed110eda7 Mon Sep 17 00:00:00 2001 From: John Oliver <1615532+johnoliver@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:41:23 +0100 Subject: [PATCH] Java: Fix required tool call being requested multiple times (#5962) ### Motivation and Context ### Description ### Contribution Checklist - [ ] The code builds clean without any errors or warnings - [ ] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone :smile: --- .../chatcompletion/OpenAIChatCompletion.java | 70 +++++++++++++++---- .../semantickernel/tests/RenderingTest.java | 57 +++++++++++++++ .../Example09_FunctionTypes.java | 41 +++++++++++ .../KernelFunctionFromMethod.java | 13 ++-- 4 files changed, 160 insertions(+), 21 deletions(-) diff --git a/java/aiservices/openai/src/main/java/com/microsoft/semantickernel/aiservices/openai/chatcompletion/OpenAIChatCompletion.java b/java/aiservices/openai/src/main/java/com/microsoft/semantickernel/aiservices/openai/chatcompletion/OpenAIChatCompletion.java index 4b72861269..445ee1c9a4 100644 --- a/java/aiservices/openai/src/main/java/com/microsoft/semantickernel/aiservices/openai/chatcompletion/OpenAIChatCompletion.java +++ b/java/aiservices/openai/src/main/java/com/microsoft/semantickernel/aiservices/openai/chatcompletion/OpenAIChatCompletion.java @@ -52,6 +52,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -200,15 +201,11 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio return performToolCall(kernel, invocationContext, requestMessages, toolCall); } + return requestMessages; }) .flatMap(it -> it) .flatMap(msgs -> { - ChatRequestMessage m = msgs.get(msgs.size() - 1); - if (m instanceof ChatRequestAssistantMessage && - ((ChatRequestAssistantMessage) m).getToolCalls().size() > 0) { - LOGGER.error("No response"); - } return internalChatMessageContentsAsync(msgs, kernel, functions, invocationContext, autoInvokeAttempts - 1); }) @@ -262,9 +259,9 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio functionResult.getResult(), functionToolCall.getId()); - ArrayList res = new ArrayList<>( - msgs); + ArrayList res = new ArrayList<>(msgs); res.add(requestToolMessage); + return res; }) .switchIfEmpty(Mono.fromSupplier( @@ -456,8 +453,11 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio .setModel(chatCompletionService.getModelId()); if (invocationContext != null && invocationContext.getToolCallBehavior() != null) { - configureToolCallBehaviorOptions(options, invocationContext.getToolCallBehavior(), - functions, autoInvokeAttempts); + configureToolCallBehaviorOptions( + options, + invocationContext.getToolCallBehavior(), + functions, + chatRequestMessages); } PromptExecutionSettings promptExecutionSettings = invocationContext != null @@ -523,7 +523,7 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio ChatCompletionsOptions options, @Nullable ToolCallBehavior toolCallBehavior, @Nullable List functions, - int autoInvokeAttempts) { + List chatRequestMessages) { if (toolCallBehavior == null) { return; @@ -537,6 +537,18 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio if (toolCallBehavior instanceof ToolCallBehavior.RequiredKernelFunction) { KernelFunction toolChoice = ((ToolCallBehavior.RequiredKernelFunction) toolCallBehavior) .getRequiredFunction(); + + String toolChoiceName = String.format("%s%s%s", + toolChoice.getPluginName(), + OpenAIFunction.getNameSeparator(), + toolChoice.getName()); + + // If required tool call has already been called dont ask for it again + boolean hasBeenExecuted = hasToolCallBeenExecuted(chatRequestMessages, toolChoiceName); + if (hasBeenExecuted) { + return; + } + List toolDefinitions = new ArrayList<>(); toolDefinitions.add(new ChatCompletionsFunctionToolDefinition( @@ -547,10 +559,7 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio options.setTools(toolDefinitions); try { String json = String.format( - "{\"type\":\"function\",\"function\":{\"name\":\"%s%s%s\"}}", - toolChoice.getPluginName(), - OpenAIFunction.getNameSeparator(), - toolChoice.getName()); + "{\"type\":\"function\",\"function\":{\"name\":\"%s\"}}", toolChoiceName); options.setToolChoice(BinaryData.fromObject(new ObjectMapper().readTree(json))); } catch (JsonProcessingException e) { throw new RuntimeException(e); @@ -582,6 +591,39 @@ public class OpenAIChatCompletion extends OpenAiService implements ChatCompletio options.setToolChoice(BinaryData.fromString("auto")); } + private static boolean hasToolCallBeenExecuted(List chatRequestMessages, + String toolChoiceName) { + return chatRequestMessages + .stream() + .flatMap(message -> { + // Extract tool calls + if (message instanceof ChatRequestAssistantMessage) { + return ((ChatRequestAssistantMessage) message).getToolCalls().stream(); + } + return Stream.empty(); + }) + .filter(toolCall -> { + // Filter if tool call has correct name + if (toolCall instanceof ChatCompletionsFunctionToolCall) { + return ((ChatCompletionsFunctionToolCall) toolCall).getFunction().getName() + .equals(toolChoiceName); + } + return false; + }) + .allMatch(toolcall -> { + String id = toolcall.getId(); + // True if tool call id has a response message + return chatRequestMessages + .stream() + .filter( + chatRequestMessage -> chatRequestMessage instanceof ChatRequestToolMessage) + .anyMatch( + chatRequestMessage -> ((ChatRequestToolMessage) chatRequestMessage) + .getToolCallId() + .equals(id)); + }); + } + private static List getChatRequestMessages(ChatHistory chatHistory) { List> messages = chatHistory.getMessages(); if (messages == null || messages.isEmpty()) { diff --git a/java/api-test/integration-tests/src/test/java/com/microsoft/semantickernel/tests/RenderingTest.java b/java/api-test/integration-tests/src/test/java/com/microsoft/semantickernel/tests/RenderingTest.java index 9e3a534a37..31539c6bb1 100644 --- a/java/api-test/integration-tests/src/test/java/com/microsoft/semantickernel/tests/RenderingTest.java +++ b/java/api-test/integration-tests/src/test/java/com/microsoft/semantickernel/tests/RenderingTest.java @@ -10,15 +10,19 @@ import com.github.tomakehurst.wiremock.junit5.WireMockTest; import com.microsoft.semantickernel.Kernel; import com.microsoft.semantickernel.aiservices.openai.chatcompletion.OpenAIChatCompletion; import com.microsoft.semantickernel.aiservices.openai.textcompletion.OpenAITextGenerationService; +import com.microsoft.semantickernel.plugin.KernelPluginFactory; import com.microsoft.semantickernel.semanticfunctions.KernelFunction; import com.microsoft.semantickernel.semanticfunctions.KernelFunctionArguments; import com.microsoft.semantickernel.semanticfunctions.PromptTemplateConfig; +import com.microsoft.semantickernel.semanticfunctions.annotations.DefineKernelFunction; import com.microsoft.semantickernel.services.chatcompletion.ChatCompletionService; import com.microsoft.semantickernel.services.textcompletion.TextGenerationService; +import java.util.List; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; @WireMockTest public class RenderingTest { @@ -150,6 +154,58 @@ public class RenderingTest { wm.getAllServeEvents().get(0).getRequest().getBodyAsString().contains("{{$ignore}}")); } + @DefineKernelFunction(name = "WithEmptyListReturn") + public List WithEmptyListReturn() { + return List.of(); + } + + @DefineKernelFunction(name = "WithListReturn") + public List WithListReturn() { + return List.of(1, 2, 3); + } + + @DefineKernelFunction(name = "WithListReturn2", returnType = "java.util.List") + public List WithListReturn2() { + return List.of(1, 2, 3); + } + + @DefineKernelFunction(name = "WithListReturn3", returnType = "java.util.List") + public Mono> WithListReturn3() { + return Mono.just(List.of(1, 2, 3)); + } + + @DefineKernelFunction(name = "WithListReturn4", returnType = "java.util.List") + public Mono> WithListReturn4() { + return Mono.just(List.of()); + } + + @Test + public void canHandleIterableReturnFromFunction() { + buildChatKernel() + .invokeAsync( + KernelFunction + .createFromPrompt(""" + {{RenderingTest-WithEmptyListReturn}} + {{RenderingTest-WithListReturn}} + {{RenderingTest-WithListReturn2}} + {{RenderingTest-WithListReturn3}} + {{RenderingTest-WithListReturn4}} + """) + .withTemplateFormat("handlebars") + .build()) + .block(); + + Assertions.assertTrue( + wm.getAllServeEvents().get(0).getRequest().getBodyAsString().equals( + "{\"messages\":[" + + "{\"role\":\"user\",\"content\":\"[]\"}," + + "{\"role\":\"user\",\"content\":\"[1, 2, 3]\"}," + + "{\"role\":\"user\",\"content\":\"[1, 2, 3]\"}," + + "{\"role\":\"user\",\"content\":\"[1, 2, 3]\"}," + + "{\"role\":\"user\",\"content\":\"[]\"}" + + "],\"model\":\"gpt-35-turbo-2\"}")); + } + private Kernel buildTextKernel() { wm.addStubMapping( stubFor( @@ -205,6 +261,7 @@ public class RenderingTest { Kernel kernel = Kernel.builder() .withAIService(ChatCompletionService.class, textGenerationService) + .withPlugin(KernelPluginFactory.createFromObject(this, "RenderingTest")) .build(); return kernel; } diff --git a/java/samples/sample-code/src/main/java/com/microsoft/semantickernel/samples/syntaxexamples/Example09_FunctionTypes.java b/java/samples/sample-code/src/main/java/com/microsoft/semantickernel/samples/syntaxexamples/Example09_FunctionTypes.java index aa28ad42e9..b4ff8ecc0c 100644 --- a/java/samples/sample-code/src/main/java/com/microsoft/semantickernel/samples/syntaxexamples/Example09_FunctionTypes.java +++ b/java/samples/sample-code/src/main/java/com/microsoft/semantickernel/samples/syntaxexamples/Example09_FunctionTypes.java @@ -277,6 +277,27 @@ public class Example09_FunctionTypes { .invokeAsync(plugin.get("AsyncWithBoxedPrimativeReturnType")) .block(); System.out.println(result12.getResult()); + + var result13 = kernel + .invokeAsync(plugin.get("WithEmptyListReturn")) + .block(); + System.out.println(result13.getResult()); + + var result14 = kernel + .invokeAsync(plugin.get("WithListReturn")) + .block(); + System.out.println(result14.getResult()); + + var result15 = kernel + .invokeAsync(plugin.get("WithListReturn2")) + .block(); + System.out.println(result15.getResult()); + + var result16 = kernel + .invokeAsync(plugin.get("WithListReturn3")) + .block(); + System.out.println(result16.getResult()); + /* * TODO: support FunctionResult * kernel @@ -514,6 +535,26 @@ public class Example09_FunctionTypes { return Mono.just(Integer.valueOf(1)); } + @DefineKernelFunction(name = "WithEmptyListReturn") + public List WithEmptyListReturn() { + return List.of(); + } + + @DefineKernelFunction(name = "WithListReturn") + public List WithListReturn() { + return List.of(1, 2, 3); + } + + @DefineKernelFunction(name = "WithListReturn2", returnType = "java.util.List") + public List WithListReturn2() { + return List.of(1, 2, 3); + } + + @DefineKernelFunction(name = "WithListReturn3", returnType = "java.util.List") + public Mono> WithListReturn3() { + return Mono.just(List.of(1, 2, 3)); + } + /* * /// * /// Example using a function to return the result of another inner function diff --git a/java/semantickernel-api/src/main/java/com/microsoft/semantickernel/semanticfunctions/KernelFunctionFromMethod.java b/java/semantickernel-api/src/main/java/com/microsoft/semantickernel/semanticfunctions/KernelFunctionFromMethod.java index 96e012fa05..68dc7684ef 100644 --- a/java/semantickernel-api/src/main/java/com/microsoft/semantickernel/semanticfunctions/KernelFunctionFromMethod.java +++ b/java/semantickernel-api/src/main/java/com/microsoft/semantickernel/semanticfunctions/KernelFunctionFromMethod.java @@ -181,13 +181,12 @@ public class KernelFunctionFromMethod extends KernelFunction { } return mono - .map(it -> { - if (it instanceof Iterable) { - // Handle return from things like Mono> - // from {{function 'input'}} as part of the prompt. - return (T) ((Iterable) it).iterator().next(); - } else { - return (T) it; + .flatMap(it -> { + try { + return Mono.just((T) it); + } catch (ClassCastException e) { + return Mono.error( + new SKException("Return type does not match the expected type", e)); } }) .map(it -> {