Skip to content
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

copilot_chat: Rename o1-preview model to o1 #23038

Merged
merged 11 commits into from
Jan 13, 2025

Conversation

SkywardSyntax
Copy link
Contributor

@SkywardSyntax SkywardSyntax commented Jan 12, 2025

https://github.blog/news-insights/openais-o1-model-available-in-copilot-chat-and-github-models/

Release Notes:

  • Renamed Github Copilot Chat "o1-preview" model to "o1".

Rename the `O1Preview` variant to `O1` in the `Model` enum and update related functions in `crates/copilot/src/copilot_chat.rs`.

* Change the `Model` enum to rename the `O1Preview` variant to `O1`.
* Update the `from_id` function to map the string "o1" to the `O1` variant.
* Update the `id` function to return "o1" for the `O1` variant.
* Update the `display_name` function to return "o1" for the `O1` variant.
* Update the `max_token_count` function to return 20000 for the `O1` variant.
Rename O1Preview variant to O1 in copilot chat
Copy link

cla-bot bot commented Jan 12, 2025

We require contributors to sign our Contributor License Agreement, and we don't have @SkywardSyntax on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@SkywardSyntax
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 12, 2025
Copy link

cla-bot bot commented Jan 12, 2025

The cla-bot has been summoned, and re-checked this pull request!

@maxdeviant maxdeviant changed the title Fixed Copilot Chat "o1-preview" model name Fix Copilot Chat "o1-preview" model name Jan 13, 2025
@maxdeviant maxdeviant self-assigned this Jan 13, 2025
@notpeter
Copy link
Member

@SkywardSyntax Thanks for opening this. One thing I noticed is that in Zed syntax highlighting is incorrect for the O1 enum variant.

Screenshot 2025-01-13 at 9 55 01

This is because of the following query:

; Assume all-caps names are constants
((identifier) @constant
(#match? @constant "^_*[A-Z][A-Z\\d_]*$"))

Not sure if it's worth fixing/working around, but just wanted to mention it.

@maxdeviant maxdeviant changed the title Fix Copilot Chat "o1-preview" model name copilot_chat: Rename o1-preview model to o1 Jan 13, 2025
Copy link
Member

@maxdeviant maxdeviant left a comment

Choose a reason for hiding this comment

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

Thank you!

@maxdeviant maxdeviant enabled auto-merge January 13, 2025 15:22
@maxdeviant maxdeviant added this pull request to the merge queue Jan 13, 2025
Merged via the queue into zed-industries:main with commit 955248f Jan 13, 2025
13 checks passed
@Angelk90
Copy link
Contributor

@notpeter : But it looks like 01Mini is getting caught.
According to the syntax you highlighted it looks like it shouldn't.

@notpeter
Copy link
Member

@notpeter : But it looks like 01Mini is getting caught. According to the syntax you highlighted it looks like it shouldn't.

01Mini isn't getting caught. O1 is.

The regex is: ^_*[A-Z][A-Z\\d_]*$

Which is all capital letters, underscores or digits beginning with at least one letter prefixed by 0 or more underscores. I think we just need to be smarter about tree-sitter contexts so that Enum values are not considered by this @constant regex match.

It seems reasonable to me that with let A1 = "A1", the A1 identifier would be considered a constant but it's definitely overzealous that this regex is also apply to enum variants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants