Chat: Edit button to rename history chats #1818

deepak2431 · 2023-11-19T20:09:15Z

This PR closes #1763.

An edit button has been added to rename the chat from the history tree view.

Test plan

REC-20231120164044.mp4

REC-20231120164313.mp4

deepak2431 · 2023-11-20T11:24:56Z

Thanks @philipp-spiess for adding the reviewers :)
Mentioning @abeatrix @kalanchan too to take a look.

kalanchan · 2023-11-20T18:15:42Z

the renaming on the side bar looks thank you!

@deepak2431 could you also propagate the name change to the Cody chat panel as well?

CleanShot.2023-11-20.at.10.13.41.mp4

deepak2431 · 2023-11-20T19:03:40Z

Thanks for the review, @kalanchan.
Sure, I will make those changes.

abeatrix · 2023-11-20T21:54:13Z

@deepak2431 is it possible to add a test for the new change as well? 👀

toolmantim

Thanks @deepak2431, this is a good idea! Left some comments on labels and icons.

Non-blocking q's:

Is it possible to add the "Rename Chat" action to the right click context menu on the chat tab too? So you can do it from the chat view, or from the treeview.
Does the treeview provider allow you to rename it directly in the treeview, same as renaming files?

toolmantim · 2023-11-21T03:31:35Z

vscode/package.json

+      {
+        "command": "cody.chat.history.edit",
+        "category": "Cody",
+        "title": "Edit Chat",


Suggested change

"title": "Edit Chat",

"title": "Rename Chat",

toolmantim · 2023-11-21T03:32:05Z

vscode/package.json

+        "category": "Cody",
+        "title": "Edit Chat",
+        "group": "Cody",
+        "icon": "$(wand)",


Suggested change

"icon": "$(wand)",

"icon": "$(edit)",

toolmantim · 2023-11-21T03:40:22Z

vscode/src/chat/chat-view/ChatPanelsManager.ts

@@ -165,6 +165,15 @@ export class ChatPanelsManager implements vscode.Disposable {
        }
    }

+    public async editChatHistory(chatID: string): Promise<void> {
+        await vscode.window.showInputBox({ prompt: 'Enter new chat history name' }).then(async message => {


Suggested change

await vscode.window.showInputBox({ prompt: 'Enter new chat history name' }).then(async message => {

await vscode.window.showInputBox({ prompt: 'Enter new chat name' }).then(async message => {

Could we also pass in the current title as value?

Yes, we can pass that @toolmantim. I will implement this.

deepak2431 · 2023-11-21T06:23:18Z

@deepak2431 is it possible to add a test for the new change as well? 👀

@abeatrix Yeah, I think it would be possible to add tests. I will add one for this.

deepak2431 · 2023-11-21T06:33:54Z

@toolmantim Thanks for the review! I will take care of the changes you suggested.

Is it possible to add the "Rename Chat" action to the right-click context menu on the chat tab too? So you can do it from the chat view or the treeview.

This would be great to have. Yeah, it would be possible to display the Edit button there too.

Does the treeview provider allow you to rename it directly in the treeview, same as renaming files?

Based on my findings, I couldn't see in the docs that Treeview provider allows to directly rename the rendered tree data view title. But I think there might be some other workaround to do this using WebView may be. Would ask @abeatrix to add her views on this too!

abeatrix · 2023-11-21T07:20:16Z

@deepak2431 i think you're right, from what I've gathered that's currently not supported.

There is also a GitHub issue opened for this feature request.

deepak2431 · 2023-11-21T14:11:37Z

Some unexpected Git issues happened while resolving merge conflicts. Working to fix it!

deepak2431 · 2023-11-21T14:19:37Z

All good to review now!
@toolmantim Made the minor changes you suggested.
@kalanchan The edit changes are now propagated to the Chat Panel.

deepak2431 · 2023-11-21T14:51:47Z

@deepak2431 is it possible to add a test for the new change as well? 👀

@abeatrix Yeah, I think it would be possible to add tests. I will add one for this.

@abeatrix I can see that currently, there's no test suite for the new ChatPanel provider where I could add the test for this change. So, I'm not sure about adding one :).
Let me know if I am missing anything!

kalanchan

nice this is great thank you @deepak2431!! MVP!!

CleanShot.2023-11-21.at.15.37.24.2.mp4

abeatrix · 2023-11-21T23:48:58Z

@deepak2431 There should be one now in main :)

…ename-chat

toolmantim

🙌🏼

deepak2431 · 2023-11-22T10:57:50Z

@deepak2431 There should be one now in main :)

Yes, I can see that now. It's been a while since I wrote e2e test, so I just wanted to ask after you run pnpm test:e2e --debug. After that, how do you start the record? I am trying to record it, but the selectors don't appear when I am in the Chat or try to sign in using the server URL and token manually.
It would be great if you could share any docs I can reference or a short loom :)

Edit: Got it working now!

deepak2431 · 2023-11-22T12:59:45Z

@abeatrix I have added the test, but it's getting stuck while filling the input box. I checked it a few times but couldn't see what's wrong there. Can you please have a look?

abeatrix · 2023-11-22T16:52:13Z

@deepak2431 Thanks! Will take a look at the test and let you know if i found anything suspicious!

abeatrix · 2023-11-22T18:15:14Z

vscode/src/services/treeViewItems.ts

@@ -57,6 +58,19 @@ export function createCodyChatTreeItems(userHistory: UserLocalHistory): CodySide
    return chatTreeItems.reverse()
 }

+export async function updateChatHistoryLastInteractionMessage(chatID: string, message: string): Promise<void> {


I dont think this is the expected behavior where renaming the conversation would change the actual chat history. I believe this should work as the rename feature in chat GPT

I might be wrong here, but as we are using the last interaction human message as a treeview history label, we are just updating that here, nothing to the assistant response. So, that's what I thought of as a solution to this.

Another solution I tried was to store the history treeview in the localStorage and then make the update or render the view accordingly. Still, in that case, I thought it was like repeating the same thing we are doing with the chatHistory, so I switched to this solution.

Let me know WDYT. If there's any other solution which we can use here, I am open to hearing about it :)

Hmm, I see the issue here as when the conversation is clicked to view again, the last interaction message is also updated to the updated title.

One more solution I can think of is that instead of updating the chat history, we can keep a map of <chatID, updatedTitle> in the local history, and when we are creating the codyTreeItems or rendering the Chat panel, just check here first, if it is defined then use the updated title else use the last human message. WDYT @abeatrix?

Hmmm instead of storing them separately, what do you think about adding a new title field (maybe optional?) for Transcript / TranscriptJSON so we can store everything together?

I tok

So, I tried this today to use an optional field under TransScript JSON, but there are a few issues I am seeing while implementing this solution:

The history gets updated after each transcript response update, and so does the chats history, which undefine the value of chatTitle.

When the response is aborted, the notifyTurnComplete function resets the title again to undefined.

During session restore, we create a new session ID in that case again chat title is reset to undefined.

Sorry for all the back and forth 😢

I took a look at the PR and I think the issues you mentioned above is due to the old chat history setup, so you shouldn't run into the same issue with the new ChatHistoryManager. With that, we should be able to set the optional chatTitle per Transcript (like chatModel in Transcript) and have everything in one place! Does that sound good to you? Or do you think storing them separately is more ideal?

@abeatrix No problem. This PR took longer than expected, I think, due to a different history setup and chat manager :)
So, I saw the new ChatHistoryManager. But using that still makes it undefined, and I am a bit unsure how this new SimpleChatPanelProvider replaces the old one. It would be great if you could brief me about it.

For you to have a look, I have updated this PR with the implemented solution of using chatTitle with the Transcript. And I agree this is a good way to do it, too.

I guess this solution works fine with SimpleChatContext enabled 👀

So, I saw the new ChatHistoryManager. But using that still makes it undefined, and I am a bit unsure how this new SimpleChatPanelProvider replaces the old one. It would be great if you could brief me about it.

Awesome! It's looking good! And yea sorry for all the massive changes, I also find it confusing 😅 But ultimately we str aiming to retire the current chatPanelProvider and move to SimpleChatPanelProvider. Instead of adding a separate function to handle chat title in history manager, I was thinking of storing it in Transcript. Here is an example of what i've envisioned: #1997 --the change in my PR doesn't work correctly as renaming the chat would generate a new chat history item because of how we are still using the old way to fetch and store history, but I hope this gives you an idea on how to get it to work with the new chat history manager and SimpleChatPanelProvider! What do you think about this approach?

Thanks for taking a look, @abeatrix. I looked over your referenced draft PR, and I got it now that, what was your suggested direction for the solution when you mentioned saving everything at once with the Transcript. This is a good approach, and it can help later too if we need to extend the Chat History Title somewhere. For example, in #1692, it would be helpful.

I will update this PR soon by making those changes :)

…ename-chat

toolmantim · 2023-11-29T08:18:36Z

@abeatrix @deepak2431 yeah, we wouldn't want to alter the chat transcript. A new property sounds right.

deepak2431 · 2023-12-04T17:55:37Z

@abeatrix I am waiting for this #2059 to merge so I can update my changes according to the updated History Manager.

Also, some build errors are happening in the main branch. Can you look into that, please?

deepak2431 · 2023-12-05T07:20:17Z

@abeatrix Updated as per the discussion. It works all good now. I saw that the new history ui test had been disabled for some reason, so I didn't update that for now.

Also, I saw some issues unrelated to this PR, such as how the chatID is being created for new chat in the Treeview and when the chat is revived from the History. So, I would ask you to look in your free time. You might also get a sense of it while reviewing this PR :)

abeatrix · 2023-12-07T12:29:42Z

@sourcegraph/cody-vscode I didn't get a chance to review this before my PTO, would appreciate some eyes on this to unblock 🙇‍♀️

deepak2431 · 2023-12-08T15:01:39Z

@philipp-spiess Can you have a look at it please whenever you have some time?

philipp-spiess

LGTM! One comment inline; let me try to run the tests

philipp-spiess · 2023-12-11T16:11:32Z

vscode/src/chat/MessageProvider.ts

@@ -100,6 +100,7 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
    protected platform: Pick<PlatformContext, 'recipes'>

    protected chatModel: string | undefined = undefined
+    protected chatTitle: string | undefined = 'init'


This looks like a strange default, should we use "Untitled" instead?

yeah, agree with you. Let me make the change.

…into deepak/rename-chat

deepak2431 · 2023-12-11T16:40:34Z

@philipp-spiess Thanks for the review. Just made the suggested changes. Do have a look :)

changes adressed

philipp-spiess · 2023-12-11T18:45:48Z

@deepak2431 Awesome stuff as usual :)

edit chat history

2daf216

philipp-spiess requested a review from a team November 20, 2023 11:19

deepak2431 marked this pull request as ready for review November 20, 2023 11:19

philipp-spiess requested a review from toolmantim November 20, 2023 11:19

add changelog

5fe90d0

abeatrix mentioned this pull request Nov 20, 2023

Users can rename chats #1763

Closed

abeatrix requested a review from kalanchan November 20, 2023 15:49

toolmantim reviewed Nov 21, 2023

View reviewed changes

propogate change to chat panel, small fixes

e611d34

deepak2431 requested a review from toolmantim November 21, 2023 14:04

fix conflicts

399f57a

kalanchan requested a review from abeatrix November 21, 2023 19:34

kalanchan approved these changes Nov 21, 2023

View reviewed changes

Merge branch 'main' of github-personal:sourcegraph/cody into deepak/r…

0f82fe6

…ename-chat

toolmantim approved these changes Nov 22, 2023

View reviewed changes

add test

392220f

abeatrix reviewed Nov 22, 2023

View reviewed changes

deepak2431 added 3 commits November 23, 2023 14:17

store title in transcript

7b8e1ff

Merge branch 'main' of github-personal:sourcegraph/cody into deepak/r…

172c719

…ename-chat

use chat history manager for title update

5291b83

deepak2431 added 2 commits December 2, 2023 18:39

fix merge conflicts

eafa407

handle chat title under transcript

4fa2658

deepak2431 added 4 commits December 5, 2023 00:19

fix merge conflicts

3d1d69b

use new chat history

b8c98df

minor fixes

151afe4

fix conflicts

dfe8633

deepak2431 requested a review from abeatrix December 5, 2023 07:17

deepak2431 added 2 commits December 6, 2023 19:08

fix conflicts

e671c72

make work with simple chat context

b0c2d26

deepak2431 added 2 commits December 7, 2023 19:22

fix merge conflicts

725e1d3

minor fix

fa6df0e

philipp-spiess approved these changes Dec 11, 2023

View reviewed changes

philipp-spiess and others added 3 commits December 11, 2023 17:27

Fix formatting

714192c

update default value

5165b62

Merge branch 'deepak/rename-chat' of github-personal:deepak2431/cody …

5cf8b2b

…into deepak/rename-chat

philipp-spiess merged commit f22d229 into sourcegraph:main Dec 11, 2023
24 of 26 checks passed

deepak2431 deleted the deepak/rename-chat branch December 11, 2023 19:05

deepak2431 mentioned this pull request Dec 13, 2023

Implement history quick pick #2250

Merged

	await vscode.window.showInputBox({ prompt: 'Enter new chat history name' }).then(async message => {
	await vscode.window.showInputBox({ prompt: 'Enter new chat name' }).then(async message => {

Chat: Edit button to rename history chats #1818

Chat: Edit button to rename history chats #1818

Conversation

deepak2431 commented Nov 19, 2023 • edited

Test plan

deepak2431 commented Nov 20, 2023

kalanchan commented Nov 20, 2023

deepak2431 commented Nov 20, 2023

abeatrix commented Nov 20, 2023

toolmantim left a comment • 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

deepak2431 commented Nov 21, 2023

deepak2431 commented Nov 21, 2023

abeatrix commented Nov 21, 2023

deepak2431 commented Nov 21, 2023

deepak2431 commented Nov 21, 2023

deepak2431 commented Nov 21, 2023

kalanchan left a comment

Choose a reason for hiding this comment

abeatrix commented Nov 21, 2023

toolmantim left a comment

Choose a reason for hiding this comment

deepak2431 commented Nov 22, 2023 • edited

deepak2431 commented Nov 22, 2023

abeatrix commented Nov 22, 2023

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

toolmantim commented Nov 29, 2023

deepak2431 commented Dec 4, 2023

deepak2431 commented Dec 5, 2023 • edited

abeatrix commented Dec 7, 2023

deepak2431 commented Dec 8, 2023

philipp-spiess left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepak2431 commented Dec 11, 2023

philipp-spiess commented Dec 11, 2023

deepak2431 commented Nov 19, 2023 •

edited

toolmantim left a comment •

edited

deepak2431 commented Nov 22, 2023 •

edited

deepak2431 commented Dec 5, 2023 •

edited