feat: [Orchestration] reasoning content #939
Conversation
ZhongpinWang
left a comment
There was a problem hiding this comment.
@CharlesDuboisSAP Regarding the signature, it does not need a convenience to get it out. It only needs to be as part of the message history and be sent back together in the next LLM call for verification reasons. So IMO providing a convenience to access reasoning content text would be enough.
| public List<String> streamReasoning( | ||
| @Nonnull final String topic, | ||
| @Nonnull final Consumer<String> onAnswerChunk, | ||
| @Nonnull final Consumer<String> onReasoningChunk) { |
There was a problem hiding this comment.
Why don't you return two streams in an object, which then can get consumed without an accumulator. It would be simpler and more comparable to existing streaming endpoints
There was a problem hiding this comment.
Why don't you return two streams in an object, which then can get consumed without an accumulator. It would be simpler and more comparable to existing streaming endpoints
I fixed it by returning one stream: Stream where ReasoningChunk is public record ReasoningOutput(@Nonnull String answer, @Nonnull List<String> reasoning) {}.
Accumulator is removed entirely.
Tell me how you feel about it now?
…llers/OrchestrationTest.java Co-authored-by: Jonas-Isr <jonas.israel@sap.com>
…llers/OrchestrationTest.java Co-authored-by: Jonas-Isr <jonas.israel@sap.com>
…llers/OrchestrationTest.java Co-authored-by: Jonas-Isr <jonas.israel@sap.com>
| * @see <a href="https://help.sap.com/docs/sap-ai-core/generative-ai/reasoning">SAP AI Core: | ||
| * Orchestration - Reasoning</a> | ||
| */ | ||
| public class ReasoningAccumulator { |
There was a problem hiding this comment.
(Personal Opinion)
I am not sure if we need this class. I think users should write these themselves.
There was a problem hiding this comment.
PS: JS has accumulator, mainly for aggregating all streaming properties, so that at the end of the streaming process, users can get an object similar to (or I think even same as) the non-streaming response object. But this is just optional :)
There was a problem hiding this comment.
I also added the following alternative solution to the accumulator as described in this somment.
Context
AI/ai-sdk-java-backlog#412.
As a user, I would like to see the thinking processes when using the reasoning models, in addition to the final outcome. This is relevant for synchronous and streaming use-cases.
The harmonized API of orchestration supports this feature (see link above), so the SDK should also follow.
Feature scope:
Definition of Done
Aligned changes with the JavaScript SDK