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

Conversation

valerybugakov
Copy link
Member

  • WIP
  • Adds PCW metadata to autocomplete requests for enterprise users:
    metadata?: {
        /**
         * File path relative to the git root.
         */
        gitFilePath?: string
        /**
         * Repository names resolved using Sourcegraph API based on remote URLs (such as `github.com/foo/bar`).
         */
        gitRepoNames?: string[]
        /**
         * git rev-parse HEAD
         */
        gitCommitHash?: string
    }
  • TODO:
    • Manual testing
    • New unit tests
    • Add gitCommitHash. Getting an always green commit hash will be tricky for autocomplete in the agent where we don't have the Git VS Code extension. We don't want to touch fs on every request, and we don't have any file-watching infra setup to get notifications about .git/HEAD being updated. I plan to add a naive timeout-based caching to start with.
  • Part of https://linear.app/sourcegraph/issue/CODY-4122/ide-clients-include-repository-name-and-git-file-path-in-chat

Test plan

CI

Changelog

  • Adds

/**
* 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.

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.

Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Thank you for this! How big of a pull would it be to additionally capture this metadata for chat requests (including edit and smart apply)?

As a starting point, we're going to focus on chat for the audit log because the autocomplete request volume is ~30-50x larger and quite a bit noisier. Almost all CLI/API clients hit chat only.

This PR LGTM 👍🏻 I mostly reviewed the metadata format, and didn't thoroughly review the diffs that propagate the information correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants