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

Cody for VS Code not rendering "Read files" in the chat when navigated from history view #52848

Closed
toolmantim opened this issue Jun 2, 2023 · 10 comments

Comments

@toolmantim
Copy link
Contributor

It appears that "Read files" isn't rendered for a historical conversation until a new message is added to a chat:

read-files-not-rendering-chat-bug.mov

Steps to reproduce

  1. Start a chat that reads some files
  2. Start a new chat
  3. Go to the history view, and load your first chat
  4. See the "Read files" is missing
  5. Send Cody a new message on that chat
  6. See the "Read files" has re-appeared on the previous message
@deepak2431
Copy link
Contributor

@toolmantim I was just looking into this issue. I found one more thing that when we click over any history content which is being rendered in the ChatView it disappears from the history panel. Is this the expected behaviour?

@abeatrix
Copy link
Contributor

abeatrix commented Jun 2, 2023

Is this the expected behaviour?

If it is expected, I don't like it 😞

@deepak2431
Copy link
Contributor

Is this the expected behaviour?

If it is expected, I don't like it 😞

For me, too, it doesn't seem good, but I think it's not the expected behaviour. As I was checking the Cody web history it works perfectly fine will try to make the VS code history work in the same way which would require some other cleanup too.

@abeatrix
Copy link
Contributor

abeatrix commented Jun 2, 2023

@deepak2431 are you planning to work on this issue? I'm wondering if this issue overlaps with sourcegraph/cody#339 so want to check with you to avoid duplicate works :)

@deepak2431
Copy link
Contributor

deepak2431 commented Jun 2, 2023

@abeatrix Yeah, I was working on this and handling the complete history implementation, as I feel it needs some cleanup and tweaks too.

Oh, I saw your draft PR for the other part of this. Shall I continue working on or will you be handling it entirely in your PR?

@abeatrix
Copy link
Contributor

abeatrix commented Jun 2, 2023

I think there will be works overlapped so I'm happy to close my PR if you want to work on that as well, or I can continue after you're done with yours.
I haven't really looked into how it works currently so I don't know if these two issues depend on each other, what do you think?

@deepak2431
Copy link
Contributor

I think there will be works overlapped so I'm happy to close my PR if you want to work on that as well, or I can continue after you're done with yours. I haven't really looked into how it works currently so I don't know if these two issues depend on each other, what do you think?

I think that both overlapped a bit as when you would work on either you need to do the rework of how the Transcript is rendered, and stored. Doing this will handle both cases of it.
It would be best not to close your PR for now, as I can possibly make a PR for this by Monday including other cleanup required for history and if the covers both the case it would be great. As I will try to take care of another part of your PR too :)

@abeatrix
Copy link
Contributor

abeatrix commented Jun 2, 2023

My PR was just a workaround hoping to solve the issue raised by tim without needing to refactor the view, but it looks like refactoring is needed for it to work as how Tim described, so what i have in my PR is just rubbish at this point :P

@deepak2431
Copy link
Contributor

Looking at Tim's comments in your PR that makes sense. Thanks, @abeatrix

philipp-spiess added a commit that referenced this issue Jun 12, 2023
This PR closes #52544,  #52848, #52889 and #52890.

The issues mostly overlapped, so I handled it in this PR.

Changes made:
1. Add history to the history list on each new conversation started.
2. Display the last human message in history instead of the assistant
message.
3. Add a delete button to delete individual chats from history.
4. Load the most recent chat, if available, from the history.
5. Fix the loading of context files.
6. Load the history messages to the end of the scroll position.

## Test plan

All E2E tests have passed.
https://www.loom.com/share/3dca98dd0db94c298df38ab79c5e72d1

---------

Co-authored-by: Philipp Spiess <hello@philippspiess.com>
@SuperAuguste
Copy link
Contributor

Closed in #52904.

ErikaRS pushed a commit that referenced this issue Jun 22, 2023
This PR closes #52544,  #52848, #52889 and #52890.

The issues mostly overlapped, so I handled it in this PR.

Changes made:
1. Add history to the history list on each new conversation started.
2. Display the last human message in history instead of the assistant
message.
3. Add a delete button to delete individual chats from history.
4. Load the most recent chat, if available, from the history.
5. Fix the loading of context files.
6. Load the history messages to the end of the scroll position.

## Test plan

All E2E tests have passed.
https://www.loom.com/share/3dca98dd0db94c298df38ab79c5e72d1

---------

Co-authored-by: Philipp Spiess <hello@philippspiess.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants