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

Implement lenses using codeVision package #2318

Merged
merged 8 commits into from
Sep 23, 2024

Conversation

pkukielka
Copy link
Contributor

@pkukielka pkukielka commented Sep 18, 2024

Fixes CODY-3743

Changes

New implementation of code lenses using IntelliJ codeVision package.

image

Unfortunately the way JetBrains implemented codeVision providers is such that only one item of a given provider can be displayed per line. That forced me to create distinct provider for every code lens action we want to support. That said it's not a bit problem, I made sure that all of them have shared configuration:

image
image

Test plan

Full QA of all edit commands is required.

@pkukielka pkukielka marked this pull request as draft September 18, 2024 14:15
@pkukielka pkukielka changed the title Initial implementation of lenses using codeVision Implement lenses using codeVision package Sep 18, 2024
@pkukielka pkukielka force-pushed the pkukielka/migrate-leneses-to-codeVision branch from fdfe93e to 7617f0c Compare September 20, 2024 10:48
@pkukielka pkukielka marked this pull request as ready for review September 20, 2024 10:56
@pkukielka pkukielka force-pushed the pkukielka/migrate-leneses-to-codeVision branch from 6c3e55a to c17ea15 Compare September 20, 2024 11:16
@pkukielka pkukielka force-pushed the pkukielka/migrate-leneses-to-codeVision branch from c17ea15 to f5a7aa8 Compare September 20, 2024 11:30
hasLensAppeared
}
}
fun runAndWaitForCleanState(actionIdToRun: String) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need runAndWaitForCleanState? we could use runAndWaitForLenses directly

Copy link
Contributor Author

@pkukielka pkukielka Sep 23, 2024

Choose a reason for hiding this comment

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

We could, but it is not obvious that runAndWaitForLenses(actionIdToRun) wait for no lenses to be visible anymore.
Or maybe I should change name of this function?

@@ -21,93 +19,65 @@ import org.junit.runner.RunWith
@RunWith(CustomJunitClassRunner::class)
class DocumentCodeTest : CodyIntegrationTextFixture() {

@Test
@Ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

so it still fails, right?

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 think the way it is written there is no way it could be stable, as there is race in the test itself.
I will try to rewrite it a bit.

@mkondratek
Copy link
Contributor

I encountered this one:

Stacktrace:

com.google.gson.JsonParseException: Unknown enum value: 2023-06-01
	at com.sourcegraph.cody.agent.EnumTypeAdapter.read(EnumTypeAdapterFactory.kt:55)
	at com.sourcegraph.cody.agent.EnumTypeAdapter.read(EnumTypeAdapterFactory.kt:22)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$2.readIntoField(ReflectiveTypeAdapterFactory.java:267)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$FieldReflectionAdapter.readField(ReflectiveTypeAdapterFactory.java:558)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:516)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$2.readIntoField(ReflectiveTypeAdapterFactory.java:267)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$FieldReflectionAdapter.readField(ReflectiveTypeAdapterFactory.java:558)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:516)
	at org.eclipse.lsp4j.jsonrpc.json.adapters.CollectionTypeAdapter.read(CollectionTypeAdapter.java:114)
	at org.eclipse.lsp4j.jsonrpc.json.adapters.CollectionTypeAdapter.read(CollectionTypeAdapter.java:40)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$2.readIntoField(ReflectiveTypeAdapterFactory.java:267)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$FieldReflectionAdapter.readField(ReflectiveTypeAdapterFactory.java:558)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:516)
	at com.google.gson.Gson.fromJson(Gson.java:1361)
	at com.google.gson.Gson.fromJson(Gson.java:1306)
	at org.eclipse.lsp4j.jsonrpc.json.adapters.MessageTypeAdapter.fromJson(MessageTypeAdapter.java:343)
	at org.eclipse.lsp4j.jsonrpc.json.adapters.MessageTypeAdapter.parseResult(MessageTypeAdapter.java:188)
	at org.eclipse.lsp4j.jsonrpc.json.adapters.MessageTypeAdapter.read(MessageTypeAdapter.java:124)
	at org.eclipse.lsp4j.jsonrpc.json.adapters.MessageTypeAdapter.read(MessageTypeAdapter.java:56)
	at com.google.gson.Gson.fromJson(Gson.java:1361)
	at com.google.gson.Gson.fromJson(Gson.java:1306)
	at org.eclipse.lsp4j.jsonrpc.json.MessageJsonHandler.parseMessage(MessageJsonHandler.java:121)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:184)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:97)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:114)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

@pkukielka pkukielka force-pushed the pkukielka/migrate-leneses-to-codeVision branch from c956fc2 to b41fd1b Compare September 23, 2024 09:24
@@ -24,4 +24,4 @@ kotlin.daemon.jvmargs=-Xmx2g -Xms500m
nodeBinaries.commit=8755ae4c05fd476cd23f2972049111ba436c86d4
nodeBinaries.version=v20.12.2
cody.autocomplete.enableFormatting=true
cody.commit=daa1693620fdf2784ecd190dd12bb07169950a90
cody.commit=5bd253c16261680490d2afb14398ff568334ffbd
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to update the cody commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not needed.
I had some problems which I thought were related to bugs in cody main but it turned out to not be the case.
I reverted this change for now.

But we should update it. I will do it in a separate PR.

@pkukielka pkukielka force-pushed the pkukielka/migrate-leneses-to-codeVision branch from b41fd1b to 48e9346 Compare September 23, 2024 10:41
@mkondratek
Copy link
Contributor

Enterprise account signed in, there is an error:

[Trace - 01:06:18 PM] Received response 'editCommands/code - (17)' in 341ms
Result: {
  "id": "mewjjl",
  "state": "Error",
  "error": {
    "message": "Request to https://sourcegraph.sourcegraph.com/.api/completions/stream?api-version\u003d2\u0026client-name\u003djetbrains\u0026client-version\u003d6.0-localbuild failed with 400 Bad Request: unsupported chat model \"sourcegraph/claude-3.5-sonnet\" (default \"anthropic::2023-06-01::claude-3.5-sonnet\")\n",
    "cause": null,
    "stack": "Error: Request to https://sourcegraph.sourcegraph.com/.api/completions/stream?api-version\u003d2\u0026client-name\u003djetbrains\u0026client-version\u003d6.0-localbuild failed with 400 Bad Request: unsupported chat model \"sourcegraph/claude-3.5-sonnet\" (default \"anthropic::2023-06-01::claude-3.5-sonnet\")\n\n    at IncomingMessage.\u003canonymous\u003e (/Users/mkondratek/IdeaProjects/jetbrains/build/sourcegraph/agent/index.js:199937:21)\n    at IncomingMessage.emit (node:events:530:35)\n    at IncomingMessage.emit (node:domain:488:12)\n    at endReadableNT (node:internal/streams/readable:1696:12)\n    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)"
  },
  "selectionRange": {
    "start": {
      "line": 2,
      "character": 0
    },
    "end": {
      "line": 3,
      "character": 0
    }
  },
  "instruction": "rename id to name",
  "model": "sourcegraph/claude-3.5-sonnet",
  "originalText": "data class WebviewCreateParams(val id: String)\n"
}
Error: null

@mkondratek
Copy link
Contributor

it looks like the model / apiversion problems are resolved in the latest main in cody

@mkondratek
Copy link
Contributor

however, it looks like the lenses does not work while indexing. maybe we should simple NOT make any lens-using action dumb aware?

@pkukielka pkukielka force-pushed the pkukielka/migrate-leneses-to-codeVision branch from 6d3150c to 529d880 Compare September 23, 2024 11:33
@pkukielka
Copy link
Contributor Author

however, it looks like the lenses does not work while indexing. maybe we should simple NOT make any lens-using action dumb aware?

It is other way around, DumbAware let things run when dumb mode is on.
https://github.com/JetBrains/intellij-community/blob/master/platform/core-api/src/com/intellij/openapi/project/DumbAware.java

But in our case problem are not the actions but CodeVisionProvider.
There is no way to explicitly make it dumb aware, but I added DumbAware to our class signature:
abstract class EditCodeVisionProvider(private val metadata: EditCodeVisionProviderMetadata) : CodeVisionProvider<Unit>, DumbAware
That said it does not seem to be always respecting it during indexing... but to some extend it does.
I'm not fully understand why.

Copy link
Contributor

@mkondratek mkondratek left a comment

Choose a reason for hiding this comment

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

lgtm with the latest changes in cody

@pkukielka pkukielka merged commit 94d565b into main Sep 23, 2024
6 of 8 checks passed
@pkukielka pkukielka deleted the pkukielka/migrate-leneses-to-codeVision branch September 23, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants