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

Adding completions to google vertex provider only for anthropic based models #4606

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

arafatkatze
Copy link
Contributor

@arafatkatze arafatkatze commented Jun 18, 2024

Linear Issue

This PR adds support for completions support for Google Vertex but only for Anthropic based models(NOTE: Gemini and other models don't support completions when used with the google provider).

Sense of Urgency: This is a P0 because of enterprise requirements so I would appreciate a fast approval and merging.

Test plan

@@ -142,6 +142,16 @@ export async function createProviderConfig(
? authStatus.configOverwrites.completionModel
: undefined,
})
case 'google':
if (authStatus.configOverwrites.completionModel?.includes('claude')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only works for claude models using the messages api

@@ -142,6 +142,15 @@ export async function createProviderConfig(
? authStatus.configOverwrites.completionModel
: undefined,
})
case 'google':
if (authStatus.configOverwrites.completionModel?.includes('claude')) {
return createAnthropicProviderConfig({
Copy link
Member

Choose a reason for hiding this comment

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

My initial impression is that we would need a new Google provider here, and that simply reusing the Anthropic one like this might be risky/weird - but I'm not sure. What thoughts have you given to this tradeoff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So basically I do think that you are right in the sense that we should have a different provider for Google but the logic and code and the messages api for anthropic work very very similarly in Bedrock+ Anthropic and vertex. Its not EXACTLY the same but its very similar. IF you go above in the same code you would see that bedrock also adopts the same thing.

NOTE: the google provider works by making a different API for every provider as we discussed before and it doesn't have the concept of consistency so we would down the line have different APIs anyway for different providers/models and that's why I added an if condition which basically says that use anthropic only if you have claude model and return null for other things(which was the previous behaviour.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image See this

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm am i overlooked something?

authStatus.configOverwrites.provider === 'sourcegraph' should never be true inside the cases where authStatus.configOverwrites.provider === 'anthropic' or authStatus.configOverwrites.provider === 'aws-bedrock' right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could have the cody gateway as the provider and then you overwrite the completion model for that case for the other cases where you define the completion model in the siteadmin config we leave model as an undefined value so that it can be "pulled" from the siteadmin config.

I just made that change.

@arafatkatze arafatkatze merged commit b5cff16 into main Jun 20, 2024
20 checks passed
@arafatkatze arafatkatze deleted the google-vertex-anthropic branch June 20, 2024 10:09
slimsag added a commit to sourcegraph/sourcegraph-public-snapshot that referenced this pull request Jun 20, 2024
…Google vertex (#63282)

[Linear
Issue](https://linear.app/sourcegraph/project/claude-3-on-gcp-8c014e1a3506/overview)

This PR adds support for anthropic models in the google provider through
google vertex.
NOTE: The current code only supported Google Gemini API and had boiler
plate code for Google vertex(only for the gemini model) this PR adds
Google Vertex for anthropic models properly so this way the google
provider can be run in 3 different configurations
1. Google Gemini API(this works but only for chat and not for
completions which is the intended behaviour for now)
2. Google Vertex API Anthropic Model(This works perfectly and is added
in this PR and tested for both chat and completions and it works great)
3. Google Vertex API Gemini Model (this doesn't work yet and can
eventually be added to this codebase but we gotta add a new decoder for
the streaming responses of the gemini model through this API we can take
care of this later)

Sense of Urgency: This is a P0 because of enterprise requirements so I
would appreciate a fast approval and merging.

<!-- 💡 To write a useful PR description, make sure that your description
covers:
- WHAT this PR is changing:
    - How was it PREVIOUSLY.
    - How it will be from NOW on.
- WHY this PR is needed.
- CONTEXT, i.e. to which initiative, project or RFC it belongs.

The structure of the description doesn't matter as much as covering
these points, so use
your best judgement based on your context.
Learn how to write good pull request description:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e?pvs=4
-->


## Test plan
- Run this branch for Cody instance ->
sourcegraph/cody#4606
- Ask @arafatkatze to dm you the siteadmin config to make things work
- Check the logs and play with completions and chat

<!-- All pull requests REQUIRE a test plan:
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->


## Changelog

<!--
1. Ensure your pull request title is formatted as: $type($domain): $what
3. Add bullet list items for each additional detail you want to cover
(see example below)
4. You can edit this after the pull request was merged, as long as
release shipping it hasn't been promoted to the public.
5. For more information, please see this how-to
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c?

Audience: TS/CSE > Customers > Teammates (in that order).

Cheat sheet: $type = chore|fix|feat $domain:
source|search|ci|release|plg|cody|local|...
-->

<!--
Example:

Title: fix(search): parse quotes with the appropriate context
Changelog section:

## Changelog

- When a quote is used with regexp pattern type, then ...
- Refactored underlying code.
-->

---------

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Co-authored-by: Beatrix <beatrix@sourcegraph.com>
Co-authored-by: Stephen Gutekanst <stephen@sourcegraph.com>
Chickensoupwithrice pushed a commit to sourcegraph/sourcegraph-public-snapshot that referenced this pull request Jun 20, 2024
…Google vertex (#63282)

[Linear
Issue](https://linear.app/sourcegraph/project/claude-3-on-gcp-8c014e1a3506/overview)

This PR adds support for anthropic models in the google provider through
google vertex.
NOTE: The current code only supported Google Gemini API and had boiler
plate code for Google vertex(only for the gemini model) this PR adds
Google Vertex for anthropic models properly so this way the google
provider can be run in 3 different configurations
1. Google Gemini API(this works but only for chat and not for
completions which is the intended behaviour for now)
2. Google Vertex API Anthropic Model(This works perfectly and is added
in this PR and tested for both chat and completions and it works great)
3. Google Vertex API Gemini Model (this doesn't work yet and can
eventually be added to this codebase but we gotta add a new decoder for
the streaming responses of the gemini model through this API we can take
care of this later)

Sense of Urgency: This is a P0 because of enterprise requirements so I
would appreciate a fast approval and merging.

<!-- 💡 To write a useful PR description, make sure that your description
covers:
- WHAT this PR is changing:
    - How was it PREVIOUSLY.
    - How it will be from NOW on.
- WHY this PR is needed.
- CONTEXT, i.e. to which initiative, project or RFC it belongs.

The structure of the description doesn't matter as much as covering
these points, so use
your best judgement based on your context.
Learn how to write good pull request description:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e?pvs=4
-->

- Run this branch for Cody instance ->
sourcegraph/cody#4606
- Ask @arafatkatze to dm you the siteadmin config to make things work
- Check the logs and play with completions and chat

<!-- All pull requests REQUIRE a test plan:
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

<!--
1. Ensure your pull request title is formatted as: $type($domain): $what
3. Add bullet list items for each additional detail you want to cover
(see example below)
4. You can edit this after the pull request was merged, as long as
release shipping it hasn't been promoted to the public.
5. For more information, please see this how-to
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c?

Audience: TS/CSE > Customers > Teammates (in that order).

Cheat sheet: $type = chore|fix|feat $domain:
source|search|ci|release|plg|cody|local|...
-->

<!--
Example:

Title: fix(search): parse quotes with the appropriate context
Changelog section:

- When a quote is used with regexp pattern type, then ...
- Refactored underlying code.
-->

---------

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Co-authored-by: Beatrix <beatrix@sourcegraph.com>
Co-authored-by: Stephen Gutekanst <stephen@sourcegraph.com>
(cherry picked from commit 1a6a7f7)
Chickensoupwithrice pushed a commit to sourcegraph/sourcegraph-public-snapshot that referenced this pull request Jun 20, 2024
…Google vertex (#63282)

[Linear
Issue](https://linear.app/sourcegraph/project/claude-3-on-gcp-8c014e1a3506/overview)

This PR adds support for anthropic models in the google provider through
google vertex.
NOTE: The current code only supported Google Gemini API and had boiler
plate code for Google vertex(only for the gemini model) this PR adds
Google Vertex for anthropic models properly so this way the google
provider can be run in 3 different configurations
1. Google Gemini API(this works but only for chat and not for
completions which is the intended behaviour for now)
2. Google Vertex API Anthropic Model(This works perfectly and is added
in this PR and tested for both chat and completions and it works great)
3. Google Vertex API Gemini Model (this doesn't work yet and can
eventually be added to this codebase but we gotta add a new decoder for
the streaming responses of the gemini model through this API we can take
care of this later)

Sense of Urgency: This is a P0 because of enterprise requirements so I
would appreciate a fast approval and merging.

<!-- 💡 To write a useful PR description, make sure that your description
covers:
- WHAT this PR is changing:
    - How was it PREVIOUSLY.
    - How it will be from NOW on.
- WHY this PR is needed.
- CONTEXT, i.e. to which initiative, project or RFC it belongs.

The structure of the description doesn't matter as much as covering
these points, so use
your best judgement based on your context.
Learn how to write good pull request description:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e?pvs=4
-->

- Run this branch for Cody instance ->
sourcegraph/cody#4606
- Ask @arafatkatze to dm you the siteadmin config to make things work
- Check the logs and play with completions and chat

<!-- All pull requests REQUIRE a test plan:
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

<!--
1. Ensure your pull request title is formatted as: $type($domain): $what
3. Add bullet list items for each additional detail you want to cover
(see example below)
4. You can edit this after the pull request was merged, as long as
release shipping it hasn't been promoted to the public.
5. For more information, please see this how-to
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c?

Audience: TS/CSE > Customers > Teammates (in that order).

Cheat sheet: $type = chore|fix|feat $domain:
source|search|ci|release|plg|cody|local|...
-->

<!--
Example:

Title: fix(search): parse quotes with the appropriate context
Changelog section:

- When a quote is used with regexp pattern type, then ...
- Refactored underlying code.
-->

---------

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Co-authored-by: Beatrix <beatrix@sourcegraph.com>
Co-authored-by: Stephen Gutekanst <stephen@sourcegraph.com>
(cherry picked from commit 1a6a7f7)
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.

3 participants