-
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
Add support for o1-preview #14356
Add support for o1-preview #14356
Conversation
identifier: 'openai/o1-preview', | ||
}], | ||
'chat', | ||
'codicon codicon-chat', |
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 think codicon-chat
doesn't exist.
ChatAgentLocation.ALL, | ||
['Chat'], | ||
true |
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 think we could just remove those as they are just replicating the defaults of the super class.
this.agentSpecificVariables = []; | ||
this.variables = []; |
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.
Imho it'd be clearer if we just initialize them above (as agentSpecificVariables
is already initialized with an empty array). So we could just remove those lines.
public name = 'O1-Preview'; | ||
public description = 'An agent for interacting with ChatGPT o1-preview'; | ||
public promptTemplates: PromptTemplate[] = []; | ||
public defaultModel = 'o1-preview'; |
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.
This doesn't appear to be used anywhere is it?
@@ -0,0 +1,46 @@ | |||
import { |
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.
There are a few linting errors in this file (no copyright, extra blank lines).
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.
With 6546049 I've suggested a fix for my comments. With them in, this change is good to go imho!
At first it was a bit surprising to see a dedicated chat agent, but since o1 has a different interaction model (no streaming, no system message, maybe in the future more progress reporting), I guess it is justified.
Works great, thank you!
fixed #14329 Signed-off-by: Jonas Helming <[email protected]>
6546049
to
a1ec8ca
Compare
fixed #14329
What it does
Adds a new agent O1-Preview to chat with o1
How to test
Select o1-preview for the O1 agents and ask it questions.
Review checklist
Reminder for reviewers