cody: Sourcegraph backend #49514

novoselrok · 2023-03-16T14:04:20Z

Implements the new Sourcegraph backend for the Cody extension. Since it's a fairly large PR, I recommend reviewing commit by commit.

Test plan

Chatting and executing recipes should still work.
pnpm run build && pnpm run test:integration && pnpm run test:unit should be green.

App preview:

Web

Check out the client app preview documentation to learn more.

…essary clients on config change.

abeatrix

I'm able to:

login with Sourcegraph Token (Super nice! 🎉 )
build and run extension
search without codebase
pass all the tests:

Cody extension activated
Setting up testing hooks
  End-to-end
    ✓ Cody registers some commands
no embeddings client for current codebase
    ✓ Explain Code (561ms)
  2 passing (584ms)
[main 2023-03-16T15:06:34.266Z] [UtilityProcess id: 1, type: extensionHost, pid: 18192]: waiting to exit...
[main 2023-03-16T15:06:34.267Z] [UtilityProcess id: 1, type: extensionHost, pid: 18192]: received exit event with code 0
[main 2023-03-16T15:06:34.267Z] Extension host with pid 18192 exited with code: 0, signal: unknown.
Exit code:   0
Done

  Prompt
    ✓ generates empty prompt with no messages


  1 passing (1ms)

bugs

Some bugs I found that might not have caused by changes introduced in this branch, but we should take a look at them later in other PRs.

debug logs is empty

Feedback buttons not working

chat bubble not responsive
chat input does not stay between views

novoselrok · 2023-03-16T17:16:23Z

@abeatrix

Re: bugs:

I purposefully removed debugging because it was obstructing the migration. I'll add it back in a separate PR.
The Sourcegraph backend does not support feedback, so I'll have to remove it entirely.
Existing problem, we should create an issue for it.
Existing problem, we should create an issue for it.

dominiccooney

Only partway through this but some thoughts inline. No need to block on me.

dominiccooney · 2023-03-17T05:17:53Z

client/cody/vscode-codegen/package.json

-          "default": "https://cody.sgdev.org",
-          "example": "https://cody.sgdev.org",
-          "description": "URL to the Cody embeddings API"
+          "default": "https://instance.sourcegraph.com",


Is is worth making this something invalid for a hostname, like instead of instance, to give a better hint that this needs to be set?

dominiccooney · 2023-03-17T05:24:01Z

client/cody/vscode-codegen/src/chat/ChatViewProvider.ts

@@ -30,51 +30,50 @@ export class ChatViewProvider implements vscode.WebviewViewProvider {
    private transcript: ChatMessage[] = []
    private messageInProgress: ChatMessage | null = null

-    private closeConnectionInProgressPromise: Promise<() => void> | null = null
-    private prompt: Transcript
+    private stopCompletionInProgressCallback: (() => void) | null = null


Phrasing of this is awkward, it reads to me like stop completion ... in the progress callback. But it's stopping the in-progress completion, or is it the callback to cause the in-progress completion to stop? It is the latter. What about cancelInProgressCompletion, cancelCompletion or something like that?

dominiccooney · 2023-03-17T05:27:08Z

client/cody/vscode-codegen/src/chat/ChatViewProvider.ts

-        private contextType: 'embeddings' | 'keyword' | 'none' | 'blended',
-        private debug: boolean,
-        private secretStorage: SecretStorage
+        private prompt: Transcript,


Not in this change, but I have been staring at Transcript a bit lately. We need more discipline over what Transcript, Chunk, and Message (displayed or actual) mean. Because we have a lot of overloading of "transcript" and "prompt" to mean, variously, sink for messages, factory for messages, array of messages; etc.

Could not agree more. The class is getting out of control. I think that should be our next order of business: to refactor the Transcript class, decide how the context is constructed, and enforce it using unit tests.

dominiccooney · 2023-03-17T05:30:54Z

client/cody/vscode-codegen/src/chat/ChatViewProvider.ts

-                this.accessToken = await this.getAccessToken()
-                this.wsclient = this.makeWSChatClient()
+            case 'endpoint': {
+                const accessToken = await getAccessToken(this.secretStorage)


There's some duplication wrt the setup in the constructor, can we factor this out into one place?

dominiccooney · 2023-03-17T05:31:34Z

client/cody/vscode-codegen/src/chat/ChatViewProvider.ts


-        this.prompt.addBotMessage(text)
+        this.messageInProgress = null


Does it make sense to put this pair into a little object so we always transition them together?

I'll try that, but it might get awkward.

dominiccooney · 2023-03-17T05:34:51Z

client/cody/vscode-codegen/src/chat/prompt.ts

@@ -58,7 +75,13 @@ export class Transcript {
    }

    private async getCodebaseContextMessages(query: string): Promise<ContextMessage[]> {
-        const { needsCurrentFileContext, needsCodebaseContext } = await this.intentDetector.detect(query)
+        const intentOrError = await this.intentDetector.detect(query)


Not for this diff, but we should experiment with a prompt architecture where we let Claude try to answer and push back on us if we need context, which we then provide. This will cut down the round trips for some queries. It will mean giving Claude some tools for requesting context which the plugin responds to.

There's also HIDE where we have to hallucinate answers and use the embedding for context fetching.

…ript classes. Replace example instance url with an invalid url.

beyang · 2023-03-20T17:23:25Z

client/cody/vscode-codegen/src/sourcegraph-api/completions/client.ts

+        this.completionsEndpoint = `${instanceUrl}/.api/completions/stream`
+    }
+
+    private sendEvents(events: Event[], cb: CompletionCallbacks): void {


would a better name here be receiveEvents? If I'm understanding correctly, this function receives events from the server and calls the appropriate callback on the client side, rather than sending data to the server.

Yes, the events were streamed from the server, but we are forwarding them to the "client" function that invoked the stream method and supplied the callbacks. So an alternative name could be forwardEvents.

This is my proposal for a new Cody extension architecture. It might not be the ideal architecture, but it's a huge improvement over the current situation. Here are the important concepts: * `Interaction` is the atomic unit of the chat. It contains the human question, the assistant response, and the context, if necessary. An interaction can be converted to a prompt or a (chat) UI message. * `Transcript` is a linear collection of `Interaction`s that can be converted to a prompt or a full UI conversation. It also handles limiting the prompt to a maximum length. * **Everything is a recipe**, including the chat questions. A recipe receives the necessary dependencies (chat input, editor, intent detection, and codebase context) and constructs an `Interaction`. The `ChatViewProvider` handles executing the recipe and adding the resulting `Interaction` to the `Transcript`. * Context fetching has been abstracted into the `CodebaseContext` class. It handles switching between `embeddings` and `keyword` searching. `blended` context type has not been implemented yet. * Context messages are included for the last message only. See unit tests for examples. * Prompt structure is now enforced through unit tests. ## Test plan * Chatting and executing recipes should still work. * `pnpm run build && pnpm run test:integration && pnpm run test:unit` should be green. ## App preview: - [Web](https://sg-web-rn-cody-refactor-extension.onrender.com/search) Check out the [client app preview documentation](https://docs.sourcegraph.com/dev/how-to/client_pr_previews) to learn more.

Rok Novosel added 18 commits March 16, 2023 14:50

Implement the Sourcegraph GraphQL API client.

ce7d2c0

Implement the Sourcegraph completions API client.

6e8915f

Expose Message type.

f8e5a18

Remove WS client and corresponding packages.

b38d0af

Refactor the React UI components to use the new Message type.

3a35c32

Refactor the KeywordContextFetcher to use the new Message type.

bc2e3ad

Refactor the Embeddings to use the Sourcegraph GQL API.

a987d88

Refactor the IntentDetector to use the Sourcegraph completions client.

e48e214

Refactor recipes to use the new Message type.

6bb86c8

Remove embeddingsEndpoint from configuration.

de3f226

Add isError util function.

eb02dda

Add a chat client powered by the Sourcegraph completions client.

6dade91

Refactor context search options.

fa320f9

Implement getAccessToken.

7c968c7

Use the new ChatClient to implement ChatViewProvider. Rebuild the nec…

93173d7

…essary clients on config change.

Use the new Message and EmbeddingsSearchResults in the Transcript class.

6c33c18

Use ChatViewProvider.create to instantiate the view.

e3d81a7

Fix integration and unit tests.

a4e7c5a

novoselrok requested review from dominiccooney and abeatrix March 16, 2023 14:04

cla-bot bot added the cla-signed label Mar 16, 2023

Rok Novosel added 2 commits March 16, 2023 15:05

Remove cody.sgdev.org.

5e3a87a

Fix formatting.

22cad01

abeatrix approved these changes Mar 16, 2023

View reviewed changes

Remove Feedback buttons.

0f85dd8

dominiccooney reviewed Mar 17, 2023

View reviewed changes

Rok Novosel added 2 commits March 17, 2023 07:56

Extract configuring external services for ChatViewProvider and Transc…

a477dc8

…ript classes. Replace example instance url with an invalid url.

Add getRgPath back.

1cba24c

sqs added the cody label Mar 18, 2023

beyang reviewed Mar 20, 2023

View reviewed changes

beyang approved these changes Mar 20, 2023

View reviewed changes

novoselrok merged commit 1b7ba64 into cody-main Mar 21, 2023

novoselrok deleted the rn/cody-sourcegraph-backend branch March 21, 2023 13:09

sourcegraph-bot-devx mentioned this pull request Apr 7, 2023

Cody tracking issue 2023-04-03–2023-04-17 #50343

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cody: Sourcegraph backend #49514

cody: Sourcegraph backend #49514

novoselrok commented Mar 16, 2023 •

edited by sourcegraph-bot

Loading

abeatrix left a comment •

edited

Loading

novoselrok commented Mar 16, 2023

dominiccooney left a comment

dominiccooney Mar 17, 2023

dominiccooney Mar 17, 2023

dominiccooney Mar 17, 2023

novoselrok Mar 17, 2023

dominiccooney Mar 17, 2023

dominiccooney Mar 17, 2023

novoselrok Mar 17, 2023

dominiccooney Mar 17, 2023

beyang Mar 20, 2023

novoselrok Mar 20, 2023


		this.prompt.addBotMessage(text)
		this.messageInProgress = null

cody: Sourcegraph backend #49514

cody: Sourcegraph backend #49514

Conversation

novoselrok commented Mar 16, 2023 • edited by sourcegraph-bot Loading

Test plan

App preview:

abeatrix left a comment • edited Loading

Choose a reason for hiding this comment

bugs

novoselrok commented Mar 16, 2023

dominiccooney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

novoselrok commented Mar 16, 2023 •

edited by sourcegraph-bot

Loading

abeatrix left a comment •

edited

Loading