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

Initial implementation for Inline Edit and Document Code #440

Merged
merged 59 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
67cfdca
initial scaffolding for inline edits
steveyegge Feb 6, 2024
c1dbd0a
some fixes from the last rebase
steveyegge Feb 7, 2024
2e20cdd
enabled workspaceEdit capability so that API works
steveyegge Feb 10, 2024
760430f
started work on workspace/edit modeling
steveyegge Feb 11, 2024
b5b6674
got e2e roundtrip for commands/document working
steveyegge Feb 12, 2024
baaf829
Code lenses are displaying, primordially - needs more work
steveyegge Feb 13, 2024
52e854e
got inline code lens widgets working
steveyegge Feb 14, 2024
dc98524
got inserting generated doc string working
steveyegge Feb 14, 2024
2fcf387
fixed a broken merge
steveyegge Feb 14, 2024
51f9243
more bad merge fixes
steveyegge Feb 14, 2024
2593cb1
fixed another merge problem
steveyegge Feb 14, 2024
52ca409
removed garbage Cody put into the file
steveyegge Feb 14, 2024
c8faca0
code lens widget cosmetic improvements
steveyegge Feb 15, 2024
3a8b828
fixed several bugs with lenses
steveyegge Feb 15, 2024
5016cc3
More work on code lenses, cleaning up dispose logic
steveyegge Feb 18, 2024
c530b2a
fix for mouse rollover in code lenses
steveyegge Feb 18, 2024
90377d6
finally got the disposal chains and state machine fixed
steveyegge Feb 18, 2024
b535c71
added support for Undo
steveyegge Feb 19, 2024
2b69a69
Got Accept, Undo and Cancel working with telemetry
steveyegge Feb 20, 2024
0ca7116
Updated protocol bindings for PR 3219
steveyegge Feb 20, 2024
83c432c
fixed LensWidgetGroup to use new icon protocol
steveyegge Feb 20, 2024
bca7b6c
got lens tooltips and hotkeys displaying
steveyegge Feb 21, 2024
ea69818
got selection ranges working
steveyegge Feb 22, 2024
f6cba1a
fixed command/execute calls
steveyegge Feb 22, 2024
a710304
force JB client to use indentation-based foldingRange
steveyegge Feb 22, 2024
e120cbd
fixed a race-condition exception
steveyegge Feb 22, 2024
5f06a55
a few minor bug fixes
steveyegge Feb 22, 2024
f52d91e
fixed some merge errors
steveyegge Feb 28, 2024
a615714
temporary fix to indent doc strings
steveyegge Mar 11, 2024
bfdf688
fixed merge error
steveyegge Mar 11, 2024
be5f32c
About to embark on dpc's branch with no lenses sent
steveyegge Mar 13, 2024
188d861
Demo scree
dominiccooney Mar 13, 2024
69b9ad7
Started restructuring for Dominic's new protocol
steveyegge Mar 13, 2024
aa5f2e3
more work on multiplexing task ids
steveyegge Mar 13, 2024
c9be1a0
More progress on the new protocol
steveyegge Mar 14, 2024
9e8693a
more work on wiring up lenses
steveyegge Mar 14, 2024
fe445c9
testing
abeatrix Mar 12, 2024
5ff5056
wired up JB side of getFoldingRanges
steveyegge Mar 14, 2024
3dbcdb5
fix for EDT violation getting caret
steveyegge Mar 16, 2024
d360a33
fixes for lens positioning
steveyegge Mar 16, 2024
3c1793a
added a couple utilities
steveyegge Mar 16, 2024
e51b607
better support for concurrent edits
steveyegge Mar 16, 2024
70abd1a
fixed spotless errors
steveyegge Mar 16, 2024
8152863
two fixes for positioning lenses
steveyegge Mar 16, 2024
7a3ba22
scroll to show lenses after displaying them
steveyegge Mar 16, 2024
552687e
fixed ghost text instructions
steveyegge Mar 17, 2024
1bc0937
Got inline-edit happy path working
steveyegge Mar 17, 2024
46a04ce
some comments
steveyegge Mar 17, 2024
a612960
rewrote and fixed all the undo/redo logic
steveyegge Mar 18, 2024
21e437c
some minor refactoring/cleanups
steveyegge Mar 18, 2024
ee40482
added experimental flag for inline edits
steveyegge Mar 18, 2024
4ae13e5
removed an experimental/unused class
steveyegge Mar 18, 2024
4c95564
got edit-instructions ghost text and focus working properly
steveyegge Mar 18, 2024
d25ded4
fixed prompt history
steveyegge Mar 19, 2024
0a143f8
Bug fixes and cleanups suggested in PR 440
steveyegge Mar 20, 2024
1f6360a
Refactoring based on Piotr's suggestions
steveyegge Mar 20, 2024
16f4a34
ran spotless
steveyegge Mar 20, 2024
d2898ae
Implemented a simple feature-flag system for hiding Inline Edits
steveyegge Mar 20, 2024
5fe3844
fixed spotless check
steveyegge Mar 20, 2024
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
# IntelliJ project
*.iml

# User local IDEA run configurations
.run/

# Build output & caches for IntelliJ plugin development
build/
idea-sandbox/
Expand Down
37 changes: 33 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,15 @@ There are two supported configurations for debugging this way:
- The Cody extension connects via socket to the "remote" agent

Option 1 is the simplest, and probably makes the most sense for you
to use if you are uncertain which method to use for debugging.
to use if you are uncertain which method to use for debugging. Option 2
is especially useful when you need to set a breakpoint very early in
the Agent startup.

## How to set up Run Configurations

Run configurations are basically IDEA's launcher scripts. You will need to create one
run configuration in each project window, using Run → Edit Configurations.
Run configurations are basically IDEA's launcher scripts. You will need
to create one run configuration in each project window, using Run → Edit
Configurations.

For both debugging setups (Cody-spawns and JB-spawned), you will need:

Expand Down Expand Up @@ -305,7 +308,7 @@ Create this configuration in the `sourcegraph/cody` project window:
- Working directory: `~/src/sg/cody` (or wherever you cloned sourcegraph/cody)
- JavaScript file: `agent/dist/index.js`
- note: this is a relative path, unlike the run configs for Option 2
- Application Parameters: <leave empty>
- Application Parameters: leave empty
- Environment variables:
- `CODY_AGENT_DEBUG_REMOTE=true`
- `CODY_AGENT_DEBUG_PORT=3113`
Expand Down Expand Up @@ -352,3 +355,29 @@ To set this up, in the Before launch section of any Run/Debug configuration, do
You only need to do most of these steps once, when you create the
External Tool. Then you can use it in any run config that spawns the
agent, to rebuild it first.

## Debugging VS Code

Sometimes, the TypeScript backend behaves differently when called from
IntelliJ via Agent than when the same code is called from the VS Code
extension, and you may need to debug the same code paths through the
Agent twice. First, when called from the JetBrains extension side, and
again when called from the VS Code extension side.

To accomplish the latter, you can use VS Code to debug itself. It works
very similarly to how it works in JetBrains. There are predefined run
configurations for debugging VS Code Cody in the file
`.vscode/launch.json` in `sourcegraph/cody`, such as `Launch VS Code
Extension (Desktop)`.

You do not launch VS Code run configurations from the command palette.
Instead, use ctrl-shift-D to open the Run and Debug view, and you can
see the configuration dropdown at the top.

## Known Issues

- Force-stopping the target often corrupts IntelliJ's project indexes,
requiring an extra restart of the debugged target to fix them.
- Workaround is to exit the target gracefully by quitting each time,
using the menus or hotkeys, rather than force-stopping it.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import com.sourcegraph.cody.agent.CodyAgent;
import com.sourcegraph.cody.agent.CodyAgentCodebase;
import com.sourcegraph.cody.agent.CodyAgentService;
import com.sourcegraph.cody.agent.protocol.TextDocument;
import com.sourcegraph.cody.agent.protocol.ProtocolTextDocument;
import com.sourcegraph.config.ConfigUtil;
import org.jetbrains.annotations.NotNull;

Expand All @@ -34,7 +34,8 @@ public void fileClosed(@NotNull FileEditorManager source, @NotNull VirtualFile f

CodyAgentService.withAgent(
source.getProject(),
agent -> agent.getServer().textDocumentDidClose(TextDocument.fromVirtualFile(file)));
agent ->
agent.getServer().textDocumentDidClose(ProtocolTextDocument.fromVirtualFile(file)));
}

public static void fileOpened(Project project, CodyAgent codyAgent, @NotNull VirtualFile file) {
Expand All @@ -43,7 +44,8 @@ public static void fileOpened(Project project, CodyAgent codyAgent, @NotNull Vir
.runReadAction(
(Computable<Document>) () -> FileDocumentManager.getInstance().getDocument(file));
if (document != null) {
TextDocument textDocument = TextDocument.fromVirtualFile(file, document.getText());
ProtocolTextDocument textDocument =
ProtocolTextDocument.fromVirtualFile(file, document.getText());
codyAgent.getServer().textDocumentDidOpen(textDocument);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import com.intellij.openapi.vfs.VirtualFile;
import com.sourcegraph.cody.agent.CodyAgentCodebase;
import com.sourcegraph.cody.agent.CodyAgentService;
import com.sourcegraph.cody.agent.protocol.TextDocument;
import com.sourcegraph.cody.agent.protocol.ProtocolTextDocument;
import com.sourcegraph.config.ConfigUtil;
import org.jetbrains.annotations.NotNull;

Expand Down Expand Up @@ -54,7 +54,7 @@ public void focusGained(@NotNull Editor editor) {
Thread.sleep(100);
} catch (InterruptedException ignored) {
}
agent.getServer().textDocumentDidFocus(TextDocument.fromVirtualFile(file));
agent.getServer().textDocumentDidFocus(ProtocolTextDocument.fromVirtualFile(file));
});

CodyAgentCodebase.getInstance(project).onFileOpened(project, file);
Expand Down
38 changes: 0 additions & 38 deletions src/main/java/com/sourcegraph/cody/PluginUtil.java

This file was deleted.

111 changes: 98 additions & 13 deletions src/main/java/com/sourcegraph/cody/agent/CodyAgentClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,26 @@
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.editor.Editor;
import com.sourcegraph.cody.agent.protocol.DebugMessage;
import com.sourcegraph.cody.agent.protocol.*;
import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import java.util.function.Supplier;
import org.eclipse.lsp4j.jsonrpc.services.JsonNotification;
import org.eclipse.lsp4j.jsonrpc.services.JsonRequest;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/** Implementation of the client part of the Cody agent protocol. */
/**
* Implementation of the client part of the Cody agent protocol. This class dispatches the requests
* and notifications sent by the agent.
*/
@SuppressWarnings("unused")
public class CodyAgentClient {

private static final Logger logger = Logger.getInstance(CodyAgentClient.class);

@Nullable public Editor editor;

// Callback that is invoked when the agent sends a "chat/updateMessageInProgress" notification.
@Nullable public Consumer<WebviewPostMessageParams> onNewMessage;

Expand All @@ -26,14 +33,92 @@ public class CodyAgentClient {
// onSetConfigFeatures
@Nullable public Consumer<WebviewPostMessageParams> onReceivedWebviewMessage;

@Nullable public Editor editor;
// Callback for the "editTask/didUpdate" notification from the agent.
@Nullable private Consumer<EditTask> onEditTaskDidUpdate;

// Callback for the "editTask/didDelete" notification from the agent.
@Nullable private Consumer<EditTask> onEditTaskDidDelete;

// Callback for the "textDocument/edit" request from the agent.
@Nullable private Consumer<TextDocumentEditParams> onTextDocumentEdit;

// Callback for the "workspace/edit" request from the agent.
@Nullable private Consumer<WorkspaceEditParams> onWorkspaceEdit;

public void setOnEditTaskDidUpdate(@Nullable Consumer<EditTask> callback) {
onEditTaskDidUpdate = callback;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine for now, but longer term is this inviting disaster with overwriting a single callback? Should this throw if the callback is already set, or can the wire-up move to some run once initialization step?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say let's remove setOnEditTaskDidUpdate method and move onEditTaskDidUpdate update code into CodyAgentService::onStartup as other similar callback.
Current code won't work is some scenarios, e.g. after agent restart after a crash or account change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but there are 8 of these callbacks (I think my PR only added one), and we should rearchitect all of them in one go. I've added issue 1136 to track it.

}

public void setOnEditTaskDidDelete(@Nullable Consumer<EditTask> callback) {
onEditTaskDidDelete = callback;
}

@JsonNotification("editTask/didUpdate")
public void editTaskDidUpdate(EditTask params) {
onEventThread(
() -> {
if (onEditTaskDidUpdate != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For brevity, what about providing a default callback which logs, making the callback non-nullable everywhere, and just calling directly here?

onEditTaskDidUpdate.accept(params);
} else {
logger.warn("No callback registered for editTask/didUpdate");
}
return null;
});
}

@JsonNotification("editTask/didDelete")
public void editTaskDidDelete(EditTask params) {
onEventThread(
() -> {
if (onEditTaskDidDelete != null) {
onEditTaskDidDelete.accept(params);
} else {
logger.warn("No callback registered for editTask/didDelete");
}
return null;
});
}

public void setOnTextDocumentEdit(@Nullable Consumer<TextDocumentEditParams> callback) {
onTextDocumentEdit = callback;
}

@JsonRequest("textDocument/edit")
public CompletableFuture<Boolean> textDocumentEdit(TextDocumentEditParams params) {
return onEventThread(
() -> {
if (onTextDocumentEdit != null) {
onTextDocumentEdit.accept(params);
} else {
logger.warn("No callback registered for textDocument/edit");
}
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this change, but I think a lot of our future hopes and dreams hinge on this boolean.

The VSCode edit API can fail, and this boolean indicates the success or failure of the edit. When we have concurrent edits (in the sense that the Agent's model of the document isn't caught up to the JetBrains reality because the user has edited but the extension sends an edit), we could detect that and fail here.

Punchline is that extension code doesn't handle those failures! We would need to have the extension handle that. But there's probably some not terribly burdensome abstraction, a "retryable edit", that could do that. Interested in your thoughts here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, let's do it -- I've seen it fail to apply once (no code returned by LLM) and would love to get more feedback to the user when this happens.

});
}

public void setOnWorkspaceEdit(@Nullable Consumer<WorkspaceEditParams> callback) {
onWorkspaceEdit = callback;
}

@JsonRequest("workspace/edit")
public CompletableFuture<Boolean> workspaceEdit(WorkspaceEditParams params) {
return onEventThread(
() -> {
if (onWorkspaceEdit != null) {
onWorkspaceEdit.accept(params);
} else {
logger.warn("No callback registered for workspace/edit");
}
return true;
});
}

/**
* Helper to run client request/notification handlers on the IntelliJ event thread. Use this
* helper for handlers that require access to the IntelliJ editor, for example to read the text
* contents of the open editor.
*/
private <T> CompletableFuture<T> onEventThread(Supplier<T> handler) {
private <T> @NotNull CompletableFuture<T> onEventThread(Supplier<T> handler) {
CompletableFuture<T> result = new CompletableFuture<>();
ApplicationManager.getApplication()
.invokeLater(
Expand All @@ -47,24 +132,24 @@ private <T> CompletableFuture<T> onEventThread(Supplier<T> handler) {
return result;
}

// Webviews
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious where this comes from, seems unrelated to edits.

Copy link
Contributor

@pkukielka pkukielka Mar 18, 2024

Choose a reason for hiding this comment

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

That is old chat API which was removed since, it should be removed from there.

@JsonRequest("webview/create")
public CompletableFuture<Void> webviewCreate(WebviewCreateParams params) {
logger.error("webview/create This request should not happen if you are using chat/new.");
return CompletableFuture.completedFuture(null);
}

// =============
// Notifications
// =============

@JsonNotification("debug/message")
public void debugMessage(DebugMessage msg) {
public void debugMessage(@NotNull DebugMessage msg) {
logger.warn(String.format("%s: %s", msg.getChannel(), msg.getMessage()));
}

// Webviews
@JsonRequest("webview/create")
public CompletableFuture<Void> webviewCreate(WebviewCreateParams params) {
logger.error("webview/create This request should not happen if you are using chat/new.");
return CompletableFuture.completedFuture(null);
}

@JsonNotification("webview/postMessage")
public void webviewPostMessage(WebviewPostMessageParams params) {
public void webviewPostMessage(@NotNull WebviewPostMessageParams params) {
ExtensionMessage extensionMessage = params.getMessage();

if (onNewMessage != null
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package com.sourcegraph.cody.agent

data class CommandExecuteParams(val command: String, val args: List<String>)
data class CommandExecuteParams(val command: String, val arguments: List<String>)
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void onFinished(Consumer<Boolean> callback) {
try {
callback.accept(isCancelled);
} catch (Exception ignored) {
// Do nothing about exceptions in cancelation callbacks
// Do nothing about exceptions in cancellation callbacks
}
});
}
Expand Down
9 changes: 5 additions & 4 deletions src/main/kotlin/com/sourcegraph/cody/agent/CodyAgent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ private constructor(
private const val DEFAULT_AGENT_DEBUG_PORT = 3113 // Also defined in agent/src/cli/jsonrpc.ts
@JvmField val executorService: ExecutorService = Executors.newCachedThreadPool()

private fun shouldConnectToDebugAgent() = System.getenv("CODY_AGENT_DEBUG_REMOTE") == "true"

private fun shouldSpawnDebuggableAgent() = System.getenv("CODY_AGENT_DEBUG_INSPECT") == "true"

fun create(project: Project): CompletableFuture<CodyAgent> {
Expand All @@ -109,7 +107,10 @@ private constructor(
version = ConfigUtil.getPluginVersion(),
workspaceRootUri =
ConfigUtil.getWorkspaceRootPath(project).toUri().toString(),
extensionConfiguration = ConfigUtil.getAgentConfiguration(project)))
extensionConfiguration = ConfigUtil.getAgentConfiguration(project),
capabilities =
ClientCapabilities(
edit = "enabled", editWorkspace = "enabled", codeLenses = "enabled")))
.thenApply { info ->
logger.info("Connected to Cody agent " + info.name)
server.initialized()
Expand All @@ -126,7 +127,7 @@ private constructor(
}

private fun startAgentProcess(): AgentConnection {
if (shouldConnectToDebugAgent()) {
if (ConfigUtil.shouldConnectToDebugAgent()) {
return connectToDebugAgent()
}
val token = CancellationToken()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class CodyAgentCodebase(val project: Project) {
companion object {
@JvmStatic
fun getInstance(project: Project): CodyAgentCodebase {
return project.service<CodyAgentCodebase>()
return project.service()
}
}
}
Loading
Loading