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

Chat/Commands: Remove LLM reranker for repos without embeddings #1722

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Nov 13, 2023

Part of #1544
Part of #1464

Description

The LLM reranker adds a significant amount of latency to reranking any fetched context. This applies to all commands, chat and edits. The latency is typically between 2-5 seconds.

This PR just disables the reranker for now.

I think this is worth doing as:

  • The added latency from the reranker is not worth the possible benefit right now. It still can sometimes rerank poorly and doesn't provide enough value to justify waiting that long.
  • Context doesn't have a constant value, in many cases context will provide little value (e.g. simple fixups) in some cases, like chat, context would provide a lot more value. We don't have a way of determining that right now, so we're often adding lots of latency for no good reason.

I created an issue here: #1721 to evaluate a different or improved approach. Possible details there on how we could improve this.

Test plan

On a repository without embeddings, run:

  • Chat
  • Commands
  • Edits

Review results and context included.

Copy link
Contributor

@abeatrix abeatrix 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!

@umpox umpox merged commit c777fef into main Nov 15, 2023
15 checks passed
@umpox umpox deleted the tr/skip-reranker branch November 15, 2023 14:02
umpox added a commit that referenced this pull request Nov 16, 2023
## Description

Adds a missing changelog for
#1722

## Test plan

N/A - Docs only change

<!-- Required. See
https://docs.sourcegraph.com/dev/background-information/testing_principles.
-->
@umpox umpox assigned umpox and unassigned umpox Nov 30, 2023
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.

2 participants