cody: log user out on invalid session + display error in-chat #51005
Conversation
abeatrix
changed the title
abeatrix
changed the title
abeatrix
changed the title
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits 9130ebd and 123ed1b or learn more. Open explanation
|
abeatrix
changed the title
| void this.sendLogin(false) | ||
| this.clearAndRestartSession() | ||
| } | ||
| this.onCompletionEnd() |
There was a problem hiding this comment.
If an error occurs, that means that there will be an empty assistant message in the transcript. Do we want to simply continue with that, or restart before the failure?
Ex:
H: Hi
A: Hi!
H: What is X
A: <error>
Should the new transcript for another submission after the error be
H: Hi
A: Hi!
H: What is X
H: Oh an error, ok. What is X?
A: It is X!
or
H: Hi
A: Hi!
H: What is X
A: ''
H: Oh an error, ok. What is X?
A: It is X!
If it's the latter, I think we should attach the error to the message in the transcript so it's also persisted that we later know that the errored message produced an empty response and why (?)
There was a problem hiding this comment.
i've replaced the empty assistant message with:
So the flow will be like:
H: Hi
A: Hi!
H: What is X
A: Failed to generate response due to server error.
H: Oh an error, ok. What is X?
A: It is X!
Does this address your concern? If not I can open a new PR for this, just lmk :D
There was a problem hiding this comment.
i've replaced the empty assistant message with:
So the flow will be like:
H: Hi
A: Hi!
H: What is X
A: Failed to generate response due to server error.
H: Oh an error, ok. What is X?
A: It is X!
Does this address your concern? If not I can open a new PR for this, just lmk :D
There was a problem hiding this comment.
Does this send that failure message in the transcript to the LLM or is that hidden from there? Just curious, might confuse the LLM 😬
But otherwise that sounds great, thank you @abeatrix!
There was a problem hiding this comment.
What's included in speaker.text is sent to LLM and hidden from user, and what's included in speaker.displayText is displayed to user, but hidden from LLM (if i not mistaken 😆 )
Example:
sourcegraph/client/cody-shared/src/prompt/prompt-mixin.ts
Lines 16 to 18 in 9130ebd
I hope this makes sense 😄
re-add `this.isMessageInProgress = false` that was removed by mistake in my last PR: #51005 ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> Minor change. Same test cases, all passing.
re-add `this.isMessageInProgress = false` that was removed by mistake in my last PR: sourcegraph/sourcegraph#51005 ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> Minor change. Same test cases, all passing.
RE #50973 #50307
Current Issue: When the token become invalid during the chat session, it does not log users out and not displaying any error message
Solution: Log the user out for invalid login credentials during the chat session.
More detail in Loom: https://www.loom.com/share/cfd51d7cd97e49989af77c8d2e103973
Updated: Display error message in frontend https://www.loom.com/share/4a4ef8a298e241c9af220ad116a4d5be
Example:
Shout out to @eseliger for his PR on sending error messages to chat client!
Test plan
All tests have passed and tested locally. See changes in the attached loom video above