Cody: Persist error messages (#51762)

RE: #51759 Issue: error displaying as chat message will disappear all of a sudden without user action I noticed in the current implementation, errors resulted from failed streaming will replace the current assistant message, which would include the previous error message, or things that Cody has already response. This PR includes a fix where we will add the error message ON TOP of the current assistant message instead of overriding it. For example, instead of removing what Cody has replied and replace it with an error message, we now add the error message after what Cody has already answered `Hello! How can I help you?`: <img width="769" alt="image" src="https://github.com/sourcegraph/sourcegraph/assets/68532117/01862761-1aa3-4f33-94f7-6c2849eb78a3"> Other changes include: - Added a new `<ErrorBanner>` component that would show 5 of the last system error messages as a banner in chat UI (instead of 1, which override the previous one). - Now log all system error messages to debug logs (`"cody.debug": true`) view in UI <img width="1683" alt="Screenshot 2023-05-10 at 7 00 43 PM" src="https://github.com/sourcegraph/sourcegraph/assets/68532117/b09f0a27-bf70-4319-9c29-df2b4f5b6bf7"> <img width="622" alt="image" src="https://github.com/sourcegraph/sourcegraph/assets/68532117/ccdfaf57-8a13-49ab-8d85-5611b52f4a8e"> <img width="609" alt="image" src="https://github.com/sourcegraph/sourcegraph/assets/68532117/8a3800ce-c275-4d4d-845e-6306eecc2c91"> ## Test plan  See screenshots above
sourcegraph · May 11, 2023 · 9b7db99 · 9b7db99
1 parent 51b1bef
commit 9b7db99
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 19 deletions.
diff --git a/client/cody-shared/src/chat/transcript/index.ts b/client/cody-shared/src/chat/transcript/index.ts
@@ -82,10 +82,17 @@ export class Transcript {
     }
 
     public addErrorAsAssistantResponse(errorText: string): void {
-        this.getLastInteraction()?.setAssistantMessage({
+        const lastInteraction = this.getLastInteraction()
+        if (!lastInteraction) {
+            return
+        }
+        // If assistant has responsed before, we will add the error message after it
+        const lastAssistantMessage = lastInteraction.getAssistantMessage().displayText || ''
+        lastInteraction.setAssistantMessage({
             speaker: 'assistant',
-            text: 'Failed to generate response due to server error.',
-            displayText: errorText,
+            text: 'Failed to generate a response due to server error.',
+            displayText:
+                lastAssistantMessage + `<div class="cody-chat-error"><span>Request failed: </span>${errorText}</div>`,
         })
     }
 

diff --git a/client/cody/CHANGELOG.md b/client/cody/CHANGELOG.md
@@ -11,6 +11,7 @@ All notable changes to Sourcegraph Cody will be documented in this file.
 ### Fixed
 
 - UI bug that capped buttons at 300px max-width with visible border [pull/51726](https://github.com/sourcegraph/sourcegraph/pull/51726)
+- Add error message on top of Cody's response instead of overriding it [pull/51762](https://github.com/sourcegraph/sourcegraph/pull/51762)
 
 ### Changed
 

diff --git a/client/cody/src/chat/ChatViewProvider.ts b/client/cody/src/chat/ChatViewProvider.ts
@@ -266,9 +266,7 @@ export class ChatViewProvider implements vscode.WebviewViewProvider, vscode.Disp
             },
             onError: (err, statusCode) => {
                 // Display error message as assistant response
-                this.transcript.addErrorAsAssistantResponse(
-                    `<div class="cody-chat-error"><span>Request failed: </span>${err}</div>`
-                )
+                this.transcript.addErrorAsAssistantResponse(err)
                 // Log users out on unauth error
                 if (statusCode && statusCode >= 400 && statusCode <= 410) {
                     void this.sendLogin(false)
@@ -347,6 +345,7 @@ export class ChatViewProvider implements vscode.WebviewViewProvider, vscode.Disp
     public async executeRecipe(recipeId: string, humanChatInput: string = '', showTab = true): Promise<void> {
         if (this.isMessageInProgress) {
             this.sendErrorToWebview('Cannot execute multiple recipes. Please wait for the current recipe to finish.')
+            return
         }
 
         const recipe = getRecipe(recipeId)

diff --git a/client/cody/webviews/App.css b/client/cody/webviews/App.css
@@ -25,6 +25,12 @@ button {
     flex: 1;
 }
 
+.error-container {
+    display: flex;
+    flex-direction: column-reverse;
+    gap: 0.5rem;
+}
+
 .error {
     flex-direction: row;
     display: flex;
@@ -34,7 +40,7 @@ button {
     align-items: center;
     min-height: 2rem;
     position: relative;
-    overflow: scroll;
+    overflow: auto;
     align-items: baseline;
 }
 

diff --git a/client/cody/webviews/App.tsx b/client/cody/webviews/App.tsx
@@ -19,7 +19,7 @@ import type { VSCodeWrapper } from './utils/VSCodeApi'
 
 export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vscodeAPI }) => {
     const [config, setConfig] = useState<Pick<Configuration, 'debug' | 'serverEndpoint'> | null>(null)
-    const [debugLog, setDebugLog] = useState(['No data yet'])
+    const [debugLog, setDebugLog] = useState<string[]>([])
     const [view, setView] = useState<View | undefined>()
     const [messageInProgress, setMessageInProgress] = useState<ChatMessage | null>(null)
     const [messageBeingEdited, setMessageBeingEdited] = useState<boolean>(false)
@@ -29,7 +29,7 @@ export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vsc
     const [inputHistory, setInputHistory] = useState<string[] | []>([])
     const [userHistory, setUserHistory] = useState<ChatHistory | null>(null)
     const [contextStatus, setContextStatus] = useState<ChatContextStatus | null>(null)
-    const [errorMessage, setErrorMessage] = useState<string>('')
+    const [errorMessages, setErrorMessages] = useState<string[]>([])
     const [suggestions, setSuggestions] = useState<string[] | undefined>()
 
     useEffect(() => {
@@ -70,7 +70,8 @@ export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vsc
                     setContextStatus(message.contextStatus)
                     break
                 case 'errors':
-                    setErrorMessage(message.errors)
+                    setErrorMessages([...errorMessages, message.errors].slice(-5))
+                    setDebugLog([...debugLog, message.errors])
                     break
                 case 'view':
                     setView(message.messages)
@@ -83,7 +84,7 @@ export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vsc
 
         vscodeAPI.postMessage({ command: 'initialized' })
         // The dependencies array is empty to execute the callback only on component mount.
-    }, [debugLog, vscodeAPI])
+    }, [debugLog, errorMessages, vscodeAPI])
 
     const onLogin = useCallback(
         (token: string, endpoint: string) => {
@@ -124,14 +125,7 @@ export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vsc
             )}
             {view === 'recipes' && <Recipes vscodeAPI={vscodeAPI} />}
             {view === 'settings' && <Settings onLogout={onLogout} serverEndpoint={config?.serverEndpoint} />}
-            {view === 'chat' && errorMessage && (
-                <div className="error">
-                    Error: {errorMessage}
-                    <button type="button" onClick={() => setErrorMessage('')} className="close-btn">
-                        ×
-                    </button>
-                </div>
-            )}
+            {view === 'chat' && errorMessages && <ErrorBanner errors={errorMessages} setErrors={setErrorMessages} />}
             {view === 'chat' && (
                 <Chat
                     messageInProgress={messageInProgress}
@@ -151,3 +145,19 @@ export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vsc
         </div>
     )
 }
+
+const ErrorBanner: React.FunctionComponent<{ errors: string[]; setErrors: (errors: string[]) => void }> = ({
+    errors,
+    setErrors,
+}) => (
+    <div className="error-container">
+        {errors.map((error, i) => (
+            <div key={i} className="error">
+                <span>{error}</span>
+                <button type="button" className="close-btn" onClick={() => setErrors(errors.filter(e => e !== error))}>
+                    ×
+                </button>
+            </div>
+        ))}
+    </div>
+)