Applied review comments

Author: Jonas-Isr

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model/OpenAiChatCompletionOutput.java

@@ -40,9 +40,6 @@ public class OpenAiChatCompletionOutput extends OpenAiCompletionOutput
    */
   @Nonnull
   public String getContent() throws OpenAiClientException {
-    if (getChoices().isEmpty()) {
-      return "";
-    }
     if ("content_filter".equals(getChoices().get(0).getFinishReason())) {
       throw new OpenAiClientException("Content filter filtered the output.");
     }

orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java

@@ -104,7 +104,7 @@ public OrchestrationResponse chatCompletion(
       throws OrchestrationClientException {
 
     val request = toCompletionPostRequest(prompt, config);
-    return executeRequest(request);
+    return new OrchestrationResponse(executeRequest(request));
   }
 
   /**
@@ -128,7 +128,7 @@ public OrchestrationResponse chatCompletion(
    * @throws OrchestrationClientException If the request fails.
    */
   @Nonnull
-  public OrchestrationResponse executeRequest(@Nonnull final CompletionPostRequest request)
+  public CompletionPostResponse executeRequest(@Nonnull final CompletionPostRequest request)
       throws OrchestrationClientException {
     final BasicClassicHttpRequest postRequest = new HttpPost("/completion");
     try {
@@ -143,13 +143,13 @@ public OrchestrationResponse executeRequest(@Nonnull final CompletionPostRequest
   }
 
   @Nonnull
-  OrchestrationResponse executeRequest(@Nonnull final BasicClassicHttpRequest request) {
+  CompletionPostResponse executeRequest(@Nonnull final BasicClassicHttpRequest request) {
     try {
       val destination = deployment.get().destination();
       log.debug("Using destination {} to connect to orchestration service", destination);
       val client = ApacheHttpClient5Accessor.getHttpClient(destination);
       return client.execute(
-          request, new OrchestrationResponseHandler<>(OrchestrationResponse.class));
+          request, new OrchestrationResponseHandler<>(CompletionPostResponse.class));
     } catch (NoSuchElementException
         | DestinationAccessException
         | DestinationNotFoundException

orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationResponse.java

@@ -1,10 +1,17 @@
 package com.sap.ai.sdk.orchestration;
 
 import com.sap.ai.sdk.orchestration.client.model.CompletionPostResponse;
+import com.sap.ai.sdk.orchestration.client.model.LLMModuleResultSynchronous;
 import javax.annotation.Nonnull;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
 
 /** Orchestration chat completion output. */
-public class OrchestrationResponse extends CompletionPostResponse {
+@RequiredArgsConstructor
+@Getter
+public class OrchestrationResponse {
+  private final CompletionPostResponse data;
+
   /**
    * Get the message content from the output.
    *
@@ -15,12 +22,12 @@ public class OrchestrationResponse extends CompletionPostResponse {
    */
   @Nonnull
   public String getContent() throws OrchestrationClientException {
-    if (getOrchestrationResult().getChoices().isEmpty()) {
-      return "";
-    }
-    if ("content_filter".equals(getOrchestrationResult().getChoices().get(0).getFinishReason())) {
+    final var choice =
+        ((LLMModuleResultSynchronous) data.getOrchestrationResult()).getChoices().get(0);
+
+    if ("content_filter".equals(choice.getFinishReason())) {
       throw new OrchestrationClientException("Content filter filtered the output.");
     }
-    return getOrchestrationResult().getChoices().get(0).getMessage().getContent();
+    return choice.getMessage().getContent();
   }
 }

orchestration/src/test/java/com/sap/ai/sdk/orchestration/OrchestrationUnitTest.java

@@ -140,11 +140,12 @@ void testTemplating() throws IOException {
     final var result =
         client.chatCompletion(new OrchestrationPrompt(inputParams, template), config);
 
-    assertThat(result.getRequestId()).isEqualTo("26ea36b5-c196-4806-a9a6-a686f0c6ad91");
-    assertThat(result.getModuleResults().getTemplating().get(0).getContent())
+    final var response = result.getData();
+    assertThat(response.getRequestId()).isEqualTo("26ea36b5-c196-4806-a9a6-a686f0c6ad91");
+    assertThat(response.getModuleResults().getTemplating().get(0).getContent())
         .isEqualTo("Reply with 'Orchestration Service is working!' in German");
-    assertThat(result.getModuleResults().getTemplating().get(0).getRole()).isEqualTo("user");
-    var llm = (LLMModuleResultSynchronous) result.getModuleResults().getLlm();
+    assertThat(response.getModuleResults().getTemplating().get(0).getRole()).isEqualTo("user");
+    var llm = (LLMModuleResultSynchronous) response.getModuleResults().getLlm();
     assertThat(llm.getId()).isEqualTo("chatcmpl-9lzPV4kLrXjFckOp2yY454wksWBoj");
     assertThat(llm.getObject()).isEqualTo("chat.completion");
     assertThat(llm.getCreated()).isEqualTo(1721224505);
@@ -159,7 +160,7 @@ void testTemplating() throws IOException {
     assertThat(usage.getCompletionTokens()).isEqualTo(7);
     assertThat(usage.getPromptTokens()).isEqualTo(19);
     assertThat(usage.getTotalTokens()).isEqualTo(26);
-    var orchestrationResult = (LLMModuleResultSynchronous) result.getOrchestrationResult();
+    var orchestrationResult = (LLMModuleResultSynchronous) response.getOrchestrationResult();
     assertThat(orchestrationResult.getId()).isEqualTo("chatcmpl-9lzPV4kLrXjFckOp2yY454wksWBoj");
     assertThat(orchestrationResult.getObject()).isEqualTo("chat.completion");
     assertThat(orchestrationResult.getCreated()).isEqualTo(1721224505);
@@ -285,7 +286,7 @@ void messagesHistory() throws IOException {
 
     final var result = client.chatCompletion(prompt, config);
 
-    assertThat(result.getRequestId()).isEqualTo("26ea36b5-c196-4806-a9a6-a686f0c6ad91");
+    assertThat(result.getData().getRequestId()).isEqualTo("26ea36b5-c196-4806-a9a6-a686f0c6ad91");
 
     // verify that the history is sent correctly
     try (var requestInputStream = fileLoader.apply("messagesHistoryRequest.json")) {
@@ -309,13 +310,13 @@ void maskingAnonymization() throws IOException {
         createMaskingConfig(DPIConfig.MethodEnum.ANONYMIZATION, DPIEntities.PHONE);
 
     final var result = client.chatCompletion(prompt, config.withMaskingConfig(maskingConfig));
+    final var response = result.getData();
 
-    assertThat(result).isNotNull();
-    GenericModuleResult inputMasking = result.getModuleResults().getInputMasking();
+    assertThat(response).isNotNull();
+    GenericModuleResult inputMasking = response.getModuleResults().getInputMasking();
     assertThat(inputMasking.getMessage()).isEqualTo("Input to LLM is masked successfully.");
     assertThat(inputMasking.getData()).isNotNull();
-    final var choices = ((LLMModuleResultSynchronous) result.getOrchestrationResult()).getChoices();
-    assertThat(choices.get(0).getMessage().getContent())
+    assertThat(result.getContent())
         .isEqualTo(
             "I'm sorry, I cannot provide information about specific individuals, including their nationality.");
 

pom.xml

@@ -74,7 +74,7 @@
     <enforcer.skipBanGeneratedModulesReference>false</enforcer.skipBanGeneratedModulesReference>
     <!-- Test coverage -->
     <coverage.instruction>74%</coverage.instruction>
-    <coverage.branch>66%</coverage.branch>
+    <coverage.branch>67%</coverage.branch>
     <coverage.complexity>67%</coverage.complexity>
     <coverage.line>75%</coverage.line>
     <coverage.method>80%</coverage.method>

sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OrchestrationTest.java

@@ -32,7 +32,8 @@ void testCompletion() {
 
   @Test
   void testTemplate() {
-    final var result = controller.template();
+    final var response = controller.template();
+    final var result = response.getData();
 
     assertThat(result.getRequestId()).isNotEmpty();
     assertThat(result.getModuleResults().getTemplating().get(0).getContent())
@@ -58,20 +59,21 @@ void testTemplate() {
     assertThat(orchestrationResult.getCreated()).isGreaterThan(1);
     assertThat(orchestrationResult.getModel())
         .isEqualTo(OrchestrationController.LLM_CONFIG.getModelName());
-    choices = ((LLMModuleResultSynchronous) orchestrationResult).getChoices();
+    choices = orchestrationResult.getChoices();
     assertThat(choices.get(0).getIndex()).isZero();
     assertThat(choices.get(0).getMessage().getContent()).isNotEmpty();
     assertThat(choices.get(0).getMessage().getRole()).isEqualTo("assistant");
     assertThat(choices.get(0).getFinishReason()).isEqualTo("stop");
-    usage = ((LLMModuleResultSynchronous) orchestrationResult).getUsage();
+    usage = orchestrationResult.getUsage();
     assertThat(usage.getCompletionTokens()).isGreaterThan(1);
     assertThat(usage.getPromptTokens()).isGreaterThan(1);
     assertThat(usage.getTotalTokens()).isGreaterThan(1);
   }
 
   @Test
   void testLenientContentFilter() {
-    var result = controller.filter(AzureThreshold.NUMBER_4);
+    var response = controller.filter(AzureThreshold.NUMBER_4);
+    var result = response.getData();
     var llmChoice =
         ((LLMModuleResultSynchronous) result.getOrchestrationResult()).getChoices().get(0);
     assertThat(llmChoice.getFinishReason()).isEqualTo("stop");
@@ -91,15 +93,16 @@ void testStrictContentFilter() {
 
   @Test
   void testMessagesHistory() {
-    CompletionPostResponse result = controller.messagesHistory();
+    CompletionPostResponse result = controller.messagesHistory().getData();
     final var choices = ((LLMModuleResultSynchronous) result.getOrchestrationResult()).getChoices();
     assertThat(choices.get(0).getMessage().getContent()).isNotEmpty();
   }
 
   @SuppressWarnings("unchecked")
   @Test
   void testMaskingAnonymization() {
-    var result = controller.maskingAnonymization();
+    var response = controller.maskingAnonymization();
+    var result = response.getData();
     var llmChoice =
         ((LLMModuleResultSynchronous) result.getOrchestrationResult()).getChoices().get(0);
     assertThat(llmChoice.getFinishReason()).isEqualTo("stop");
@@ -118,7 +121,8 @@ void testMaskingAnonymization() {
   @SuppressWarnings("unchecked")
   @Test
   void testMaskingPseudonymization() {
-    var result = controller.maskingPseudonymization();
+    var response = controller.maskingPseudonymization();
+    var result = response.getData();
     var llmChoice =
         ((LLMModuleResultSynchronous) result.getOrchestrationResult()).getChoices().get(0);
     assertThat(llmChoice.getFinishReason()).isEqualTo("stop");