Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Implement server-side single-repo context fetching for RFC 969#63980

Merged
rafax merged 13 commits into
mainfrom
rg/rfc969context
Jul 23, 2024
Merged

Implement server-side single-repo context fetching for RFC 969#63980
rafax merged 13 commits into
mainfrom
rg/rfc969context

Conversation

@rafax

@rafax rafax commented Jul 22, 2024

Copy link
Copy Markdown
Contributor

Implements server-side part of AI-132, exposing a single-repo context endpoint backed by Zoekt.

Differences from the current Enterprise endpoint (that is left untouched):

  • supports a single repo only
  • supports passing a repo name, not Sourcegraph-assigned repo ID
  • has a shorter, 5s timeout
  • reports partial errors (if we hit issues converting search results to git results, or if some retrievers time out)

Test plan

  • tested locally with
{
  chatContext(
    query: "squirrel"
    repo: "github.com/sourcegraph/sourcegraph"
    interactionId: "foo"
  ) {
    contextItems {
      score
      retriever
      item {
        ... on FileChunkContext {
          blob {
            path
            repository {
              id
              name
            }
            commit {
              oid
            }
            url
          }
          startLine
          endLine
          chunkContent
        }
      }
    }
    partialErrors
  }
}

@cla-bot cla-bot Bot added the cla-signed label Jul 22, 2024
This reverts commit 4f465e9.
@rafax rafax requested a review from a team July 22, 2024 13:04
@rafax rafax changed the title Rg/rfc969context Implement server-side single-repo context fetching for RFC 969 Jul 22, 2024

@janhartman janhartman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Comment thread cmd/frontend/internal/context/resolvers/context.go Outdated
Comment thread cmd/frontend/graphqlbackend/cody_context.graphql
Comment thread cmd/frontend/graphqlbackend/cody_context.graphql Outdated
Comment thread cmd/frontend/internal/context/resolvers/context.go Outdated
}
success = true
res.contextItems = append(res.contextItems, items...)
partialErrors = append(partialErrors, pe...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This isn't thread safe - should we map (concurrently) first and then aggregate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair, let me use iter.Map

@rafax rafax requested a review from janhartman July 23, 2024 11:32

@janhartman janhartman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for the fix!

@rafax rafax merged commit a7f787a into main Jul 23, 2024
@rafax rafax deleted the rg/rfc969context branch July 23, 2024 11:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants