Skip to content

Commit

Permalink
Autocomplete: Improve suggest widget interop and add feature flag (#1158
Browse files Browse the repository at this point in the history
)

Fixes #808

This PR does a few distinct things:

1. It fixes a bug in the `completionMatchesPopupItem` behavior (where we
reimplement when a completion is shown in VS Code). Turns out that
sometimes, with the completion range being extended to start at the
beginning of the line, we don't have the right text to compare it
against. This fixes it.
2. Does some bookeeping about the previous call to the completion
callback in `lastCompletionRequest`. We can use this to know if _only_
the suggest widget was changed and use this to complete from the suggest
widget selection. This fixes the number 1 issue people had with #808
where low quality suggest widget selection caused a bad experience. Now,
we only use the suggest widget if the user changed it _or_ it happens to
match the completion we would do without considering this. This results
in a much nicer UX IMO.
3. Adds a feature flag to enable this behavior

https://github.com/sourcegraph/cody/assets/458591/fd640143-a09f-4567-983c-2f32d939589e

(One time it appears to be buggy but you can see the backend returns
`go` instead of `og` for `.log` 🙃

<!-- Required. See
https://docs.sourcegraph.com/dev/background-information/testing_principles.
-->
  • Loading branch information
philipp-spiess committed Sep 26, 2023
1 parent b7b0f28 commit af56d18
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 13 deletions.
1 change: 1 addition & 0 deletions lib/shared/src/experimentation/FeatureFlagProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export enum FeatureFlag {
CodyAutocompleteMinimumLatency350 = 'cody-autocomplete-minimum-latency-350',
CodyAutocompleteMinimumLatency600 = 'cody-autocomplete-minimum-latency-600',
CodyAutocompleteGraphContext = 'cody-autocomplete-graph-context',
CodyAutocompleteCompleteSuggestWidgetSelection = 'cody-autocomplete-complete-suggest-widget-selection',
}

const ONE_HOUR = 60 * 60 * 1000
Expand Down
2 changes: 2 additions & 0 deletions vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Starting from `0.2.0`, Cody is using `major.EVEN_NUMBER.patch` for release versi

### Changed

- Improves interop with the VS Code suggest widget when using the `completeSuggestWidgetSelection` feature flag. [pull/1158](https://github.com/sourcegraph/cody/pull/1158)

## [0.12.1]

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ export async function createInlineCompletionItemProvider({

const disposables: vscode.Disposable[] = []

const [providerConfig, graphContextFlag] = await Promise.all([
const [providerConfig, graphContextFlag, completeSuggestWidgetSelectionFlag] = await Promise.all([
createProviderConfig(config, client, featureFlagProvider, authProvider.getAuthStatus().configOverwrites),
featureFlagProvider?.evaluateFeatureFlag(FeatureFlag.CodyAutocompleteGraphContext),
featureFlagProvider?.evaluateFeatureFlag(FeatureFlag.CodyAutocompleteCompleteSuggestWidgetSelection),
])
if (providerConfig) {
const history = new VSCodeDocumentHistory()
Expand All @@ -60,7 +61,8 @@ export async function createInlineCompletionItemProvider({
getCodebaseContext: () => contextProvider.context,
isEmbeddingsContextEnabled: config.autocompleteAdvancedEmbeddings,
graphContextFetcher: sectionObserver,
completeSuggestWidgetSelection: config.autocompleteExperimentalCompleteSuggestWidgetSelection,
completeSuggestWidgetSelection:
config.autocompleteExperimentalCompleteSuggestWidgetSelection || completeSuggestWidgetSelectionFlag,
featureFlagProvider,
})

Expand Down
45 changes: 44 additions & 1 deletion vscode/src/completions/inline-completion-item-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ describe('InlineCompletionItemProvider', () => {
})

describe('completeSuggestWidgetSelection', () => {
it('appends the current selected widget item to the doc context for the completer and removes the injected prefix from the result', async () => {
it('does not append the current selected widget item to the doc context on a new request', async () => {
const { document, position } = documentAndPosition(
dedent`
function foo() {
Expand All @@ -315,6 +315,49 @@ describe('InlineCompletionItemProvider', () => {
selectedCompletionInfo: { text: 'log', range: new vsCodeMocks.Range(1, 12, 1, 13) },
})

expect(fn).toBeCalledWith(
expect.objectContaining({
docContext: expect.objectContaining({
prefix: 'function foo() {\n console.l',
suffix: '\n console.foo()\n}',
currentLinePrefix: ' console.l',
currentLineSuffix: '',
nextNonEmptyLine: ' console.foo()',
prevNonEmptyLine: 'function foo() {',
}),
})
)
expect(items).toBe(null)
})

it('appends the current selected widget item to the doc context for the completer and removes the injected prefix from the result when the context item was changed', async () => {
const { document, position } = documentAndPosition(
dedent`
function foo() {
console.l█
console.foo()
}
`,
'typescript'
)
const fn = vi.fn(getInlineCompletions).mockResolvedValue({
logId: '1',
items: [{ insertText: "('hello world!')", range: new vsCodeMocks.Range(1, 12, 1, 13) }],
source: InlineCompletionsResultSource.Network,
})

const provider = new MockableInlineCompletionItemProvider(fn, { completeSuggestWidgetSelection: true })

// Ignore the first call, it will not use the selected completion info
await provider.provideInlineCompletionItems(document, position, {
triggerKind: vsCodeMocks.InlineCompletionTriggerKind.Automatic,
selectedCompletionInfo: { text: 'dir', range: new vsCodeMocks.Range(1, 12, 1, 13) },
})
const items = await provider.provideInlineCompletionItems(document, position, {
triggerKind: vsCodeMocks.InlineCompletionTriggerKind.Automatic,
selectedCompletionInfo: { text: 'log', range: new vsCodeMocks.Range(1, 12, 1, 13) },
})

expect(fn).toBeCalledWith(
expect.objectContaining({
docContext: expect.objectContaining({
Expand Down
87 changes: 77 additions & 10 deletions vscode/src/completions/inline-completion-item-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,17 @@ export interface CodyCompletionItemProviderConfig {
featureFlagProvider: FeatureFlagProvider
}

interface CompletionRequest {
document: vscode.TextDocument
position: vscode.Position
context: vscode.InlineCompletionContext
}

export class InlineCompletionItemProvider implements vscode.InlineCompletionItemProvider {
private promptChars: number
private maxPrefixChars: number
private maxSuffixChars: number
private lastCompletionRequest: CompletionRequest | null = null
// private reportedErrorMessages: Map<string, number> = new Map()
private resetRateLimitErrorsAfter: number | null = null

Expand Down Expand Up @@ -127,6 +134,11 @@ export class InlineCompletionItemProvider implements vscode.InlineCompletionItem
// Making it optional here to execute multiple suggestion in parallel from the CLI script.
token?: vscode.CancellationToken
): Promise<AutocompleteResult | null> {
// Update the last request
const lastCompletionRequest = this.lastCompletionRequest
const completionRequest: CompletionRequest = { document, position, context }
this.lastCompletionRequest = completionRequest

const start = performance.now()
// We start the request early so that we have a high chance of getting a response before we
// need it.
Expand Down Expand Up @@ -160,14 +172,25 @@ export class InlineCompletionItemProvider implements vscode.InlineCompletionItem
return null
}

let takeSuggestWidgetSelectionIntoAccount = false
// Only take the completion widget selection into account if the selection was actively changed
// by the user
if (
this.config.completeSuggestWidgetSelection &&
lastCompletionRequest &&
onlyCompletionWidgetSelectionChanged(lastCompletionRequest, completionRequest)
) {
takeSuggestWidgetSelectionIntoAccount = true
}

const docContext = getCurrentDocContext({
document,
position,
maxPrefixLength: this.maxPrefixChars,
maxSuffixLength: this.maxSuffixChars,
enableExtendedTriggers: this.config.providerConfig.enableExtendedMultilineTriggers,
// We ignore the current context selection if completeSuggestWidgetSelection is not enabled
context: this.config.completeSuggestWidgetSelection ? context : undefined,
context: takeSuggestWidgetSelectionIntoAccount ? context : undefined,
})

const isIncreasedDebounceTimeEnabled = await this.config.featureFlagProvider.evaluateFeatureFlag(
Expand Down Expand Up @@ -267,17 +290,19 @@ export class InlineCompletionItemProvider implements vscode.InlineCompletionItem
document,
position,
result.items,
context
context,
takeSuggestWidgetSelectionIntoAccount
)

// A completion that won't be visible in VS Code will not be returned and not be logged.
if (
!isCompletionVisible(
items,
document,
position,
docContext,
context,
this.config.completeSuggestWidgetSelection,
takeSuggestWidgetSelectionIntoAccount,
abortController.signal
)
) {
Expand Down Expand Up @@ -354,7 +379,8 @@ export class InlineCompletionItemProvider implements vscode.InlineCompletionItem
document: vscode.TextDocument,
position: vscode.Position,
items: InlineCompletionItemWithAnalytics[],
context: vscode.InlineCompletionContext
context: vscode.InlineCompletionContext,
takeSuggestWidgetSelectionIntoAccount: boolean = false
): vscode.InlineCompletionItem[] {
return items.map(completion => {
const currentLine = document.lineAt(position)
Expand All @@ -363,7 +389,7 @@ export class InlineCompletionItemProvider implements vscode.InlineCompletionItem

// Append any eventual inline completion context item to the prefix if
// completeSuggestWidgetSelection is enabled.
if (this.config.completeSuggestWidgetSelection && context.selectedCompletionInfo) {
if (takeSuggestWidgetSelectionIntoAccount && context.selectedCompletionInfo) {
const { range, text } = context.selectedCompletionInfo
insertText = text.slice(position.character - range.start.character) + insertText
}
Expand Down Expand Up @@ -456,6 +482,7 @@ function createTracerForInvocation(tracer: ProvideInlineCompletionItemsTracer):
function isCompletionVisible(
completions: vscode.InlineCompletionItem[],
document: vscode.TextDocument,
position: vscode.Position,
docContext: DocumentContext,
context: vscode.InlineCompletionContext,
completeSuggestWidgetSelection: boolean,
Expand All @@ -482,7 +509,7 @@ function isCompletionVisible(
const isAborted = abortSignal ? abortSignal.aborted : false
const isMatchingPopupItem = completeSuggestWidgetSelection
? true
: completionMatchesPopupItem(completions, document, context)
: completionMatchesPopupItem(completions, position, document, context)
const isMatchingSuffix = completionMatchesSuffix(completions, docContext)
const isVisible = !isAborted && isMatchingPopupItem && isMatchingSuffix

Expand Down Expand Up @@ -522,18 +549,27 @@ function currentEditorContentMatchesPopupItem(
// VS Code won't show a completion if it won't.
function completionMatchesPopupItem(
completions: vscode.InlineCompletionItem[],
position: vscode.Position,
document: vscode.TextDocument,
context: vscode.InlineCompletionContext
): boolean {
if (context.selectedCompletionInfo) {
const currentText = document.getText(context.selectedCompletionInfo.range)
const selectedText = context.selectedCompletionInfo.text

if (completions.length > 0) {
const visibleCompletion = completions[0]
if (
typeof visibleCompletion.insertText === 'string' &&
!(currentText + visibleCompletion.insertText).startsWith(selectedText)
) {
const insertText = visibleCompletion.insertText
if (typeof insertText !== 'string') {
return true
}

// To ensure a good experience, the VS Code insertion might have the range start at the
// beginning of the line. When this happens, the insertText needs to be adjusted to only
// contain the insertion after the current position.
const offset = position.character - (visibleCompletion.range?.start.character ?? position.character)
const correctInsertText = insertText.slice(offset)
if (!(currentText + correctInsertText).startsWith(selectedText)) {
return false
}
}
Expand Down Expand Up @@ -563,3 +599,34 @@ function completionMatchesSuffix(completions: vscode.InlineCompletionItem[], doc

return false
}

/**
* Returns true if the only difference between the two requests is the selected completions info
* item from the completions widget.
*/
function onlyCompletionWidgetSelectionChanged(prev: CompletionRequest, next: CompletionRequest): boolean {
if (prev.document.uri.toString() !== next.document.uri.toString()) {
return false
}

if (!prev.position.isEqual(next.position)) {
return false
}

if (prev.context.triggerKind !== next.context.triggerKind) {
return false
}

const prevSelectedCompletionInfo = prev.context.selectedCompletionInfo
const nextSelectedCompletionInfo = next.context.selectedCompletionInfo

if (!prevSelectedCompletionInfo || !nextSelectedCompletionInfo) {
return false
}

if (!prevSelectedCompletionInfo.range.isEqual(nextSelectedCompletionInfo.range)) {
return false
}

return prevSelectedCompletionInfo.text !== nextSelectedCompletionInfo.text
}

0 comments on commit af56d18

Please sign in to comment.