feat: [Orchestration] Allow for streaming configuration convenience API #561
Conversation
| try (var requestInputStream = fileLoader.apply("groundingRequest.json")) { | ||
| final String request = new String(requestInputStream.readAllBytes()); | ||
| verify( | ||
| postRequestedFor(urlPathEqualTo("/v2/completion")).withRequestBody(equalToJson(request))); | ||
| } | ||
| final String request = fileLoaderStr.apply("groundingRequest.json"); | ||
| verify( | ||
| postRequestedFor(urlPathEqualTo("/v2/completion")).withRequestBody(equalToJson(request))); |
There was a problem hiding this comment.
(Comment)
Drive-by test code change to eliminate redundant code.
There was a problem hiding this comment.
Can you compare this to an implementation that would accept these flags globally, together with other stream-related settings?
Considering that JS already does it that way: https://sap.github.io/ai-sdk/docs/js/orchestration/chat-completion#streaming-options and we probably need to support the other stream options as well eventually 😉
| /** Configuration of optional streaming options for output filtering. */ | ||
| @With(AccessLevel.NONE) | ||
| @Nullable | ||
| FilteringStreamOptions outputFilteringStreamOptions; | ||
|
|
There was a problem hiding this comment.
(Preference)
I like this is set on OrchestrationModuleConfig.
I would go one more step to draft a convenience class to capture all streaming options together. This will be an abstraction over FilteringStreamOptions and GlobalStreamOptions (eg: chunking delimiters and chunk size)
There was a problem hiding this comment.
With your feedback and Matthias', I will consider an extra convenience class.
…filter-streaming-options # Conflicts: # docs/release_notes.md
newtork
changed the title
| ### ✨ New Functionality | ||
|
|
||
| - | ||
| - [Orchestration] For streaming, add convenience configuration AOU for output-filter-overlap, chunk-size, and delimiters via `OrchestrationModuleConfig#withStreamConfig`. |
There was a problem hiding this comment.
What is AOU? Those release notes are not clear.
| /** Configuration of optional streaming options for output filtering. */ | ||
| @With(AccessLevel.PRIVATE) // may be exposed to public in the future | ||
| @Getter(AccessLevel.PACKAGE) | ||
| @Nullable | ||
| GlobalStreamOptions globalStreamOptions; |
There was a problem hiding this comment.
Wrong Javadoc? It is the same as the param above
Context
https://github.com/SAP/ai-sdk-java-backlog/issues/317
https://help.sap.com/docs/sap-ai-core/sap-ai-core-service-guide/streaming
(Stage 1) Allow for low-level class
OrchestrationModuleConfig config; // high-level ContentFilter azureFilter; // high-level ContentFilter llamaFilter; // high-level config .withInputFiltering(azureFilter, llamaFilter) .withOutputFiltering(azureFilter) + .withOutputFilteringStreamOptions(FilteringStreamOptions.create().overlap(1_000)) // low-level argument(Stage 2) Allow for high-level convenience class
OrchestrationModuleConfig config; // high-level ContentFilter azureFilter; // high-level ContentFilter llamaFilter; // high-level config .withInputFiltering(azureFilter, llamaFilter) .withOutputFiltering(azureFilter) + .withStreamConfig(new OrchestrationStreamConfig().withChunkSize(42).withFilterOverlap(1_000))Feature scope:
Definition of Done
Error handling created / updated & covered by the tests aboveAligned changes with the JavaScript SDKDocumentation updated