Prepare draft

Author: Jonas-Isr

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

@@ -50,7 +50,6 @@ static TemplatingModuleConfig toTemplateModuleConfig(
      */
     val messages = template instanceof Template t ? t.getTemplate() : List.<ChatMessagesInner>of();
     val messagesWithPrompt = new ArrayList<>(messages);
-//    TODO: Here createChatMessage is used to convert Message to ChatMessage
     messagesWithPrompt.addAll(
         prompt.getMessages().stream().map(Message::createChatMessage).toList());
     if (messagesWithPrompt.isEmpty()) {

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

@@ -89,9 +89,6 @@ default ChatMessagesInner createChatMessage() {
     }
   }
 
-  //  TODO: ask in PR:
-  //    Is there a better way to distribute the logic of the `add...` methods between the Message
-  //    interface and its implementing classes?
   static Message addTextMessages(
       @Nullable MessageContent oldContent, @Nonnull String role, @Nonnull String... messages) {
     if (oldContent == null && messages.length == 1) {

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

@@ -2,9 +2,6 @@
 
 public sealed interface MessageContent permits MessageContentSingle, MessageContentMulti {
 
-  //  TODO: Ask in PR: This is to turn the message content into a string.
-  //      Is there a more elegant way/place to do this? Also, I put a semicolon between multiple
-  //       messages. Is that okay? Or just a space?
   static String toString(MessageContent content) {
     if (content instanceof MessageContentSingle mCSingle) {
       return mCSingle.content();

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

@@ -11,9 +11,6 @@ public record MessageContentMulti(List<MultiMessageContent> multiContentList)
     implements MessageContent {
   public MessageContentMulti {}
 
-  // TODO: Note in PR:
-  //  I use MultiChatMessageContent... here since using List<MultiChatMessageContent> results in a
-  //  compilation error (constructors have same erasure). Is there a more elegant way?
   public MessageContentMulti(MultiChatMessageContent... multiChatContents) {
     this(convertIntoMultiMessageList(multiChatContents));
   }

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

@@ -1,5 +1,6 @@
 package com.sap.ai.sdk.orchestration;
 
-public sealed interface MultiMessageContent permits MultiMessageTextContent, MultiMessageImageContent{
+public sealed interface MultiMessageContent
+    permits MultiMessageTextContent, MultiMessageImageContent {
   public String type();
 }

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

@@ -1,6 +1,7 @@
 package com.sap.ai.sdk.orchestration;
 
-public record MultiMessageImageContent(String imageUrl, DetailLevel detailLevel) implements MultiMessageContent {
+public record MultiMessageImageContent(String imageUrl, DetailLevel detailLevel)
+    implements MultiMessageContent {
   private static final String type = "image_url";
 
   public String type() {
@@ -21,4 +22,3 @@ public static DetailLevel fromString(String str) {
     }
   }
 }
-

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

@@ -16,18 +16,12 @@ public class UserMessage implements Message {
   /** The role of the assistant. */
   @Nonnull String role = "user";
 
-  //  TODO: mention in PR:
-  //   Deleted non-null annotation from content to allow for dynamic creation using a no-args
-  //   constructor.
   /** The content of the message. */
   MessageContent content;
 
   @Nonnull
   @Override
   public String content() {
-    //    TODO: Ask in PR:
-    //     Maybe we want this to be called "contentAsString()" and have an actual getter instead?
-    //     Right now, this version avoids a breaking change.
     return MessageContent.toString(content);
   }
 
@@ -37,11 +31,6 @@ public MessageContent getContent() {
     return content != null ? content : new MessageContentSingle("");
   }
 
-  //  TODO: mention in PR
-  //   I added these constructors for backwards compatibility and convenience.
-  //   To enable dynamic message building, I added the addTextMessages and addImage methods.
-  //   Instead, we could use a builder pattern, but I did not know how to make that work without
-  //   breaking changes to the constructors.
   public UserMessage(String singleMessage) {
     content = new MessageContentSingle(singleMessage);
   }
@@ -51,55 +40,19 @@ public UserMessage(MessageContent messageContent) {
   }
 
   public UserMessage(List<MultiChatMessageContent> multiChatMessageContentList) {
-    //    List<MultiMessageContent> newContentList = new java.util.ArrayList<>(List.of());
-    //    for (var chatMessageContent : multiChatMessageContentList ) {
-    //      if (chatMessageContent instanceof TextContent textContent) {
-    //        newContentList.add(new MultiMessageTextContent(textContent.getText()));
-    //      } else if (chatMessageContent instanceof ImageContent imageContent) {
-    //        var imageUrl = imageContent.getImageUrl();
-    //        newContentList.add(new MultiMessageImageContent(
-    //                imageUrl.getUrl(),
-    // MultiMessageImageContent.DetailLevel.fromString(imageUrl.getDetail())));
-    //      } else {
-    //        throw new NotImplementedException("Unknown subtype of MultiChatMessageContent: " +
-    // chatMessageContent.getClass());
-    //      }
-    //    }
-    //    content = new MessageContentMulti(newContentList);
     content =
         new MessageContentMulti(
             multiChatMessageContentList.toArray(MultiChatMessageContent[]::new));
   }
 
   @Nonnull
   public UserMessage addTextMessages(@Nonnull String... messages) {
-    //    if (content == null && messages.length == 1) {
-    //      //      this.content will be of type MessageContentSingle only here
-    //      return new UserMessage(messages[0]);
-    //    }
-    //    var multiContentList = new ArrayList<MultiMessageContent>();
-    //    if (content instanceof MessageContentSingle mCSingle) {
-    //      multiContentList.add(new MultiMessageTextContent(mCSingle.content()));
-    //    } else {
-    //      multiContentList.addAll(((MessageContentMulti) content).multiContentList());
-    //    }
-    //
-    // multiContentList.addAll((Arrays.stream(messages).map(MultiMessageTextContent::new)).toList());
-    //    return new UserMessage(new MessageContentMulti(multiContentList));
     return ((UserMessage) Message.addTextMessages(content, role, messages));
   }
 
   @Nonnull
   public UserMessage addImage(
       @Nonnull String imageUrl, MultiMessageImageContent.DetailLevel detailLevel) {
-    //    var multiContentList = new ArrayList<MultiMessageContent>();
-    //    if (content instanceof MessageContentSingle mCSingle) {
-    //      multiContentList.add(new MultiMessageTextContent(mCSingle.content()));
-    //    } else {
-    //      multiContentList.addAll(((MessageContentMulti) content).multiContentList());
-    //    }
-    //    multiContentList.add(new MultiMessageImageContent(imageUrl, detailLevel));
-    //    return new UserMessage(new MessageContentMulti(multiContentList));
     return ((UserMessage) Message.addImage(content, role, imageUrl, detailLevel));
   }
 }

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

@@ -825,7 +825,9 @@ void testOrchestrationImageSupport() {
     }
     assertThat(strBuilder.toString())
         .isEqualTo(
-            "[user] Can you solve this captcha? Please help me prove my humanity!; https://sample.sap.com/image\n"
-                + "[assistant] Of course! Just let me put on my human glasses... Oh wait, I left them in the matrix\n");
+            """
+                [user] Can you solve this captcha? Please help me prove my humanity!; https://sample.sap.com/image
+                [assistant] Of course! Just let me put on my human glasses... Oh wait, I left them in the matrix
+                """);
   }
 }