Add export action #1096

mkondratek · 2024-03-15T14:19:03Z

Fixes #405.

verify against use rich editor for chat prompt editor cody#3287 or later
chatRestore instead of createFromState
restore a single chat
update cody commit
test on windows

Related PR to the agent: sourcegraph/cody#3487

---	---

Test plan

tbd

mkondratek · 2024-03-21T11:04:32Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+        if (result != null) {
+          onSuccess.invoke(result)
+        } else {
+          throw Error("Request timed out") // todo: handle it


@pkukielka, this is a scenario when we time out. Do you have any suggestions how we should handle that? 🙏

We are timing out because of .completeOnTimeout(null, 15, TimeUnit.SECONDS) or something else? I do not think we should have restrictive timeout there. If we are concerned about some arbitrary big number of chat being exported (e.g. > 1000) we can check that at the very start and notify user that e.g. only last 1000 chats will be exported.

I have not encountered the timeout. I can imagine that someone can lose their connection during the export.

If we are considering some constraints we may need to find some more sophisticated way to determine the limit. One can have a single chat with 2000 messages while the other can have 1000 chats with only two messages each (human question, assistant response).

I just tried with a chat that has over 5000 messages (the number of assistant messages) - it fails 🔴

2024-03-21 16:42:32,131 [ 342766] WARN - #c.i.d.PerformanceWatcherImpl - UI was frozen for 5191ms, details saved to /Users/mkondratek/IdeaProjects/jetbrains/build/idea-sandbox/system/log/threadDumps-freeze-20240321-164231-IC-221.5080.210-5sec java.lang.Throwable: Invocation timed out (1sec) at java.desktop/sun.lwawt.macosx.LWCToolkit.invokeAndWait(LWCToolkit.java:818) at java.desktop/sun.lwawt.macosx.LWCToolkit.invokeAndWait(LWCToolkit.java:664) at java.desktop/sun.lwawt.macosx.CAccessibility.invokeAndWait(CAccessibility.java:114) at java.desktop/sun.lwawt.macosx.CAccessibility.invokeAndWait(CAccessibility.java:108) at java.desktop/sun.lwawt.macosx.CAccessibility.getFocusOwner(CAccessibility.java:568) java.lang.OutOfMemoryError: Java heap space java.lang.OutOfMemoryError: Java heap space java.lang.OutOfMemoryError: Java heap space java.lang.OutOfMemoryError: Java heap space java.lang.Throwable: Invocation timed out (1sec) at java.desktop/sun.lwawt.macosx.LWCToolkit.invokeAndWait(LWCToolkit.java:818) at java.desktop/sun.lwawt.macosx.LWCToolkit.invokeAndWait(LWCToolkit.java:664) at java.desktop/sun.lwawt.macosx.CAccessibility.invokeAndWait(CAccessibility.java:114) at java.desktop/sun.lwawt.macosx.CAccessibility.invokeAndWait(CAccessibility.java:108) at java.desktop/sun.lwawt.macosx.CAccessibility.getFocusOwner(CAccessibility.java:568) Exception in thread "HttpClient-1-SelectorManager" java.lang.OutOfMemoryError: Java heap space

I tried with ~1500 messages (the number of assistant messages) - it works (I was able to export) but IntelliJ complains about running low on memory.

I tried with 1000 messages (the number of assistant messages) - it works - no complaints.

mkondratek · 2024-03-21T11:13:49Z

We found one bug btw - our restored chats have lastInteractionTimestamp as the chat id we assign to the chats (e.g. 0c912af5-f2a9-4d60-b8b0-3a75e472a1da). While VS Code generates and export chats with a "proper" timestamp (e.g. Tue, 27 Feb 2024 23:55:50 GMT).

(Issue: #1152)

pkukielka · 2024-03-21T11:34:52Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+            .filter { it.messages.isNotEmpty() }
+
+    chats.forEachIndexed { index, chatState ->
+      AgentChatSessionService.getInstance(project).getOrCreateFromState(chatState)


TBH I do not like that approach although I know it's easiest from the perspective of the code reuse.
We are creating there a lot more than we need, including all UI panels, fetching LLM models, and what not, while all we really want is to call agent.server.chatRestore. Maybe we should refactor that bit of logic out and reuse (it will be very close to existing AgentChatSession::restoreAgentSession method, no)?

What's the purpose of reviving the chat here? chat/export below gives you all the chats in local history for the authed account.

Some account may not be available in the local history in the agent. We need to restore them from our persistency state so the agent is aware of them.

pkukielka · 2024-03-21T11:42:01Z

src/main/kotlin/com/sourcegraph/cody/chat/actions/ExportChatsAction.kt

+                }
+              }
+            },
+            onFinished = { isRunning = false })


isRunning is accessed from multiple threads? Please mark it as volatile.

it's always AWT

pkukielka · 2024-03-21T11:43:44Z

src/main/kotlin/com/sourcegraph/cody/chat/actions/ExportChatsAction.kt

+              val json = gson.toJson(chatHistory)
+              invokeLater {
+                WriteAction.run<RuntimeException> {
+                  saveTextToFile(json.toByteArray(), result.file)


Use getVirtualFile instead of file, so you don't need to convert it back and forth risking conversion issues.

good point - thanks

mkondratek · 2024-03-21T16:52:21Z

I ve just found some designs (#405) 😅

src/main/kotlin/com/sourcegraph/cody/agent/CodyAgentServer.kt

dominiccooney · 2024-03-22T17:17:44Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+            .filter { it.messages.isNotEmpty() }
+
+    chats.forEachIndexed { index, chatState ->
+      AgentChatSessionService.getInstance(project).getOrCreateFromState(chatState)


What's the purpose of reviving the chat here? chat/export below gives you all the chats in local history for the authed account.

pkukielka · 2024-03-26T10:38:51Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+            .filter { it.internalId != null }
+            .filter { chat -> if (internalId != null) chat.internalId == internalId else true }
+
+    AgentChatSession.createNew(project) { _ ->


Why do you even need that call? You are not using connectionId generated by it in any way.

whoops - that are some leftover - good point, thanks

pkukielka · 2024-03-26T10:40:05Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+          }
+        }
+
+        val result = agent.server.chatExport().completeOnTimeout(null, 15, TimeUnit.SECONDS).get()


I do not like that random timeout values in the code.
Maybe we can use one default in the agent itself, or if we need a few tiers just have one object which defines them.

I do not understand what do you mean by default. I tried .getNow(null) but it gives null.

pkukielka · 2024-03-26T10:42:16Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+            if (singleChatHistory != null) {
+              onSuccess.invoke(singleChatHistory)
+            } else {
+              // todo: handle error


TODO in the code

timeout error

@danielmarquespt, can you pls take a look? 🙏

Note that it is a "tool window" notification (the balloon points to the tool window tab - Cody, it's less aggressive than the usual notifications)

pkukielka · 2024-03-26T10:47:52Z

src/main/kotlin/com/sourcegraph/cody/chat/actions/ExportChatsAction.kt

+                }
+              }
+            },
+            onFinished = { isRunning = false })


I do not like passing state variable as general purpose callback.
We already have domain purpose class exactly for what you need there: CancellationToken

This is a minor refactor needed for #1096. It extracts the chat restore logic so it is doable to send the restore request without restoring the full session (with panels, etc). ## Test plan 1. Restore works properly

pkukielka · 2024-03-27T14:38:33Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+      restoreChatSession(agent, chatState)
+      indicator.fraction = ((index + 1.0) / (chats.size + 1.0))
+
+      Thread.sleep(3000)


What is that? :O

whoops, sorry - that was for testing only

pkukielka · 2024-03-27T14:40:48Z

src/main/kotlin/com/sourcegraph/cody/chat/ExportChatsBackgroundable.kt

+    private val agent: CodyAgent,
+    private val internalId: String?,
+    private val onSuccess: (Any) -> Unit,
+    private val cancellationToken: CancellationToken


Thank you for changing it to use CancellationToken!

pkukielka · 2024-03-27T14:44:41Z

src/main/kotlin/com/sourcegraph/cody/chat/actions/ExportChatsAction.kt

+
+  companion object {
+    val gson: Gson = GsonBuilder().create()
+    var isRunning = AtomicReference(false)


I see what you mean talking about AtomicReference.
It seems that if you need to check it externally there really is no easier way (although that probably could be just volatile boolean).

mkondratek · 2024-03-27T17:17:20Z

tested on windows - works

pkukielka

LGTM

mkondratek self-assigned this Mar 15, 2024

mkondratek marked this pull request as draft March 15, 2024 14:19

mkondratek force-pushed the mkondratek/feat/chat-export branch 2 times, most recently from a7e31c9 to 51414fb Compare March 21, 2024 00:32

mkondratek mentioned this pull request Mar 21, 2024

Add chatExport endpoint & tests sourcegraph/cody#3487

Merged

mkondratek marked this pull request as ready for review March 21, 2024 11:03

mkondratek commented Mar 21, 2024

View reviewed changes

mkondratek force-pushed the mkondratek/feat/chat-export branch 2 times, most recently from 1d29096 to 0087a31 Compare March 21, 2024 11:08

mkondratek mentioned this pull request Mar 21, 2024

bug: lastInteractionTimestamp persistence after restore #1152

Closed

pkukielka reviewed Mar 21, 2024

View reviewed changes

dominiccooney reviewed Mar 22, 2024

View reviewed changes

mkondratek force-pushed the mkondratek/feat/chat-export branch from 316b04d to 2143d6f Compare March 25, 2024 23:09

mkondratek changed the base branch from main to mkondratek/chore/restore March 25, 2024 23:10

mkondratek mentioned this pull request Mar 25, 2024

Refactor restore logic #1178

Merged

mkondratek force-pushed the mkondratek/chore/restore branch from 6d78a71 to 6e34e48 Compare March 25, 2024 23:15

mkondratek force-pushed the mkondratek/feat/chat-export branch from 2143d6f to ed9df6a Compare March 25, 2024 23:15

pkukielka reviewed Mar 26, 2024

View reviewed changes

Base automatically changed from mkondratek/chore/restore to main March 26, 2024 15:21

mkondratek force-pushed the mkondratek/feat/chat-export branch 2 times, most recently from a3893cb to a020e25 Compare March 27, 2024 14:34

pkukielka reviewed Mar 27, 2024

View reviewed changes

mkondratek added 17 commits March 27, 2024 19:34

Add export action

272a1fc

Fixes

2a69fb7

wip

92d31fe

fixes

36ddd07

wip

294b7fc

wip

fdd9c3d

Use the new endpoint

8098c2e

Show file save dialog before running the export

4fee9a6

Fixes

0541c58

Fixes

1dd9e4a

Fixes

d640b54

Fixes

8da04c7

Fixes

9b05546

Add error handling

5fecdf3

Fixes

d8bbc9a

Fixes

dc72859

fix

92e9999

mkondratek force-pushed the mkondratek/feat/chat-export branch from 52c833a to 92e9999 Compare March 27, 2024 18:35

fix

77debe0

pkukielka approved these changes Mar 28, 2024

View reviewed changes

Update cody commit

903c5ce

mkondratek enabled auto-merge (squash) March 28, 2024 13:29

mkondratek disabled auto-merge March 28, 2024 13:33

Minor fixes

6465972

mkondratek enabled auto-merge (squash) March 28, 2024 13:46

mkondratek merged commit e94ce34 into main Mar 28, 2024
5 checks passed

mkondratek deleted the mkondratek/feat/chat-export branch March 28, 2024 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add export action #1096

Add export action #1096

mkondratek commented Mar 15, 2024 •

edited

mkondratek Mar 21, 2024

pkukielka Mar 21, 2024 •

edited

mkondratek Mar 21, 2024

mkondratek Mar 21, 2024

mkondratek commented Mar 21, 2024 •

edited

pkukielka Mar 21, 2024 •

edited

dominiccooney Mar 22, 2024

mkondratek Mar 22, 2024

pkukielka Mar 21, 2024

mkondratek Mar 21, 2024

pkukielka Mar 21, 2024

mkondratek Mar 21, 2024

mkondratek commented Mar 21, 2024

dominiccooney Mar 22, 2024

pkukielka Mar 26, 2024

mkondratek Mar 26, 2024

pkukielka Mar 26, 2024

mkondratek Mar 26, 2024

pkukielka Mar 26, 2024

mkondratek Mar 26, 2024

mkondratek Mar 26, 2024

pkukielka Mar 26, 2024

pkukielka Mar 27, 2024

mkondratek Mar 27, 2024

pkukielka Mar 27, 2024

pkukielka Mar 27, 2024

mkondratek commented Mar 27, 2024

pkukielka left a comment

Add export action #1096

Add export action #1096

Conversation

mkondratek commented Mar 15, 2024 • edited

Test plan

Choose a reason for hiding this comment

pkukielka Mar 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkondratek commented Mar 21, 2024 • edited

pkukielka Mar 21, 2024 • edited

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

mkondratek commented Mar 21, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkondratek commented Mar 27, 2024

pkukielka left a comment

Choose a reason for hiding this comment

mkondratek commented Mar 15, 2024 •

edited

pkukielka Mar 21, 2024 •

edited

mkondratek commented Mar 21, 2024 •

edited

pkukielka Mar 21, 2024 •

edited