-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[Theia AI] Support DeepSeek reasoner (and future models with special "system" prompt requirements #14867
Comments
Yeah I guess we could introduce a property as follows instead of the simple flag we had before: developerMessageSettings: {
/** Override the role name of the developer message, defaults to 'developer */
role?: string,
/** Specify how the developer message is included in the request, defaults to 'putAsFirstMessage' */
includeInMessages?: 'putAsFirstMessage' | 'mergeWithFirstUserMessage' | 'skip',
} |
@planger great suggestion, just one question: Whats the difference between "putAsFirstMessage" and leaving out "includeInMessage"? |
I think I just thought when we now start to add different strategies here, we should give a name to what we do now and continue to do by default. |
Supporting arbitrary strings as roles has quite some implications for unknown benefit. Therefore, i went for: enum: ['user', 'system', 'developer', 'mergeWithFirstUserMessage', 'skip'], reasoning is that 'mergeWithFirstUserMessage' and 'skip' do not make sense if the role is also defined. |
the resoner complains if the first two messages are "user" and does not support developerMessages (nor system). This is currently our default if developer messages are not supported.
To avoid similar issues in the future, I would propose to:
This would kind of make the setting "supportsDeveloperMessage" obsolete.
@planger WDYT?
The text was updated successfully, but these errors were encountered: