-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix: refresh codebase index on config change #6131
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
fix: refresh codebase index on config change #6131
Conversation
👷 Deploy request for continuedev pending review.Visit the deploys page to approve it
|
😱 Found 1 issue. Time to roll up your sleeves! 😱 |
0623705
to
cc82654
Compare
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.
@shssoichiro it would be great to refresh codebase indexing on config change. I think it should only update when the embeddings model or @codebase config changes. Indexing (even just computing potential changes) can be pretty expensive.
Could you add something similar to the DocsService's siteIndexingConfigsAreEqual
that checks if the embeddings model config has changed OR the codebase context provider config has changed?
58c86a5
to
d222d37
Compare
Thanks, I've updated both indexers as requested and fixed the tests which were failing due to caching. |
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.
See comment!
74f07e6
to
111303a
Compare
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.
return (
embedModelsAreEqual(
config1?.selectedModelByRole.embed,
config2.selectedModelByRole.embed,
) && codebaseProvider1 === codebaseProvider2
);
This will always cause a refresh because codebaseProvider1 and codebaseProvider2 will never be equal, because the references will reset on config reload.
What would be a good way to check for this? Looking at the |
@shssoichiro you are right that changes in |
111303a
to
29536db
Compare
Thanks, that makes sense. In that case it probably makes sense to remove that check and just check if the embed provider has changed. I've done that and fixed the merge conflicts. |
5b4da13
to
f4f13db
Compare
a50822c
to
c1f2faa
Compare
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.
@shssoichiro the docConfigsAreEqual
feels redundant since the 3rd param does not do anything. I don't want to nitpick too much but the docs config logic has been touchy and confusing in the past and I want to make sure we arrange it in a way that unused args aren't passed around. Let's rearrange so that the embedModelsAreEqual is only used when needed
c1f2faa
to
634aafb
Compare
🎉 This PR is included in version 1.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Updates the CodebaseIndexer to refresh the index when the assistant config is changed. The DocsService already does this, so this change copies the approach that the DocsService is using.
The biggest current issue is that the codebase indexing will begin before Continue has loaded remote assistant configs, which means it will use whatever model is available locally (usually Transformers.js in VSCode), or worse, silently fail. By refreshing after the config loads, we will ensure we are indexing the codebase using the user's configured embed model.
Checklist
Screenshots
N/A This is a backend change.
Tests
Added appropriate tests to CodebaseIndexer.test.ts