From 1c6f5cd331aadce91125daf7fb382b391ebe42d3 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Mon, 22 Apr 2024 13:36:00 +0200 Subject: [PATCH 1/4] Autocomplete: Implement context filter --- .../completions/context/context-mixer.test.ts | 67 ++++++++++++++++++- .../src/completions/context/context-mixer.ts | 20 +++++- .../inline-completion-item-provider.test.ts | 20 ++++++ .../inline-completion-item-provider.ts | 5 ++ 4 files changed, 109 insertions(+), 3 deletions(-) diff --git a/vscode/src/completions/context/context-mixer.test.ts b/vscode/src/completions/context/context-mixer.test.ts index b6739b1da5ce..06e745aa69bf 100644 --- a/vscode/src/completions/context/context-mixer.test.ts +++ b/vscode/src/completions/context/context-mixer.test.ts @@ -1,8 +1,9 @@ -import { beforeAll, describe, expect, it, vi } from 'vitest' +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { type AutocompleteContextSnippet, CODY_IGNORE_URI_PATH, + contextFiltersProvider, ignores, isCodyIgnoredFile, testFileUri, @@ -17,6 +18,8 @@ import { Utils } from 'vscode-uri' import { ContextMixer } from './context-mixer' import type { ContextStrategyFactory } from './context-strategy' +import type * as vscode from 'vscode' + function createMockStrategy(resultSets: AutocompleteContextSnippet[][]): ContextStrategyFactory { const retrievers = [] for (const [index, set] of resultSets.entries()) { @@ -55,6 +58,10 @@ const defaultOptions = { } describe('ContextMixer', () => { + beforeEach(() => { + vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation(() => Promise.resolve(true)) + }) + describe('with no retriever', () => { it('returns empty result if no retrievers', async () => { const mixer = new ContextMixer(createMockStrategy([])) @@ -225,7 +232,7 @@ describe('ContextMixer', () => { }) }) - describe('retrived context is filtered by .cody/ignore', () => { + describe('retrieved context is filtered by .cody/ignore', () => { const workspaceRoot = testFileUri('') beforeAll(() => { ignores.setActiveState(true) @@ -287,6 +294,62 @@ describe('ContextMixer', () => { } }) }) + + describe('retrieved context is filtered by context filters', () => { + beforeAll(() => { + vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation( + async (uri: vscode.Uri) => { + if (uri.path.includes('foo.ts')) { + return false + } + return true + } + ) + }) + it('mixes results are filtered', async () => { + const mixer = new ContextMixer( + createMockStrategy([ + [ + { + uri: testFileUri('foo.ts'), + content: 'function foo1() {}', + startLine: 0, + endLine: 0, + }, + { + uri: testFileUri('foo/bar.ts'), + content: 'function bar1() {}', + startLine: 0, + endLine: 0, + }, + ], + [ + { + uri: testFileUri('test/foo.ts'), + content: 'function foo3() {}', + startLine: 10, + endLine: 10, + }, + { + uri: testFileUri('foo.ts'), + content: 'function foo1() {}\nfunction foo2() {}', + startLine: 0, + endLine: 1, + }, + { + uri: testFileUri('example/bar.ts'), + content: 'function bar1() {}\nfunction bar2() {}', + startLine: 0, + endLine: 1, + }, + ], + ]) + ) + const { context } = await mixer.getContext(defaultOptions) + const contextFiles = normalize(context) + expect(contextFiles.map(c => c.fileName)).toEqual(['bar.ts', 'bar.ts']) + }) + }) }) }) diff --git a/vscode/src/completions/context/context-mixer.ts b/vscode/src/completions/context/context-mixer.ts index 46debbcbed62..afbebce493d6 100644 --- a/vscode/src/completions/context/context-mixer.ts +++ b/vscode/src/completions/context/context-mixer.ts @@ -3,6 +3,7 @@ import type * as vscode from 'vscode' import { type AutocompleteContextSnippet, type DocumentContext, + contextFiltersProvider, isCodyIgnoredFile, wrapInActiveSpan, } from '@sourcegraph/cody-shared' @@ -95,7 +96,8 @@ export class ContextMixer implements vscode.Disposable { }, }) ) - const filteredSnippets = allSnippets.filter(snippet => !isCodyIgnoredFile(snippet.uri)) + + const filteredSnippets = await filter(allSnippets) return { identifier: retriever.identifier, @@ -177,3 +179,19 @@ export class ContextMixer implements vscode.Disposable { this.strategyFactory.dispose() } } + +async function filter(snippets: AutocompleteContextSnippet[]): Promise { + return ( + await Promise.all( + snippets.map(async snippet => { + if (isCodyIgnoredFile(snippet.uri)) { + return null + } + if (!(await contextFiltersProvider.isUriAllowed(snippet.uri))) { + return null + } + return snippet + }) + ) + ).filter((snippet): snippet is AutocompleteContextSnippet => snippet !== null) +} diff --git a/vscode/src/completions/inline-completion-item-provider.test.ts b/vscode/src/completions/inline-completion-item-provider.test.ts index f9e19b6a0750..fac4041b36dd 100644 --- a/vscode/src/completions/inline-completion-item-provider.test.ts +++ b/vscode/src/completions/inline-completion-item-provider.test.ts @@ -6,6 +6,7 @@ import { type AuthStatus, type GraphQLAPIClientConfig, RateLimitError, + contextFiltersProvider, graphqlClient, } from '@sourcegraph/cody-shared' @@ -90,6 +91,9 @@ describe('InlineCompletionItemProvider', () => { beforeAll(async () => { await initCompletionProviderConfig({}) }) + beforeEach(() => { + vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation(() => Promise.resolve(true)) + }) it('returns results that span the whole line', async () => { const { document, position } = documentAndPosition('const foo = █', 'typescript') @@ -225,6 +229,22 @@ describe('InlineCompletionItemProvider', () => { expect(fn.mock.calls.map(call => call[0].lastCandidate?.result.items)).toEqual([[item]]) }) + it('no-ops on files that are ignored by the context filter policy', async () => { + vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementationOnce(() => + Promise.resolve(false) + ) + const { document, position } = documentAndPosition('const foo = █', 'typescript') + const fn = vi.fn() + const provider = new MockableInlineCompletionItemProvider(fn) + const completions = await provider.provideInlineCompletionItems( + document, + position, + DUMMY_CONTEXT + ) + expect(completions).toBe(null) + expect(fn).not.toHaveBeenCalled() + }) + describe('onboarding', () => { // Set up local storage backed by an object. Local storage is used to // track whether a completion was accepted for the first time. diff --git a/vscode/src/completions/inline-completion-item-provider.ts b/vscode/src/completions/inline-completion-item-provider.ts index bd229858fd07..c5b014692946 100644 --- a/vscode/src/completions/inline-completion-item-provider.ts +++ b/vscode/src/completions/inline-completion-item-provider.ts @@ -5,6 +5,7 @@ import { ConfigFeaturesSingleton, FeatureFlag, RateLimitError, + contextFiltersProvider, isCodyIgnoredFile, wrapInActiveSpan, } from '@sourcegraph/cody-shared' @@ -206,6 +207,10 @@ export class InlineCompletionItemProvider } return wrapInActiveSpan('autocomplete.provideInlineCompletionItems', async span => { + if (!(await contextFiltersProvider.isUriAllowed(document.uri))) { + return null + } + // Update the last request const lastCompletionRequest = this.lastCompletionRequest const completionRequest: CompletionRequest = { From 6da09efb6d6f6d83b94920d7967288e425ce0897 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Mon, 22 Apr 2024 15:51:12 +0200 Subject: [PATCH 2/4] Add a debug log on why we're ignoring --- .../inline-completion-item-provider.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/vscode/src/completions/inline-completion-item-provider.ts b/vscode/src/completions/inline-completion-item-provider.ts index c5b014692946..1888004a02aa 100644 --- a/vscode/src/completions/inline-completion-item-provider.ts +++ b/vscode/src/completions/inline-completion-item-provider.ts @@ -203,11 +203,13 @@ export class InlineCompletionItemProvider ): Promise { // Do not create item for files that are on the cody ignore list if (isCodyIgnoredFile(document.uri)) { + logIgnored(document.uri, 'cody-ignore') return null } return wrapInActiveSpan('autocomplete.provideInlineCompletionItems', async span => { if (!(await contextFiltersProvider.isUriAllowed(document.uri))) { + logIgnored(document.uri, 'context-filter') return null } @@ -799,3 +801,16 @@ function onlyCompletionWidgetSelectionChanged( return prevSelectedCompletionInfo.text !== nextSelectedCompletionInfo.text } + +let lasIgnoredUriLogged: string | undefined = undefined +function logIgnored(uri: vscode.Uri, reason: 'cody-ignore' | 'context-filter') { + const string = uri.toString() + if (lasIgnoredUriLogged === string) { + return + } + lasIgnoredUriLogged = string + logDebug( + 'CodyCompletionProvider:ignored', + 'Cody is disabled in file ' + uri.toString() + '(' + reason + ')' + ) +} From 42965c783ce4260185f8a2e32a764d0603b1ae36 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Mon, 22 Apr 2024 15:51:42 +0200 Subject: [PATCH 3/4] Nit --- vscode/src/completions/inline-completion-item-provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vscode/src/completions/inline-completion-item-provider.ts b/vscode/src/completions/inline-completion-item-provider.ts index 1888004a02aa..3010df504cd7 100644 --- a/vscode/src/completions/inline-completion-item-provider.ts +++ b/vscode/src/completions/inline-completion-item-provider.ts @@ -811,6 +811,6 @@ function logIgnored(uri: vscode.Uri, reason: 'cody-ignore' | 'context-filter') { lasIgnoredUriLogged = string logDebug( 'CodyCompletionProvider:ignored', - 'Cody is disabled in file ' + uri.toString() + '(' + reason + ')' + 'Cody is disabled in file ' + uri.toString() + ' (' + reason + ')' ) } From 1a025f3fcb184ea36359b5cd50decf4dd13e479a Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Mon, 22 Apr 2024 16:50:24 +0200 Subject: [PATCH 4/4] Update --- vscode/src/completions/context/context-mixer.test.ts | 9 ++++----- vscode/src/completions/context/context-mixer.ts | 2 +- .../completions/inline-completion-item-provider.test.ts | 6 ++---- .../src/completions/inline-completion-item-provider.ts | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/vscode/src/completions/context/context-mixer.test.ts b/vscode/src/completions/context/context-mixer.test.ts index 06e745aa69bf..9b79e3731c02 100644 --- a/vscode/src/completions/context/context-mixer.test.ts +++ b/vscode/src/completions/context/context-mixer.test.ts @@ -59,7 +59,7 @@ const defaultOptions = { describe('ContextMixer', () => { beforeEach(() => { - vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation(() => Promise.resolve(true)) + vi.spyOn(contextFiltersProvider, 'isUriIgnored').mockResolvedValue(false) }) describe('with no retriever', () => { @@ -98,7 +98,6 @@ describe('ContextMixer', () => { ]) ) const { context, logSummary } = await mixer.getContext(defaultOptions) - expect(normalize(context)).toEqual([ { fileName: 'foo.ts', @@ -297,12 +296,12 @@ describe('ContextMixer', () => { describe('retrieved context is filtered by context filters', () => { beforeAll(() => { - vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation( + vi.spyOn(contextFiltersProvider, 'isUriIgnored').mockImplementation( async (uri: vscode.Uri) => { if (uri.path.includes('foo.ts')) { - return false + return true } - return true + return false } ) }) diff --git a/vscode/src/completions/context/context-mixer.ts b/vscode/src/completions/context/context-mixer.ts index afbebce493d6..270e1f0b55ca 100644 --- a/vscode/src/completions/context/context-mixer.ts +++ b/vscode/src/completions/context/context-mixer.ts @@ -187,7 +187,7 @@ async function filter(snippets: AutocompleteContextSnippet[]): Promise { await initCompletionProviderConfig({}) }) beforeEach(() => { - vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation(() => Promise.resolve(true)) + vi.spyOn(contextFiltersProvider, 'isUriIgnored').mockResolvedValue(false) }) it('returns results that span the whole line', async () => { @@ -230,9 +230,7 @@ describe('InlineCompletionItemProvider', () => { }) it('no-ops on files that are ignored by the context filter policy', async () => { - vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementationOnce(() => - Promise.resolve(false) - ) + vi.spyOn(contextFiltersProvider, 'isUriIgnored').mockResolvedValueOnce(true) const { document, position } = documentAndPosition('const foo = █', 'typescript') const fn = vi.fn() const provider = new MockableInlineCompletionItemProvider(fn) diff --git a/vscode/src/completions/inline-completion-item-provider.ts b/vscode/src/completions/inline-completion-item-provider.ts index 3010df504cd7..8b68c74bc1f1 100644 --- a/vscode/src/completions/inline-completion-item-provider.ts +++ b/vscode/src/completions/inline-completion-item-provider.ts @@ -208,7 +208,7 @@ export class InlineCompletionItemProvider } return wrapInActiveSpan('autocomplete.provideInlineCompletionItems', async span => { - if (!(await contextFiltersProvider.isUriAllowed(document.uri))) { + if (await contextFiltersProvider.isUriIgnored(document.uri)) { logIgnored(document.uri, 'context-filter') return null }