-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: [Orchestration] Convenience for response format #341
base: main
Are you sure you want to change the base?
Conversation
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationModuleConfig.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationModuleConfig.java
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationModuleConfig.java
Outdated
Show resolved
Hide resolved
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationModuleConfig.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments
sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OrchestrationTest.java
Show resolved
Hide resolved
sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/services/OrchestrationService.java
Show resolved
Hide resolved
} | ||
|
||
@Test | ||
void testResponseFormatOverwrittenByNewTemplateConfigWithResponseFormat() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the overwriting behaviours throw?
It could avoid customers asking why their response format is broken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if the users set a response format in their new template config they will expect it to change, right? So I don't think we should throw here.
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationModuleConfig.java
Outdated
Show resolved
Hide resolved
…strationModuleConfig.java Co-authored-by: Charles Dubois <[email protected]>
Context
AI/ai-sdk-java-backlog#195.
This PR adds convenience methods to set the response format in Orchestration. A JSON schema can be created from a class and then used as a response schema as follows:
If only any JSON response is wanted, without strict adherence to a given schema, the user can use
var configWithJsonResponse = config.withJsonResponse()
.Feature scope:
ResponseJsonSchema
OrchestrationModulConfig.withTemplateConfig()
to make it work convenientlyDefinition of Done
Aligned changes with the JavaScript SDK