Skip to content

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
merged 18 commits into from
Jul 9, 2025

Conversation

CharlesDuboisSAP
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP commented Jun 4, 2025

Context

AI/ai-sdk-java-backlog#448.

Update to the latest spec from orchestration from this branch

Feature scope:

  • Split the response into synchronous and streaming instead of a oneof
  • DPI
  • Azure
  • Embedding
  • Prompt Shield
  • Remove "Synchronous" suffix

Definition of Done

@CharlesDuboisSAP CharlesDuboisSAP self-assigned this Jun 4, 2025
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accepted breaking change, very minor

…onse-type

# Conflicts:
#	orchestration/src/main/java/com/sap/ai/sdk/orchestration/DpiMasking.java
@rpanackal
Copy link
Member

Should embedding endpoint already be public for the next sdk release?

rpanackal and others added 2 commits July 1, 2025 11:07
…ffix and Embedding property renaming (#469)

* Introduce filtering schema changes and update generated class names

- synchronous suffix removed
- `createConfig` removed from `ContentFilter`
- release notes updated for filtering changes

* Introduce filtering schema changes and update generated class names

- synchronous suffix removed
- `createConfig` removed from `ContentFilter`
- release notes updated for filtering changes

* Release notes and jacoco coverage work around

* Lower min required jacoco coverage complexity and branch rating.

- Release note paraphrasing

* Update e2e for input filters

---------

Co-authored-by: Roshin Rajan Panackal <[email protected]>
@rpanackal
Copy link
Member

rpanackal commented Jul 1, 2025

Should embedding endpoint already be public for the next sdk release?

Ans:

embeddings will only be available in v2 spec. It is not supported in v1 API.

Checkout the discussion here,

Embedding models are left unchanged and client endpoint is package private

CharlesDuboisSAP and others added 4 commits July 2, 2025 13:50
…onse-type

# Conflicts:
#	docs/release_notes.md
#	orchestration/pom.xml
#	orchestration/src/main/java/com/sap/ai/sdk/orchestration/JacksonMixins.java
#	sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OrchestrationTest.java
*/
@Nonnull
FilterConfig createConfig();
InputFilterConfig createInputFilterConfig();
Copy link
Contributor

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

Copy link
Contributor Author

@CharlesDuboisSAP CharlesDuboisSAP Jul 2, 2025

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.

Copy link
Contributor

@newtork newtork left a comment

Choose a reason for hiding this comment

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

Looks good. Do not merge yet.
As of 2025-07-02 version 0.77.3 is active in production.

There's still a spec diff, we have to wait out

image

0.81.4 is in pre-production with almost no changes necessary

image

@rpanackal rpanackal added the please-review Request to review a pull-request label Jul 4, 2025
@newtork
Copy link
Contributor

newtork commented Jul 8, 2025

If you merge the spec from 0.81.4 we could merge and release. Currently that's the version live in production. 👍

Confirmed by Christoph.

@newtork newtork merged commit 9fccd37 into main Jul 9, 2025
7 checks passed
@newtork newtork deleted the spec-update/orchestration/fix/streaming-response-type branch July 9, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please-review Request to review a pull-request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants