generated from SAP/repository-template
-
Notifications
You must be signed in to change notification settings - Fork 15
fix: [Orchestration] spec update #460
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
Merged
newtork
merged 18 commits into
main
from
spec-update/orchestration/fix/streaming-response-type
Jul 9, 2025
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
b36c9d4
Update orchestration based on fix/streaming-response-type
bot-sdk-js eafed16
Merge branch 'main' into spec-update/orchestration/fix/streaming-resp…
CharlesDuboisSAP bd1ddb7
WiP
CharlesDuboisSAP 5cff32a
Merge branch 'refs/heads/main' into spec-update/orchestration/fix/str…
CharlesDuboisSAP 50add6c
Latest spec with error classes
CharlesDuboisSAP 224fbc2
regenerate
CharlesDuboisSAP d4cfe63
Merge branch 'main' into spec-update/orchestration/fix/streaming-resp…
CharlesDuboisSAP 494855e
regenerate
CharlesDuboisSAP e327c1b
DPI
CharlesDuboisSAP e62952d
feat: Add embeddings endpoint and refactor orchestration client (#464)
rpanackal b54ab6e
fix: [Orchestration] Spec update - Filtering, Remove "Synchronous" su…
rpanackal 4d15b91
Make embedding client endpoint non-visible and update javadoc (minor)
rpanackal a8ff4c6
Merge branch 'main' into spec-update/orchestration/fix/streaming-resp…
CharlesDuboisSAP 05ac10f
merge main
CharlesDuboisSAP b58bc5e
Formatting
bot-sdk-js cae1a48
Merge branch 'main' into spec-update/orchestration/fix/streaming-resp…
CharlesDuboisSAP 46413d0
0.81.4
CharlesDuboisSAP 0dc1378
Merge branch 'main' into spec-update/orchestration/fix/streaming-resp…
newtork File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 6 additions & 17 deletions
23
...stration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationChatCompletionDelta.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,45 +1,34 @@ | ||
| package com.sap.ai.sdk.orchestration; | ||
|
|
||
| import com.sap.ai.sdk.core.common.StreamedDelta; | ||
| import com.sap.ai.sdk.orchestration.model.CompletionPostResponse; | ||
| import com.sap.ai.sdk.orchestration.model.LLMModuleResultSynchronous; | ||
| import java.util.Map; | ||
| import com.sap.ai.sdk.orchestration.model.CompletionPostResponseStreaming; | ||
| import javax.annotation.Nonnull; | ||
| import javax.annotation.Nullable; | ||
| import lombok.val; | ||
|
|
||
| /** Orchestration chat completion output delta for streaming. */ | ||
| public class OrchestrationChatCompletionDelta extends CompletionPostResponse | ||
| public class OrchestrationChatCompletionDelta extends CompletionPostResponseStreaming | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accepted breaking change, very minor |
||
| implements StreamedDelta { | ||
|
|
||
| @Nonnull | ||
| @Override | ||
| // will be fixed once the generated code add a discriminator which will allow this class to extend | ||
| // CompletionPostResponseStreaming | ||
| @SuppressWarnings("unchecked") | ||
| public String getDeltaContent() { | ||
| val choices = ((LLMModuleResultSynchronous) getOrchestrationResult()).getChoices(); | ||
| val choices = getOrchestrationResult().getChoices(); | ||
| // Avoid the first delta: "choices":[] | ||
| if (!choices.isEmpty() | ||
| // Multiple choices are spread out on multiple deltas | ||
| // A delta only contains one choice with a variable index | ||
| && choices.get(0).getIndex() == 0) { | ||
|
|
||
| final var message = (Map<String, Object>) choices.get(0).toMap().get("delta"); | ||
| // Avoid the second delta: "choices":[{"delta":{"content":"","role":"assistant"}}] | ||
| if (message != null && message.get("content") != null) { | ||
| return message.get("content").toString(); | ||
| } | ||
| final var message = choices.get(0).getDelta(); | ||
| return message.getContent(); | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public String getFinishReason() { | ||
| return ((LLMModuleResultSynchronous) getOrchestrationResult()) | ||
| .getChoices() | ||
| .get(0) | ||
| .getFinishReason(); | ||
| return getOrchestrationResult().getChoices().get(0).getFinishReason(); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
(Comment)
While I agree to the solution, it doesn't feel great that we're changing public API on convenience layer. That would be considered a breaking change normally. I do understand we only use it as workaround to expose the low-level converter method(s), but still.. 🤔
Uh oh!
There was an error while loading. Please reload this page.
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 would have blocked the generation entirely until they reverted the spec, but I was on vacation and you accepted the breaking change. I'm still in favor of blocking future changes that don't compile.