Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Integrate Cohere re-ranking API#63877

Merged
rafax merged 7 commits into
mainfrom
rg/cohere_rerank
Jul 17, 2024
Merged

Integrate Cohere re-ranking API#63877
rafax merged 7 commits into
mainfrom
rg/cohere_rerank

Conversation

@rafax

@rafax rafax commented Jul 17, 2024

Copy link
Copy Markdown
Contributor

Integrates Cohere re-ranking API for server-side Cody Context (RFC 969).
Before this PR, we only supported identity ranker (which returned all items in the input order), which is still the default choice (when Cohere API key is not provided).

Closes https://linear.app/sourcegraph/issue/AI-134/add-non-poc-ranking

Test plan

  • tested locally, use
"cody.serverSideContext": {
      "reranker": {
        "type": "cohere",
        "apiKey": "TOKEN"
      }
    }

to test locally

@cla-bot cla-bot Bot added the cla-signed label Jul 17, 2024
@rafax rafax requested a review from a team July 17, 2024 11:45
Comment thread schema/site.schema.json
"reranker" : {
"description": "Reranker to use for rankContext requests",
"type": "object",
"properties": {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make this a bit more futureproof? I'd add a string corresponding to the CodyReranker type here and then use that one so we don't rely on behavior like what's described below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm actually changing this to allow specifying the model too, thank you for quick turnaround on PR review ;)

@rafax rafax Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And, to comment on your specific concern, it'd be ideal to do something like oneOf in Protocol buffers instead of having 2 properties that need to be set in sync - I'll see if that's possible using JSON schema

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@janhartman PTAL now

@rafax rafax requested a review from janhartman July 17, 2024 13:13
@rafax rafax requested a review from a team July 17, 2024 13:40

@janhartman janhartman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

@rafax rafax enabled auto-merge (squash) July 17, 2024 18:57
@rafax rafax merged commit 25929d1 into main Jul 17, 2024
@rafax rafax deleted the rg/cohere_rerank branch July 17, 2024 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants