fix(lightspeed): ux improvements and persist display mode preference

workspaces/lightspeed/.changeset/eleven-candles-unite.md

@@ -0,0 +1,5 @@
+---
+'@red-hat-developer-hub/backstage-plugin-lightspeed': patch
+---
+
+some ux improvements and persisting the display mode preference

workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md

@@ -43,15 +43,12 @@ readonly "conversation.delete.confirm.action": string;
 readonly "conversation.rename.confirm.title": string;
 readonly "conversation.rename.confirm.action": string;
 readonly "conversation.rename.placeholder": string;
-readonly "conversation.action.error": string;
 readonly "permission.required.title": string;
 readonly "permission.required.description": string;
 readonly "footer.accuracy.label": string;
-readonly "footer.accuracy.popover.title": string;
 readonly "footer.accuracy.popover.description": string;
 readonly "footer.accuracy.popover.image.alt": string;
 readonly "footer.accuracy.popover.cta.label": string;
-readonly "footer.accuracy.popover.link.label": string;
 readonly "common.cancel": string;
 readonly "common.close": string;
 readonly "common.readMore": string;

workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts

@@ -195,9 +195,19 @@ export class LightspeedApiClient implements LightspeedAPI {
     );
 
     if (!response.ok) {
-      throw new Error(
-        `failed to delete conversation, status ${response.status}: ${response.statusText}`,
-      );
+      let errorMessage = `failed to delete conversation, status ${response.status}: ${response.statusText}`;
+
+      try {
+        const errorBody = await response.json();
+        if (errorBody?.error) {
+          errorMessage = errorBody.error;
+        }
+      } catch (e) {
+        // eslint-disable-next-line no-console
+        console.warn(e);
+      }
+
+      throw new Error(errorMessage);
     }
     return { success: true };
   }
@@ -217,9 +227,19 @@ export class LightspeedApiClient implements LightspeedAPI {
     );
 
     if (!response.ok) {
-      throw new Error(
-        `failed to rename conversation, status ${response.status}: ${response.statusText}`,
-      );
+      let errorMessage = `failed to rename conversation, status ${response.status}: ${response.statusText}`;
+
+      try {
+        const errorBody = await response.json();
+        if (errorBody?.error) {
+          errorMessage = errorBody.error;
+        }
+      } catch (e) {
+        // eslint-disable-next-line no-console
+        console.warn(e);
+      }
+
+      throw new Error(errorMessage);
     }
     return { success: true };
   }

workspaces/lightspeed/plugins/lightspeed/src/api/__tests__/LightspeedApiClient.test.ts

@@ -214,14 +214,44 @@ describe('LightspeedApiClient', () => {
         ok: false,
         status: 400,
         statusText: 'Bad Request',
+        json: jest.fn().mockResolvedValue({
+          error:
+            'Error from lightspeed-core server: The conversation ID conversationId- has invalid format.',
+        }),
       } as unknown as Response);
 
       await expect(
         client.renameConversation('conv-123', 'New Name'),
       ).rejects.toThrow(
-        'failed to rename conversation, status 400: Bad Request',
+        'Error from lightspeed-core server: The conversation ID conversationId- has invalid format.',
       );
     });
+
+    it('should throw error with statusText when JSON parsing fails', async () => {
+      mockFetchApi.fetch.mockResolvedValue({
+        ok: false,
+        status: 400,
+        statusText: 'Bad Request',
+        json: jest.fn().mockRejectedValue(new Error('Invalid JSON')),
+      } as unknown as Response);
+
+      await expect(
+        client.renameConversation('conv-123', 'New Name'),
+      ).rejects.toThrow('Bad Request');
+    });
+
+    it('should throw error with statusText when JSON does not contain error field', async () => {
+      mockFetchApi.fetch.mockResolvedValue({
+        ok: false,
+        status: 400,
+        statusText: 'Bad Request',
+        json: jest.fn().mockResolvedValue({ message: 'Some other error' }),
+      } as unknown as Response);
+
+      await expect(
+        client.renameConversation('conv-123', 'New Name'),
+      ).rejects.toThrow('Bad Request');
+    });
   });
 
   describe('getFeedbackStatus', () => {

workspaces/lightspeed/plugins/lightspeed/src/components/DeleteModal.tsx

@@ -99,12 +99,8 @@ export const DeleteModal = ({
         {t('conversation.delete.confirm.message')}
       </DialogContent>
       {isError && (
-        <Box maxWidth="650px" marginLeft="20px">
-          <Alert severity="error">
-            {t('conversation.action.error' as any, {
-              error: String(error),
-            })}
-          </Alert>
+        <Box maxWidth="650px" marginLeft="20px" marginRight="20px">
+          <Alert severity="error">{String(error)}</Alert>
         </Box>
       )}
       <DialogActions style={{ justifyContent: 'left', padding: '20px' }}>

workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx

@@ -222,7 +222,7 @@ export const LightspeedChatBox = forwardRef(
           // Map first tool call to PatternFly's toolCall prop
           const firstToolCall = message.toolCalls?.[0];
           const toolCallProp = firstToolCall
-            ? mapToPatternFlyToolCall(firstToolCall, t)
+            ? mapToPatternFlyToolCall(firstToolCall, t, message.role)
             : undefined;
 
           // Handle additional tool calls (if any) via extraContent
@@ -256,26 +256,48 @@ export const LightspeedChatBox = forwardRef(
                 body: reasoningContent,
                 expandableSectionProps: {},
               };
-              extraContentParts.beforeMainContent = (
-                <DeepThinking {...deepThinking} />
-              );
             }
           }
 
+          const allToolCalls: React.ReactNode[] = [];
+
+          // Add first tool call if it exists
+          if (toolCallProp && firstToolCall) {
+            allToolCalls.push(
+              <div
+                key={`tool-${firstToolCall.id}-${firstToolCall.toolName}`}
+                style={{ marginTop: '8px' }}
+              >
+                <PatternFlyToolCall {...toolCallProp} />
+              </div>,
+            );
+          }
+
+          // Add additional tool calls
           if (additionalToolCalls && additionalToolCalls.length > 0) {
-            extraContentParts.afterMainContent = (
+            additionalToolCalls.forEach(tc => {
+              const tcProps = mapToPatternFlyToolCall(tc, t, message.role);
+              allToolCalls.push(
+                <div
+                  key={`tool-${tc.id}-${tc.toolName}`}
+                  style={{ marginTop: '8px' }}
+                >
+                  <PatternFlyToolCall {...tcProps} />
+                </div>,
+              );
+            });
+          }
+
+          // Show Thinking first, then tool calls
+          if (deepThinking || allToolCalls.length > 0) {
+            extraContentParts.beforeMainContent = (
               <>
-                {additionalToolCalls.map(tc => {
-                  const tcProps = mapToPatternFlyToolCall(tc, t);
-                  return (
-                    <div
-                      key={`tool-${tc.id}-${tc.toolName}`}
-                      style={{ marginTop: '8px' }}
-                    >
-                      <PatternFlyToolCall {...tcProps} />
-                    </div>
-                  );
-                })}
+                {deepThinking && <DeepThinking {...deepThinking} />}
+                {allToolCalls.length > 0 && (
+                  <div style={{ marginTop: deepThinking ? '8px' : '0' }}>
+                    {allToolCalls}
+                  </div>
+                )}
               </>
             );
           }
@@ -295,7 +317,6 @@ export const LightspeedChatBox = forwardRef(
           return (
             <Message
               key={`${message.role}-${index}`}
-              toolCall={toolCallProp}
               extraContent={extraContent}
               {...finalMessage}
             />

workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerProvider.tsx

@@ -27,6 +27,7 @@ import { useLocation, useMatch, useNavigate } from 'react-router-dom';
 import { makeStyles } from '@mui/styles';
 import { ChatbotDisplayMode, ChatbotModal } from '@patternfly/chatbot';
 
+import { useBackstageUserIdentity, useDisplayModeSettings } from '../hooks';
 import { FileContent } from '../types';
 import { LightspeedChatContainer } from './LightspeedChatContainer';
 import { LightspeedDrawerContext } from './LightspeedDrawerContext';
@@ -47,10 +48,14 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
   const classes = useStyles();
   const navigate = useNavigate();
   const location = useLocation();
+  const user = useBackstageUserIdentity();
+  const {
+    displayMode: persistedDisplayMode,
+    setDisplayMode: setPersistedDisplayMode,
+  } = useDisplayModeSettings(user, ChatbotDisplayMode.default);
 
-  const [displayModeState, setDisplayModeState] = useState<ChatbotDisplayMode>(
-    ChatbotDisplayMode.default,
-  );
+  const [displayModeState, setDisplayModeState] =
+    useState<ChatbotDisplayMode>(persistedDisplayMode);
   const [isOpen, setIsOpen] = useState<boolean>(false);
   const [drawerWidth, setDrawerWidth] = useState<number>(400);
   const [currentConversationIdState, setCurrentConversationIdState] = useState<
@@ -89,38 +94,59 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
       }
       // Update this to fullscreen only when it is not already in the docked mode
       setDisplayModeState(prev => {
-        if (prev === ChatbotDisplayMode.docked) {
-          return prev; // Don't override docked mode
+        if (
+          prev === ChatbotDisplayMode.docked ||
+          persistedDisplayMode === ChatbotDisplayMode.docked
+        ) {
+          return ChatbotDisplayMode.docked; // Don't override docked mode preference
         }
         return ChatbotDisplayMode.embedded;
       });
+      // When opening via URL
+      if (persistedDisplayMode !== ChatbotDisplayMode.embedded) {
+        setPersistedDisplayMode(ChatbotDisplayMode.embedded);
+      }
       setIsOpen(true);
     } else {
-      // When leaving lightspeed route, update this only when the current mode is fullscreen
-      setDisplayModeState(prev => {
-        if (prev === ChatbotDisplayMode.embedded) {
-          return ChatbotDisplayMode.default;
-        }
-        return prev;
-      });
+      // When leaving lightspeed route, restore the persisted display mode
+      // If persisted mode is embedded, reset to default
+      if (persistedDisplayMode === ChatbotDisplayMode.embedded) {
+        setDisplayModeState(ChatbotDisplayMode.default);
+      } else {
+        setDisplayModeState(persistedDisplayMode);
+      }
     }
-  }, [conversationId, isLightspeedRoute]);
+  }, [
+    conversationId,
+    isLightspeedRoute,
+    persistedDisplayMode,
+    setPersistedDisplayMode,
+  ]);
 
-  // Open chatbot in overlay mode
+  // Open chatbot using the persisted display mode preference
   const openChatbot = useCallback(() => {
     openedViaFABRef.current = true;
-    setDisplayModeState(ChatbotDisplayMode.default);
+
+    const modeToUse = persistedDisplayMode || ChatbotDisplayMode.default;
+    setDisplayModeState(modeToUse);
+
+    if (modeToUse === ChatbotDisplayMode.embedded) {
+      const convId = currentConversationIdState;
+      const path = convId
+        ? `/lightspeed/conversation/${convId}`
+        : '/lightspeed';
+      navigate(path);
+    }
+
     setIsOpen(true);
-  }, []);
+  }, [persistedDisplayMode, currentConversationIdState, navigate]);
 
-  // Close chatbot
   const closeChatbot = useCallback(() => {
     // If in embedded mode on the lightspeed route, navigate back
     if (displayModeState === ChatbotDisplayMode.embedded && isLightspeedRoute) {
       navigateBackOrGoToCatalog();
     }
     setIsOpen(false);
-    setDisplayModeState(ChatbotDisplayMode.default);
   }, [displayModeState, isLightspeedRoute, navigateBackOrGoToCatalog]);
 
   const toggleChatbot = useCallback(() => {
@@ -135,7 +161,6 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
     (id: string | undefined) => {
       setCurrentConversationIdState(id);
 
-      // Update route if in embedded mode
       if (
         displayModeState === ChatbotDisplayMode.embedded &&
         isLightspeedRoute
@@ -164,6 +189,11 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
 
       setDisplayModeState(mode);
 
+      // Persist the display mode preference when user explicitly changes it, but not for route-based changes to embedded mode
+      if (mode !== ChatbotDisplayMode.embedded || !isLightspeedRoute) {
+        setPersistedDisplayMode(mode);
+      }
+
       // Navigate to fullscreen route with conversation ID if available
       if (mode === ChatbotDisplayMode.embedded) {
         const convId = conversationIdParam ?? currentConversationIdState;
@@ -185,6 +215,7 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => {
       currentConversationIdState,
       displayModeState,
       navigateBackOrGoToCatalog,
+      setPersistedDisplayMode,
     ],
   );
 

workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerStateExposer.tsx

@@ -54,10 +54,16 @@ export type DrawerStateExposerProps = {
 export const LightspeedDrawerStateExposer = ({
   onStateChange,
 }: DrawerStateExposerProps) => {
-  const { displayMode, drawerWidth, setDrawerWidth, toggleChatbot } =
-    useLightspeedDrawerContext();
+  const {
+    displayMode,
+    drawerWidth,
+    setDrawerWidth,
+    toggleChatbot,
+    isChatbotActive,
+  } = useLightspeedDrawerContext();
 
-  const isDrawerOpen = displayMode === ChatbotDisplayMode.docked;
+  const isDrawerOpen =
+    displayMode === ChatbotDisplayMode.docked && isChatbotActive;
 
   const toggleChatbotRef = useRef(toggleChatbot);
   toggleChatbotRef.current = toggleChatbot;