feat: [Orchestration] Enable local prompt templates#423
Conversation
# Conflicts: # orchestration/src/main/java/com/sap/ai/sdk/orchestration/ConfigToRequestTransformer.java
# Conflicts: # sample-code/spring-app/src/main/resources/static/index.html
| final Object obj; | ||
| try { | ||
| final ObjectMapper yamlReader = new ObjectMapper(new YAMLFactory()); | ||
| obj = yamlReader.readValue(inputYaml, Object.class); | ||
| } catch (JsonProcessingException ex) { | ||
| throw new IOException("Failed to parse the YAML input: " + ex.getMessage(), ex); | ||
| } | ||
| try { | ||
| final ObjectMapper jsonWriter = new ObjectMapper(); | ||
| return fromJson(jsonWriter.writeValueAsString(obj)); | ||
| } catch (JsonProcessingException ex) { | ||
| throw new IOException("Failed to deserialize the input: " + ex.getMessage(), ex); | ||
| } |
There was a problem hiding this comment.
I am not sure if these are the correct/best Exception types. I am open to suggestions :)
There was a problem hiding this comment.
Instead of using two objectmappers, I was about to pitch something like...
private static class TemplateYaml {
@JsonProperty("spec")
private OrchestrationTemplate spec;
}But that would've required additional customization to propagate the modules/rules along for Yaml Factory.
Jonas-Isr
changed the title
| private OrchestrationTemplate fromJson(@Nonnull final String inputString) throws IOException { | ||
| final ObjectMapper objectMapper = | ||
| OrchestrationJacksonConfiguration.getOrchestrationObjectMapper(); | ||
| final JsonNode rootNode = objectMapper.readTree(inputString); | ||
| return objectMapper.treeToValue(rootNode.get("spec"), OrchestrationTemplate.class); | ||
| } |
There was a problem hiding this comment.
This method could principally be made public as an additional feature. I left it private because it is not part of the ticket, not tested on its own, and not aligned with JS SDK (which only has the yaml feature afaik).
| * @since 1.7.0 | ||
| */ | ||
| @Nullable | ||
| public OrchestrationTemplate fromYaml(@Nonnull final String inputYaml) throws IOException { |
There was a problem hiding this comment.
(Question to reviewer)
Should we maybe annotate this with @Beta?
Pro: We might not be able to continue to keep this method/feature alive without additional work if the specs of prompt-registry and orchestration were to diverge at some point (see "Note 1" in the PR description).
Con: Additional beta annotation for convenience code.
There was a problem hiding this comment.
as discussed, since this API has no alternative to users, I would keep it non-Beta
orchestration/src/test/java/com/sap/ai/sdk/orchestration/OrchestrationUnitTest.java
Show resolved
Hide resolved
...le-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/OrchestrationController.java
Outdated
Show resolved
Hide resolved
3 participants
Context
AI/ai-sdk-java-backlog#215
This PR enables users to validate and use prompt templates from their local environment, without the use of prompt registry. For this, the user can provide a String containing a YAML representation of the prompt template and we will deserialise this into our (high level)
OrchestrationTemplateclass, which then can be used to configure an Orchestration call. If the provided prompt template does not match theprompt-registryspec, an exception is thrown.Note 1: We have to assume that the
prompt-registryspec keeps exactly equal to theorchestrationspec for the relevant fields. If the two diverge, we can no longer deserialise the prompt template (which followsprompt-registryspec) into our Java classes (which follow mostly theorchestrationprompt). Since we deserialise into high-level classes, we might have some wiggle room, but that would probably become quite messy.Note 2: The whole process on our side (i.e. validation and deserialisation) ignores the
additionalFieldsof the prompt templates. This is aligned with PO and JS SDK. (The reason is that we cannot know what is inside these fields, and it usually is information we cannot handle in our template classes.)Feature scope:
Definition of Done