Merge pull request #1134 from buster-so/nate/fix-streaming

Add window focus trigger for changing page

apps/web/src/context/Dashboards/useIsDashboardChanged.tsx

@@ -35,6 +35,7 @@ export const useIsDashboardChanged = ({
         }),
         []
       ),
+      enabled: false,
     }
   );
 

apps/web/src/context/Reports/useIsReportChanged.ts

@@ -12,7 +12,7 @@ import { useGetOriginalReport } from './useOriginalReportStore';
 
 export const useIsReportChanged = ({
   reportId,
-  enabled = true,
+  enabled = false,
 }: {
   reportId: string | undefined;
   enabled?: boolean;

apps/web/src/hooks/useDocumentTitle.tsx

@@ -8,3 +8,9 @@ export const useDocumentTitle = (title: string | undefined) => {
     }, 25);
   }, [title]);
 };
+
+export const updateDocumentTitle = (callback: (currentTitle: string) => string) => {
+  const currentTitle = document.title;
+  const newTitle = callback(currentTitle);
+  document.title = newTitle;
+};

apps/web/src/layouts/ChatLayout/ChatContent/ChatUserMessage.tsx

@@ -38,22 +38,16 @@ export const ChatUserMessage: React.FC<{
     (e?: React.ClipboardEvent) => {
       // Check if user has selected text
       const selection = window.getSelection();
-      const hasSelection = selection && selection.toString().length > 0;
+      const selectedText = selection?.toString().trim() || '';
+      const hasSelection = selectedText.length > 0;
 
-      // If user has selected text, let browser handle it naturally
-      if (hasSelection && e?.clipboardData) {
-        // Don't prevent default - let browser copy the selected text
-        return;
-      }
-
-      // Only override copy behavior when no text is selected
-      // This handles the case where user presses Ctrl+C without selection
-      // or when the copy button is clicked
+      // Always override copy behavior to provide clean text
       if (e?.clipboardData) {
         e.preventDefault();
-        e.clipboardData.setData('text/plain', request || '');
+        // Copy selected text if there is a selection, otherwise copy full message
+        e.clipboardData.setData('text/plain', hasSelection ? selectedText : request || '');
       } else {
-        navigator.clipboard.writeText(request || '');
+        navigator.clipboard.writeText(hasSelection ? selectedText : request || '');
       }
       openSuccessMessage('Copied to clipboard');
     },
@@ -77,11 +71,10 @@ export const ChatUserMessage: React.FC<{
         />
       ) : (
         <>
-          <div>
-            <Paragraph className="break-words whitespace-pre-wrap" onCopy={handleCopy}>
-              {request}
-            </Paragraph>
-          </div>
+          <Paragraph className="break-words whitespace-pre-line" onCopy={handleCopy}>
+            {request}
+          </Paragraph>
+
           {isStreamFinished && canEditChat && (
             <RequestMessageTooltip
               isTooltipOpen={isTooltipOpen}

apps/web/src/layouts/ChatLayout/ChatLayoutContext/useAutoRedirectStreaming.test.ts

@@ -0,0 +1,310 @@
+import { renderHook } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type { BusterChatResponseMessage_file } from '@/api/asset_interfaces/chat';
+import { useAutoRedirectStreaming } from './useAutoRedirectStreaming';
+
+// Mock all the dependencies
+vi.mock('@tanstack/react-router', () => ({
+  useNavigate: vi.fn(),
+}));
+
+vi.mock('@/api/buster_rest/chats', () => ({
+  useGetChatMessageMemoized: vi.fn(),
+}));
+
+vi.mock('@/context/AppVersion/useAppVersion', () => ({
+  useIsVersionChanged: vi.fn(),
+}));
+
+vi.mock('@/context/Chats/useGetChat', () => ({
+  useHasLoadedChat: vi.fn(),
+}));
+
+vi.mock('@/context/Chats/useGetChatMessage', () => ({
+  useGetChatMessageCompleted: vi.fn(),
+  useGetChatMessageHasResponseFile: vi.fn(),
+  useGetChatMessageIsFinishedReasoning: vi.fn(),
+  useGetChatMessageLastReasoningMessageId: vi.fn(),
+}));
+
+vi.mock('@/hooks/useWindowFocus', () => ({
+  useWindowFocus: vi.fn(),
+}));
+
+vi.mock('@/lib/assets/assetParamsToRoute', () => ({
+  assetParamsToRoute: vi.fn(),
+}));
+
+// Import the mocked functions to use in tests
+import { useNavigate } from '@tanstack/react-router';
+import { useGetChatMessageMemoized } from '@/api/buster_rest/chats';
+import { useIsVersionChanged } from '@/context/AppVersion/useAppVersion';
+import { useHasLoadedChat } from '@/context/Chats/useGetChat';
+import {
+  useGetChatMessageCompleted,
+  useGetChatMessageHasResponseFile,
+  useGetChatMessageIsFinishedReasoning,
+  useGetChatMessageLastReasoningMessageId,
+} from '@/context/Chats/useGetChatMessage';
+import { assetParamsToRoute } from '@/lib/assets/assetParamsToRoute';
+
+describe('useAutoRedirectStreaming', () => {
+  const defaultProps = {
+    lastMessageId: 'message-123',
+    chatId: 'chat-456',
+  };
+
+  const mockChatMessage = {
+    id: 'message-123',
+    response_message_ids: ['file-789'],
+    response_messages: {
+      'file-789': {
+        id: 'file-789',
+        type: 'file',
+        file_type: 'dashboard_file',
+        file_name: 'Test Dashboard',
+        version_number: 1,
+      } as BusterChatResponseMessage_file,
+    },
+  };
+
+  let mockNavigateFn: ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    // Setup navigation mock
+    mockNavigateFn = vi.fn();
+    vi.mocked(useNavigate).mockReturnValue(mockNavigateFn);
+
+    // Set default mock implementations
+    vi.mocked(useHasLoadedChat).mockReturnValue(true);
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageHasResponseFile).mockReturnValue(false);
+    vi.mocked(useGetChatMessageIsFinishedReasoning).mockReturnValue(false);
+    vi.mocked(useGetChatMessageLastReasoningMessageId).mockReturnValue(undefined);
+    vi.mocked(useIsVersionChanged).mockReturnValue(false);
+
+    // useGetChatMessageMemoized returns a function that returns a message
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => undefined as any);
+    vi.mocked(assetParamsToRoute).mockReturnValue({
+      to: '/app/dashboards/$dashboardId',
+      params: { dashboardId: 'dashboard-123' },
+    } as any);
+  });
+
+  it('should navigate to file route when streaming and has file in response', () => {
+    // Setup: streaming with file
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => mockChatMessage as any);
+
+    renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    expect(assetParamsToRoute).toHaveBeenCalledWith({
+      assetId: 'file-789',
+      assetType: 'dashboard_file',
+      chatId: 'chat-456',
+      versionNumber: 1,
+    });
+
+    expect(mockNavigateFn).toHaveBeenCalledWith({
+      to: '/app/dashboards/$dashboardId',
+      params: { dashboardId: 'dashboard-123' },
+      replace: true,
+      reloadDocument: false,
+    });
+  });
+
+  it('should navigate to file route when stream completed and has file (first time completion)', () => {
+    // Setup: Start with stream not finished, then complete it to simulate first-time completion
+    const mockCompleted = vi.mocked(useGetChatMessageCompleted);
+    mockCompleted.mockReturnValue(false); // Start as not completed
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => mockChatMessage as any);
+
+    // First render with stream not completed
+    const { rerender } = renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    // Now complete the stream to trigger first-time completion
+    mockCompleted.mockReturnValue(true);
+    rerender();
+
+    expect(assetParamsToRoute).toHaveBeenCalledWith({
+      assetId: 'file-789',
+      assetType: 'dashboard_file',
+      chatId: 'chat-456',
+      versionNumber: 1,
+    });
+
+    expect(mockNavigateFn).toHaveBeenCalledWith({
+      to: '/app/dashboards/$dashboardId',
+      params: { dashboardId: 'dashboard-123' },
+      replace: true,
+      reloadDocument: false,
+    });
+  });
+
+  it('should navigate to reasoning route when streaming with reasoning but not finished', () => {
+    // Setup: streaming with reasoning, not finished
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageIsFinishedReasoning).mockReturnValue(false);
+    vi.mocked(useGetChatMessageLastReasoningMessageId).mockReturnValue('reasoning-123');
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(
+      () => ({ response_message_ids: [] }) as any
+    );
+
+    renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    expect(mockNavigateFn).toHaveBeenCalledWith({
+      to: '/app/chats/$chatId/reasoning/$messageId',
+      params: {
+        chatId: 'chat-456',
+        messageId: 'message-123',
+      },
+      replace: true,
+      reloadDocument: false,
+    });
+  });
+
+  it('should navigate to chat route when finished reasoning and stream completed (first time)', () => {
+    // Setup: Start with stream not finished, then complete it to simulate first-time completion
+    const mockCompleted = vi.mocked(useGetChatMessageCompleted);
+    mockCompleted.mockReturnValue(false); // Start as not completed
+    vi.mocked(useGetChatMessageIsFinishedReasoning).mockReturnValue(true);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(
+      () => ({ response_message_ids: [] }) as any
+    );
+
+    // First render with stream not completed
+    const { rerender } = renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    // Now complete the stream to trigger first-time completion
+    mockCompleted.mockReturnValue(true);
+    rerender();
+
+    expect(mockNavigateFn).toHaveBeenCalledWith({
+      to: '/app/chats/$chatId',
+      params: {
+        chatId: 'chat-456',
+      },
+      replace: true,
+      reloadDocument: false,
+    });
+  });
+
+  it('should not navigate when chat has not loaded', () => {
+    // Setup: chat not loaded
+    vi.mocked(useHasLoadedChat).mockReturnValue(false);
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => mockChatMessage as any);
+
+    renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    expect(mockNavigateFn).not.toHaveBeenCalled();
+  });
+
+  it('should not navigate when chatId is undefined', () => {
+    // Setup: no chatId
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => mockChatMessage as any);
+
+    renderHook(() => useAutoRedirectStreaming({ ...defaultProps, chatId: undefined }));
+
+    expect(mockNavigateFn).not.toHaveBeenCalled();
+  });
+
+  it('should navigate with version changed flag when version changed', () => {
+    // Setup: streaming with file, version changed
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => mockChatMessage as any);
+    vi.mocked(useIsVersionChanged).mockReturnValue(true);
+
+    renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    expect(mockNavigateFn).toHaveBeenCalledWith({
+      to: '/app/dashboards/$dashboardId',
+      params: { dashboardId: 'dashboard-123' },
+      replace: true,
+      reloadDocument: true,
+    });
+  });
+
+  it('should handle metric file type navigation correctly', () => {
+    // Setup: streaming with metric file
+    const metricChatMessage = {
+      id: 'message-123',
+      response_message_ids: ['metric-789'],
+      response_messages: {
+        'metric-789': {
+          id: 'metric-789',
+          type: 'file',
+          file_type: 'metric_file',
+          file_name: 'Test Metric',
+          version_number: 2,
+        } as BusterChatResponseMessage_file,
+      },
+    };
+
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => metricChatMessage as any);
+    vi.mocked(assetParamsToRoute).mockReturnValue({
+      to: '/app/metrics/$metricId',
+      params: { metricId: 'metric-123' },
+    } as any);
+
+    renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    expect(assetParamsToRoute).toHaveBeenCalledWith({
+      assetId: 'metric-789',
+      assetType: 'metric_file',
+      chatId: 'chat-456',
+      versionNumber: 2,
+    });
+
+    expect(mockNavigateFn).toHaveBeenCalledWith({
+      to: '/app/metrics/$metricId',
+      params: { metricId: 'metric-123' },
+      replace: true,
+      reloadDocument: false,
+    });
+  });
+
+  it('should not navigate when streaming is completed and it was already a completed stream', () => {
+    // Setup: Simulate a stream that completes with a file, then gets re-rendered
+    const mockCompleted = vi.mocked(useGetChatMessageCompleted);
+    mockCompleted.mockReturnValue(false); // Start as not completed
+
+    // Start without a file, so first render doesn't navigate
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(
+      () => ({ response_message_ids: [] }) as any
+    );
+
+    // First render - stream not completed yet, no file
+    const { rerender } = renderHook(() => useAutoRedirectStreaming(defaultProps));
+    expect(mockNavigateFn).not.toHaveBeenCalled();
+
+    // Complete the stream AND add a file - should trigger navigation (first time)
+    mockCompleted.mockReturnValue(true);
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(() => mockChatMessage as any);
+    rerender();
+    expect(mockNavigateFn).toHaveBeenCalledTimes(1);
+
+    // Reset the navigation mock and re-render - should not navigate again (previousIsCompletedStream = true)
+    mockNavigateFn.mockClear();
+    rerender();
+    expect(mockNavigateFn).not.toHaveBeenCalled();
+  });
+
+  it('should not navigate to reasoning when reasoning is finished', () => {
+    // Setup: streaming with reasoning but reasoning is finished
+    vi.mocked(useGetChatMessageCompleted).mockReturnValue(false);
+    vi.mocked(useGetChatMessageIsFinishedReasoning).mockReturnValue(true);
+    vi.mocked(useGetChatMessageLastReasoningMessageId).mockReturnValue('reasoning-123');
+    vi.mocked(useGetChatMessageMemoized).mockReturnValue(
+      () => ({ response_message_ids: [] }) as any
+    );
+
+    renderHook(() => useAutoRedirectStreaming(defaultProps));
+
+    expect(mockNavigateFn).not.toHaveBeenCalled();
+  });
+});

apps/web/src/layouts/ChatLayout/ChatLayoutContext/useAutoRedirectStreaming.ts

@@ -1,5 +1,5 @@
-import { useLocation, useNavigate } from '@tanstack/react-router';
-import { useEffect, useLayoutEffect, useRef } from 'react';
+import { useNavigate } from '@tanstack/react-router';
+import { useEffect, useLayoutEffect, useRef, useState } from 'react';
 import type { BusterChatResponseMessage_file } from '@/api/asset_interfaces/chat';
 import { useGetChatMessageMemoized } from '@/api/buster_rest/chats';
 import { useIsVersionChanged } from '@/context/AppVersion/useAppVersion';
@@ -10,6 +10,7 @@ import {
   useGetChatMessageIsFinishedReasoning,
   useGetChatMessageLastReasoningMessageId,
 } from '@/context/Chats/useGetChatMessage';
+import { useWindowFocus } from '@/hooks/useWindowFocus';
 import { assetParamsToRoute } from '@/lib/assets/assetParamsToRoute';
 
 export const useAutoRedirectStreaming = ({
@@ -31,6 +32,7 @@ export const useAutoRedirectStreaming = ({
   const previousIsCompletedStream = useRef<boolean>(isStreamFinished);
   const hasLoadedChat = useHasLoadedChat({ chatId: chatId || '' });
   const hasReasoning = !!lastReasoningMessageId;
+  const [triggerAutoNavigate, setTriggerAutoNavigate] = useState<number>(0);
 
   useLayoutEffect(() => {
     previousIsCompletedStream.current = isStreamFinished;
@@ -49,7 +51,11 @@ export const useAutoRedirectStreaming = ({
     });
 
     //this will happen if it is streaming and has a file in the response
-    if (!isStreamFinished && firstFileId) {
+    // or if the chat is completed and has a file in the response
+    if (
+      (!isStreamFinished && firstFileId) ||
+      (isStreamFinished && firstFileId && previousIsCompletedStream.current === false)
+    ) {
       const firstFile = chatMessage?.response_messages[firstFileId] as
         | BusterChatResponseMessage_file
         | undefined;
@@ -64,6 +70,8 @@ export const useAutoRedirectStreaming = ({
 
         navigate({ ...linkProps, replace: true, reloadDocument: versionChanged });
       }
+
+      previousIsCompletedStream.current = true;
     }
 
     //this will trigger when the chat is streaming and is has not completed yet (new chat)
@@ -86,8 +94,6 @@ export const useAutoRedirectStreaming = ({
       previousIsCompletedStream.current === false &&
       !firstFileId
     ) {
-      //no file is found, so we need to collapse the chat
-
       navigate({
         to: '/app/chats/$chatId',
         params: {
@@ -96,12 +102,27 @@ export const useAutoRedirectStreaming = ({
         replace: true,
         reloadDocument: versionChanged,
       });
+      previousIsCompletedStream.current = true;
     }
-  }, [isStreamFinished, hasReasoning, hasResponseFile, chatId, lastMessageId, isFinishedReasoning]); //only use these values to trigger the useEffect
+  }, [
+    isStreamFinished,
+    hasReasoning,
+    hasResponseFile,
+    chatId,
+    lastMessageId,
+    isFinishedReasoning,
+    triggerAutoNavigate,
+  ]); //only use these values to trigger the useEffect
 
   useEffect(() => {
     if (!isStreamFinished && versionChanged) {
       window.location.reload();
     }
   }, [isStreamFinished, versionChanged]);
+
+  useWindowFocus(() => {
+    if (isStreamFinished && previousIsCompletedStream.current === false) {
+      setTriggerAutoNavigate((prev) => prev + 1);
+    }
+  });
 };

apps/web/src/layouts/ChatLayout/ChatLayoutContext/useChatStreaming.tsx

@@ -1,7 +1,7 @@
 import { useQueryClient } from '@tanstack/react-query';
 import { useEffect } from 'react';
 import type { BusterChat, BusterChatMessage, IBusterChat } from '@/api/asset_interfaces/chat';
-import { useGetChatMessageMemoized } from '@/api/buster_rest/chats';
+import { useGetChat, useGetChatMessageMemoized } from '@/api/buster_rest/chats';
 import { prefetchGetMetricDataClient } from '@/api/buster_rest/metrics';
 import { useTrackAndUpdateChatChanges } from '@/api/buster-electric/chats';
 import {
@@ -11,10 +11,11 @@ import {
 import { chatQueryKeys } from '@/api/query_keys/chat';
 import { metricsQueryKeys } from '@/api/query_keys/metric';
 import { useBlackboxMessage } from '@/context/BlackBox/useBlackboxMessage';
+import { updateDocumentTitle } from '@/hooks/useDocumentTitle';
 import { useMemoizedFn } from '@/hooks/useMemoizedFn';
-import { useMount } from '@/hooks/useMount';
 import { updateChatToIChat } from '@/lib/chat';
 
+const stableChatTitleSelector = (chat: IBusterChat) => chat.title;
 export const useChatStreaming = ({
   chatId,
   isStreamingMessage,
@@ -27,6 +28,10 @@ export const useChatStreaming = ({
   const { checkBlackBoxMessage, removeBlackBoxMessage } = useBlackboxMessage();
   const queryClient = useQueryClient();
   const getChatMessageMemoized = useGetChatMessageMemoized();
+  const { data: chatTitle } = useGetChat(
+    { id: chatId || '' },
+    { select: stableChatTitleSelector, notifyOnChangeProps: ['data'] }
+  );
 
   const _prefetchLastMessageMetricData = (
     iChat: IBusterChat,

apps/web/src/routes/app.tsx

@@ -27,7 +27,7 @@ export const Route = createFileRoute('/app')({
     const { queryClient, supabaseSession } = context;
     try {
       const [user] = await Promise.all([prefetchGetMyUserInfo(queryClient)]);
-      if (!user || !user.organizations || user.organizations.length === 0) {
+      if (user && user?.organizations?.length === 0) {
         throw redirect({ href: BUSTER_SIGN_UP_URL, replace: true, statusCode: 307 });
       }
       return {