Skip to content

Conversation

@newtork
Copy link
Contributor

@newtork newtork commented Jan 23, 2025

We've noticed inconsistent handling of system prompt in convenience class OpenAiClient.

If the user defined a base system prompt, then it may be ignored on subsequent calls.
Instead of just logging a warning, we could harden the API contract(?)

@rpanackal
Copy link
Member

I like the solution. Though, I am a bit hesitant on including an additional interface for this.

How are we feeling about removing the withSystemPrompt() feature?

@newtork
Copy link
Contributor Author

newtork commented Jan 24, 2025

How are we feeling about removing the withSystemPrompt() feature?

Not possible due to API stability.
However we could deprecate to discourage future use, and drop it with next major release (in a year?)

@newtork
Copy link
Contributor Author

newtork commented Jan 24, 2025

I realized the model classes generated by our generator allow for easily adding a system message anyway.

https://github.com/SAP/ai-sdk-java/blob/generated-openai/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model2/CreateChatCompletionRequest.java

@newtork newtork closed this Jan 24, 2025
@CharlesDuboisSAP CharlesDuboisSAP deleted the openai/fix-inconsistent-system-prompt branch February 20, 2025 08:59
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