Minor comments

Author: MatKuhr

docs/adrs/003-code-generation.md

@@ -78,8 +78,16 @@ For the **foundation models** we choose alternative **2**, generating about 80-9
 
 For the **orchestration service** we choose a mix of alternatives **2** and **3**.
 The main reason is that the desired API quality and stability can not be achieved with generated code alone.
+
 - We generate all DTO classes but mark them as beta and leave them in a dedicated `.dto` package.
 - We offer a hand-crafted convenience API on top that uses the DTO objects internally.
 - The convenience API is considered stable and is tested in full
 - The convenience API has the best API quality, but may not cover all edge cases and not allow for arbitrary customization
 - Users may transition to the low-level API, directly working with the DTOs, if the convenience API isn't flexible enough for their use case
+
+Key benefits:
+- We can guarantee API stability and quality for the orchestration service
+- Allows for additional convenience & clarity (e.g. azure content filter levels, flattened out API, smart defaults etc.) 
+- Clear separation of concerns: User-facing API vs. POJOs for JSON (de-) serialization
+- Avoid feature gaps in the generator impacting end users
+- 

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

@@ -23,7 +23,7 @@ public enum Sensitivity {
     HIGH(0),
     MEDIUM(2),
     LOW(4);
-
+    // note: we leave out the value 6, as setting it is equivalent to not setting the filter at all
     private final int value;
   }
 

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

@@ -20,12 +20,15 @@
 import lombok.NoArgsConstructor;
 import lombok.val;
 
+/**
+ * Factory to create all DTOs from an orchestration configuration.
+ */
 @NoArgsConstructor(access = AccessLevel.NONE)
 final class ModuleConfigFactory {
   @Nonnull
   static ModuleConfigs toModuleConfigDTO(
       @Nonnull final OrchestrationConfig<?> config, @Nonnull final List<Message> messages) {
-    val llm =
+    val llmDto =
         config
             .getLlmConfig()
             .map(ModuleConfigFactory::toLlmModuleConfigDTO)
@@ -34,15 +37,15 @@ static ModuleConfigs toModuleConfigDTO(
     val template = config.getTemplate().getOrElse(() -> TemplateConfig.fromMessages(List.of()));
     val templateDto = toTemplateModuleConfigDTO(template, messages);
 
-    var dto = ModuleConfigs.create().llmModuleConfig(llm).templatingModuleConfig(templateDto);
+    var resultDto = ModuleConfigs.create().llmModuleConfig(llmDto).templatingModuleConfig(templateDto);
 
     config
         .getMaskingConfig()
         .filter(DpiMaskingConfig.class::isInstance)
         .map(DpiMaskingConfig.class::cast)
         .map(DpiMaskingConfig::toMaskingProviderDTO)
         .map(it -> MaskingModuleConfig.create().maskingProviders(it))
-        .forEach(dto::maskingModuleConfig);
+        .forEach(resultDto::maskingModuleConfig);
 
     val maybeInputFilter = config.getInputContentFilter();
     val maybeOutputFilter = config.getOutputContentFilter();
@@ -61,22 +64,22 @@ static ModuleConfigs toModuleConfigDTO(
           .map(AzureContentFilter::toFilterConfigDTO)
           .map(it -> OutputFilteringConfig.create().filters(it))
           .forEach(filter::output);
-      dto = dto.filteringModuleConfig(filter);
+      resultDto = resultDto.filteringModuleConfig(filter);
     }
 
-    return dto;
+    return resultDto;
   }
 
   @Nonnull
   static LLMModuleConfig toLlmModuleConfigDTO(@Nonnull final AiModel model) {
     if (model instanceof LlmConfig llmConfig) {
       return llmConfig.toLLMModuleConfigDTO();
     }
-    val result = LLMModuleConfig.create().modelName(model.name()).modelParams(Map.of());
+    val dto = LLMModuleConfig.create().modelName(model.name()).modelParams(Map.of());
     if (model.version() != null) {
-      result.modelVersion(model.version());
+      dto.modelVersion(model.version());
     }
-    return result;
+    return dto;
   }
 
   @Nonnull

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

@@ -21,6 +21,7 @@
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import lombok.AllArgsConstructor;
+import lombok.RequiredArgsConstructor;
 import lombok.experimental.Delegate;
 import lombok.val;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
@@ -29,7 +30,7 @@
 import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
 import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 
-@AllArgsConstructor
+@RequiredArgsConstructor
 public class OrchestrationClient implements OrchestrationConfig<OrchestrationClient> {
   static final ObjectMapper JACKSON;
 

sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/OrchestrationController.java

@@ -1,5 +1,6 @@
 package com.sap.ai.sdk.app.controllers;
 
+import com.sap.ai.sdk.foundationmodels.openai.OpenAiModel;
 import com.sap.ai.sdk.orchestration.AzureContentFilter;
 import com.sap.ai.sdk.orchestration.AzureContentFilter.Sensitivity;
 import com.sap.ai.sdk.orchestration.DpiMaskingConfig;
@@ -23,10 +24,9 @@
 @RestController
 @RequestMapping("/orchestration")
 class OrchestrationController {
-  static final String MODEL = "gpt-35-turbo";
 
   private final OrchestrationClient client =
-      new OrchestrationClient().withLlmConfig(new LlmConfig(MODEL));
+      new OrchestrationClient().withLlmConfig(OpenAiModel.GPT_35_TURBO);
 
   /**
    * Chat request to OpenAI through the Orchestration service with a template

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

@@ -4,6 +4,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import com.sap.ai.sdk.foundationmodels.openai.OpenAiModel;
 import com.sap.ai.sdk.orchestration.AzureContentFilter;
 import com.sap.ai.sdk.orchestration.OrchestrationClientException;
 import com.sap.ai.sdk.orchestration.OrchestrationResponse;
@@ -39,7 +40,7 @@ void testCompletion() {
     assertThat(llm.getId()).isNotEmpty();
     assertThat(llm.getObject()).isEqualTo("chat.completion");
     assertThat(llm.getCreated()).isGreaterThan(1);
-    assertThat(llm.getModel()).isEqualTo(OrchestrationController.MODEL);
+    assertThat(llm.getModel()).isEqualTo(OpenAiModel.GPT_35_TURBO.name());
     var choices = llm.getChoices();
     assertThat(choices.get(0).getIndex()).isZero();
     assertThat(choices.get(0).getMessage().getContent()).isNotEmpty();
@@ -52,7 +53,7 @@ void testCompletion() {
     assertThat(responseDto.getOrchestrationResult().getObject()).isEqualTo("chat.completion");
     assertThat(responseDto.getOrchestrationResult().getCreated()).isGreaterThan(1);
     assertThat(responseDto.getOrchestrationResult().getModel())
-        .isEqualTo(OrchestrationController.MODEL);
+        .isEqualTo(OpenAiModel.GPT_35_TURBO.name());
     choices = responseDto.getOrchestrationResult().getChoices();
     assertThat(choices.get(0).getIndex()).isZero();
     assertThat(choices.get(0).getMessage().getContent()).isNotEmpty();