-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update fetch tool to use metis #1192
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR updates the GitHub repository tool to use the "metis" service by refactoring the GithubAvailableEmbeddingTypesManager
into a proper dependency injection service pattern. The change introduces a service interface and registers it with the DI container to improve code organization and testability.
Key changes:
- Converts
GithubAvailableEmbeddingTypesManager
to a proper service with interface - Updates all consumers to use dependency injection instead of manual instantiation
- Adds proper service registration in both production and test environments
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/platform/workspaceChunkSearch/common/githubAvailableEmbeddingTypes.ts | Creates service interface and renames class to follow service pattern |
src/platform/workspaceChunkSearch/node/workspaceChunkSearchService.ts | Updates to use injected service instead of manual instantiation |
src/platform/urlChunkSearch/node/urlChunkEmbeddingsIndex.ts | Adds service injection and updates embedding computation calls |
src/platform/embeddings/common/embeddingsComputer.ts | Extracts EmbeddingInputType to a named type |
src/extension/tools/node/githubRepoTool.tsx | Updates to use injected service instead of lazy instantiation |
src/extension/extension/vscode-node/services.ts | Registers the new service in the DI container |
test/base/simulationContext.ts | Adds service registration for test environment |
@IRunCommandExecutionService _commandService: IRunCommandExecutionService, | ||
@IInstantiationService private readonly _instantiationService: IInstantiationService, | ||
@IGithubCodeSearchService private readonly _githubCodeSearch: IGithubCodeSearchService, | ||
@IGithubAvailableEmbeddingTypesService private readonly _availableEmbeddingTypesManager: Lazy<GithubAvailableEmbeddingTypesService>, |
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.
The parameter type annotation is incorrect. It should be IGithubAvailableEmbeddingTypesService
not Lazy<GithubAvailableEmbeddingTypesService>
since the service is now injected directly through DI rather than being lazy-loaded.
Copilot uses AI. Check for mistakes.
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.
@mjbvz yeah, actually this is interesting. Intentional?
If the changes appear safe, you can manually trigger the pipeline by commenting |
If the changes appear safe, you can manually trigger the pipeline by commenting |
* Update fetch tool to use metis microsoft/vscode#268956 * Add service dep for tests to * Use mock service for tests * Fix lazy
Fixes microsoft/vscode#268956