Skip to content

feat: [Orchestration] reasoning content #939

Open
n-o-u-r-h-a-n wants to merge 19 commits into
mainfrom
reasoning-content
Open

feat: [Orchestration] reasoning content #939
n-o-u-r-h-a-n wants to merge 19 commits into
mainfrom
reasoning-content

Conversation

@n-o-u-r-h-a-n

@n-o-u-r-h-a-n n-o-u-r-h-a-n commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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:

  • supported reasoning content using model classes code
  • added sample code as well as updated controller and index page
  • added e2e and unit tests

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Aligned changes with the JavaScript SDK
  • Documentation updated
  • Release notes updated

Comment thread orchestration/src/main/java/com/sap/ai/sdk/orchestration/AssistantMessage.java Outdated
Comment thread orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java Outdated

@ZhongpinWang ZhongpinWang left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Comment thread orchestration/src/main/java/com/sap/ai/sdk/orchestration/AssistantMessage.java Outdated
Comment thread orchestration/src/main/java/com/sap/ai/sdk/orchestration/AssistantMessage.java Outdated
public List<String> streamReasoning(
@Nonnull final String topic,
@Nonnull final Consumer<String> onAnswerChunk,
@Nonnull final Consumer<String> onReasoningChunk) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@n-o-u-r-h-a-n n-o-u-r-h-a-n Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread orchestration/src/main/java/com/sap/ai/sdk/orchestration/AssistantMessage.java Outdated
Comment thread orchestration/src/test/resources/reasoningRequest.json
n-o-u-r-h-a-n and others added 7 commits July 14, 2026 13:11
…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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Personal Opinion)

I am not sure if we need this class. I think users should write these themselves.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added the following alternative solution to the accumulator as described in this somment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants