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

Autocomplete: add git metadata to autocomplete requests #5955

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/shared/src/inferenceClient/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ export type CodeCompletionsParams = Omit<CompletionParameters, 'fast'> & {
timeoutMs: number
// TODO: apply the same type to the underlying `CompletionParameters`
model?: LegacyModelRefStr | ModelRefStr
metadata?: {
/**
* File path relative to the git root.
*/
gitFilePath?: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this an array? I can imagine we'll support multi-file edits at one point, and the request will be associated to multiple files.

/**
* Repository names resolved using Sourcegraph API based on remote URLs (such as `github.com/foo/bar`).
*/
gitRepoNames?: string[]
/**
* git rev-parse HEAD
*/
gitCommitHash?: string
}
}
export type SerializedCodeCompletionsParams = Omit<SerializedCompletionParameters, 'fast'>

Expand Down
1 change: 1 addition & 0 deletions lib/shared/src/misc/observableOperation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { catchError, filter, firstValueFrom, shareReplay, startWith, switchMap }

/** A sentinel value to indicate that the value has not yet been emitted. */
export const pendingOperation = Symbol.for('@@pendingOperation')
export type MaybePendingObservable<T> = Observable<T | typeof pendingOperation>

/**
* Run an operation with the outer observable as input. The result is replayed to all subscribers
Expand Down
12 changes: 10 additions & 2 deletions vscode/src/completions/get-inline-completions-tests/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import dedent from 'dedent'
import { Observable } from 'observable-fns'
import { vi } from 'vitest'
import type { URI } from 'vscode-uri'
import { URI } from 'vscode-uri'

import {
AUTH_STATUS_FIXTURE_AUTHED,
Expand All @@ -28,7 +28,7 @@ import type {
CodeCompletionsParams,
CompletionResponseWithMetaData,
} from '@sourcegraph/cody-shared/src/inferenceClient/misc'

import { repoNameResolver } from '../../repository/repo-name-resolver'
import { DEFAULT_VSCODE_SETTINGS } from '../../testutils/mocks'
import type { SupportedLanguage } from '../../tree-sitter/grammars'
import { updateParseTreeCache } from '../../tree-sitter/parse-tree-cache'
Expand Down Expand Up @@ -411,6 +411,14 @@ export function initCompletionProviderConfig({
authStatus,
}: Partial<Pick<ParamsResult, 'configuration' | 'authStatus'>>): void {
setEditorWindowIsFocused(() => true)

vi.spyOn(repoNameResolver, 'getRepoInfoContainingUri').mockReturnValue(
Observable.of({
rootUri: URI.file('/repoName'),
repoNames: ['sourcegraph/repoName'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing that these repo names will be using sourcegraph formatting like github.com/sourcegraph/name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we use Sourcegraph API to resolve repo names based on remote URLs.

})
)

vi.spyOn(featureFlagProvider, 'evaluateFeatureFlagEphemerally').mockResolvedValue(false)
vi.spyOn(featureFlagProvider, 'evaluatedFeatureFlag').mockReturnValue(Observable.of(false))
vi.spyOn(ClientConfigSingleton.getInstance(), 'getConfig').mockResolvedValue({
Expand Down
31 changes: 30 additions & 1 deletion vscode/src/completions/providers/shared/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@ import {
type GitContext,
type LegacyModelRefStr,
type Model,
currentAuthStatus,
firstResultFromOperation,
isDotCom,
pathFunctionsForURI,
toLegacyModel,
tokensToChars,
} from '@sourcegraph/cody-shared'

import { repoNameResolver } from '../../../repository/repo-name-resolver'
import type * as CompletionAnalyticsLogger from '../../analytics-logger'
import { defaultCodeCompletionsClient } from '../../default-client'
import type { TriggerKind } from '../../get-inline-completions'
Expand Down Expand Up @@ -232,14 +237,38 @@ export abstract class Provider {
return Promise.resolve(this.client.complete(requestParams, abortController))
}

protected async getRequestParamsMetadata(
document: TextDocument,
abortSignal: AbortSignal
): Promise<CodeCompletionsParams['metadata']> {
const repoInfo = await firstResultFromOperation(
repoNameResolver.getRepoInfoContainingUri(document.uri),
abortSignal
)

const gitFilePath = repoInfo?.rootUri
? pathFunctionsForURI(document.uri).relative(repoInfo?.rootUri.path, document.uri.path)
: undefined

// TODO: add git commit hash
return {
gitFilePath,
gitRepoNames: repoInfo?.repoNames,
}
}

public async generateCompletions(
generateOptions: GenerateCompletionsOptions,
abortSignal: AbortSignal,
tracer?: CompletionProviderTracer
): Promise<AsyncGenerator<FetchCompletionResult[]>> {
const { docContext, numberOfCompletionsToGenerate } = generateOptions
const { docContext, numberOfCompletionsToGenerate, document } = generateOptions

const requestParams = this.getRequestParams(generateOptions) as CodeCompletionsParams
// Add git metadata for enterprise users.
if (!('metadata' in requestParams) && !isDotCom(currentAuthStatus())) {
requestParams.metadata = await this.getRequestParamsMetadata(document, abortSignal)
}
tracer?.params(requestParams)

const completionsGenerators = Array.from({ length: numberOfCompletionsToGenerate }).map(
Expand Down
25 changes: 16 additions & 9 deletions vscode/src/repository/remote-urls-from-parent-dirs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@ import * as vscode from 'vscode'
import { URI } from 'vscode-uri'

import { vscodeGitAPI } from './git-extension-api'
import { gitRemoteUrlsForUri } from './remote-urls-from-parent-dirs'
import { gitRemoteUrlsInfoForUri } from './remote-urls-from-parent-dirs'
import { mockFsCalls } from './test-helpers'

describe('gitRemoteUrlsForUri', () => {
async function gitRemoteUrlsForUri(uri: vscode.Uri): Promise<string[] | undefined> {
const repoInfo = await gitRemoteUrlsInfoForUri(uri)
return repoInfo?.remoteUrls
}

describe('gitRemoteUrlsInfoForUri', () => {
beforeAll(() => {
// Ensure that the `vscodeGitAPI` is not somehow set, because these tests were written to
// test the behavior that is the fallback for when it is not set.
Expand Down Expand Up @@ -75,9 +80,9 @@ describe('gitRemoteUrlsForUri', () => {

const remoteUrls = await gitRemoteUrlsForUri(fileUri)
expect(remoteUrls).toEqual([
'https://github.com/username/yourproject.git',
'https://github.com/originalauthor/yourproject.git',
'git@backupserver:repositories/yourproject.git',
'https://github.com/originalauthor/yourproject.git',
'https://github.com/username/yourproject.git',
])
})

Expand Down Expand Up @@ -109,7 +114,7 @@ describe('gitRemoteUrlsForUri', () => {
.mockRejectedValue(new vscode.FileSystemError('file does not exist'))

const uri = URI.file('repo/src/dir/foo.ts')
const remoteUrls = await gitRemoteUrlsForUri(uri)
const remoteUrls = await gitRemoteUrlsInfoForUri(uri)

expect(statMock).toBeCalledTimes(5)
expect(remoteUrls).toBe(undefined)
Expand Down Expand Up @@ -209,10 +214,12 @@ describe('gitRemoteUrlsForUri', () => {
gitConfig: 'a',
})

expect(await gitRemoteUrlsForUri(URI.parse('https://example.com/foo/bar'))).toBe(undefined)
expect(await gitRemoteUrlsForUri(URI.parse('https://gitlab.com/foo/bar'))).toBe(undefined)
expect(await gitRemoteUrlsForUri(URI.parse('https://github.com/foo/bar'))).toBe(undefined)
expect(await gitRemoteUrlsForUri(URI.parse('ssh://git@github.com:foo/bar.git'))).toBe(undefined)
expect(await gitRemoteUrlsInfoForUri(URI.parse('https://example.com/foo/bar'))).toBe(undefined)
expect(await gitRemoteUrlsInfoForUri(URI.parse('https://gitlab.com/foo/bar'))).toBe(undefined)
expect(await gitRemoteUrlsInfoForUri(URI.parse('https://github.com/foo/bar'))).toBe(undefined)
expect(await gitRemoteUrlsInfoForUri(URI.parse('ssh://git@github.com:foo/bar.git'))).toBe(
undefined
)
expect(statMock).toBeCalledTimes(0)
expect(readFileMock).toBeCalledTimes(0)
})
Expand Down
52 changes: 39 additions & 13 deletions vscode/src/repository/remote-urls-from-parent-dirs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,44 @@ import { vscodeGitAPI } from './git-extension-api'

const textDecoder = new TextDecoder('utf-8')

export interface GitRemoteUrlsInfo {
rootUri: vscode.Uri
remoteUrls: string[]
}

/**
* Get the Git remote URLs for a given URI, which is assumed to be a file or path in a Git
* repository. If it's not in a Git repository or there are no remote URLs, it returns `undefined`.
*
* This function tries 2 different ways to get the remote URLs: (1) by using the Git extension API
* available in VS Code only, and (2) by crawling the file system for the `.git/config` file.
*/
export async function gitRemoteUrlsForUri(
export async function gitRemoteUrlsInfoForUri(
uri: vscode.Uri,
signal?: AbortSignal
): Promise<string[] | undefined> {
const fromGitExtension = gitRemoteUrlsFromGitExtension(uri)
if (fromGitExtension && fromGitExtension.length > 0) {
return fromGitExtension
): Promise<GitRemoteUrlsInfo | undefined> {
let remoteUrlsInfo = gitRemoteUrlsInfoFromGitExtension(uri)

if (!remoteUrlsInfo || remoteUrlsInfo.remoteUrls.length === 0) {
remoteUrlsInfo = await gitRemoteUrlsInfoFromParentDirs(uri, signal)
}
return await gitRemoteUrlsFromParentDirs(uri, signal)

if (remoteUrlsInfo && remoteUrlsInfo.remoteUrls.length > 0) {
remoteUrlsInfo.remoteUrls = Array.from(new Set(remoteUrlsInfo.remoteUrls)).sort()
return remoteUrlsInfo
}

return undefined
}

/**
* Walks the tree from the current directory to find the `.git` folder and
* extracts remote URLs.
*/
async function gitRemoteUrlsFromParentDirs(
async function gitRemoteUrlsInfoFromParentDirs(
uri: vscode.Uri,
signal?: AbortSignal
): Promise<string[] | undefined> {
): Promise<GitRemoteUrlsInfo | undefined> {
if (!isFileURI(uri)) {
return undefined
}
Expand All @@ -40,15 +52,25 @@ async function gitRemoteUrlsFromParentDirs(
const dirUri = isFile ? vscode.Uri.joinPath(uri, '..') : uri

const gitRepoURIs = await gitRepoURIsFromParentDirs(dirUri, signal)
return gitRepoURIs
? await gitRemoteUrlsFromGitConfigUri(gitRepoURIs.gitConfigUri, signal)
: undefined

if (gitRepoURIs) {
const remoteUrls = await gitRemoteUrlsFromGitConfigUri(gitRepoURIs.gitConfigUri, signal)

if (remoteUrls && remoteUrls.length > 0) {
return {
rootUri: gitRepoURIs.rootUri,
remoteUrls: remoteUrls || [],
}
}
}

return undefined
}

/**
* ❗️ The Git extension API instance is only available in the VS Code extension. ️️❗️
*/
function gitRemoteUrlsFromGitExtension(uri: vscode.Uri): string[] | undefined {
function gitRemoteUrlsInfoFromGitExtension(uri: vscode.Uri): GitRemoteUrlsInfo | undefined {
const repository = vscodeGitAPI?.getRepository(uri)
if (!repository) {
return undefined
Expand All @@ -63,7 +85,11 @@ function gitRemoteUrlsFromGitExtension(uri: vscode.Uri): string[] | undefined {
remoteUrls.add(remote.pushUrl)
}
}
return remoteUrls.size ? Array.from(remoteUrls) : undefined
repository.state.HEAD?.commit

return remoteUrls.size
? { rootUri: repository.rootUri, remoteUrls: Array.from(remoteUrls) }
: undefined
}

interface GitRepoURIs {
Expand Down
15 changes: 9 additions & 6 deletions vscode/src/repository/repo-name-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
mockResolvedConfig,
} from '@sourcegraph/cody-shared'

import { URI } from 'vscode-uri'
import * as remoteUrlsFromParentDirs from './remote-urls-from-parent-dirs'
import { RepoNameResolver } from './repo-name-resolver'
import { mockFsCalls } from './test-helpers'
Expand All @@ -24,9 +25,10 @@ describe('getRepoNamesContainingUri', () => {
mockResolvedConfig({ auth: {} })
mockClientCapabilities(CLIENT_CAPABILITIES_FIXTURE)

vi.spyOn(remoteUrlsFromParentDirs, 'gitRemoteUrlsForUri').mockResolvedValue([
'git@github.com:sourcegraph/cody.git',
])
vi.spyOn(remoteUrlsFromParentDirs, 'gitRemoteUrlsInfoForUri').mockResolvedValue({
rootUri: URI.file('/repo'),
remoteUrls: ['git@github.com:sourcegraph/cody.git'],
})

const { fileUri } = mockFsCalls({
filePath: '/repo/submodule/foo.ts',
Expand Down Expand Up @@ -55,9 +57,10 @@ describe('getRepoNamesContainingUri', () => {
const repoNameResolver = new RepoNameResolver()
mockAuthStatus(AUTH_STATUS_FIXTURE_AUTHED_DOTCOM)

vi.spyOn(remoteUrlsFromParentDirs, 'gitRemoteUrlsForUri').mockResolvedValue([
'git@github.com:sourcegraph/cody.git',
])
vi.spyOn(remoteUrlsFromParentDirs, 'gitRemoteUrlsInfoForUri').mockResolvedValue({
rootUri: URI.file('/repo'),
remoteUrls: ['git@github.com:sourcegraph/cody.git'],
})

const { fileUri } = mockFsCalls({
filePath: '/repo/submodule/foo.ts',
Expand Down
Loading
Loading