From 0e8041bcac616949c42a68dfb8f108ccc4db5151 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 28 Apr 2025 18:18:13 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=83=20refactor:=20Streamline=20Navigat?= =?UTF-8?q?ion,=20Message=20Loading=20UX=20(#7118)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: fix logging for illegal target endpoints in getEndpointFromSetup * fix: prevent querying agent by ID for ephemeral agents * refactor: reorder variable declarations in MessagesView for clarity * fix: localize 'nothing found' message in MessagesView * refactor: streamline navigation logic and enhance loading spinner component in ChatView * refactor: simplify loading spinner logic in ChatView component * fix: ensure message queries are invalidated after new conversation creation in HeaderNewChat, MobileNav, and NewChat components * 🐛 First run dev mode will have error occur. 🐛 First run dev mode will have error occur. * fix font-size localstorage presist bug * Don't ping meilisearch if the search is disabled via env var * simplify logic in search/enable endpoint * refactor: simplify enable endpoint condition check * feat: add useIdChangeEffect hook and integrate it into ChatRoute --------- Co-authored-by: Ne0 <20765145+zeeklog@users.noreply.github.com> Co-authored-by: TinyTin Co-authored-by: Denis Palnitsky --- api/server/routes/search.js | 34 ++------ client/src/components/Chat/ChatView.tsx | 23 +++-- .../components/Chat/Menus/HeaderNewChat.tsx | 1 + .../components/Chat/Messages/MessagesView.tsx | 3 +- .../Chat/Messages/SearchButtons.tsx | 7 +- client/src/components/Conversations/Convo.tsx | 12 +-- client/src/components/Nav/MobileNav.tsx | 1 + client/src/components/Nav/NewChat.tsx | 1 + client/src/components/Share/MessagesView.tsx | 4 +- .../SidePanel/Agents/AgentPanel.tsx | 3 +- client/src/hooks/Agents/useSelectAgent.ts | 2 +- client/src/hooks/Chat/index.ts | 1 + client/src/hooks/Chat/useIdChangeEffect.ts | 21 +++++ .../Conversations/useNavigateToConvo.tsx | 84 +++---------------- client/src/hooks/ThemeContext.tsx | 8 +- client/src/routes/ChatRoute.tsx | 4 +- client/src/utils/getDefaultEndpoint.ts | 2 +- 17 files changed, 80 insertions(+), 131 deletions(-) create mode 100644 client/src/hooks/Chat/useIdChangeEffect.ts diff --git a/api/server/routes/search.js b/api/server/routes/search.js index 36c68cc2b..5c7846aee 100644 --- a/api/server/routes/search.js +++ b/api/server/routes/search.js @@ -1,40 +1,17 @@ -const { Keyv } = require('keyv'); const express = require('express'); const { MeiliSearch } = require('meilisearch'); -const { Conversation } = require('~/models/Conversation'); const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); -const { Message } = require('~/models/Message'); const { isEnabled } = require('~/server/utils'); -const keyvRedis = require('~/cache/keyvRedis'); const router = express.Router(); -const expiration = 60 * 1000; -const cache = isEnabled(process.env.USE_REDIS) - ? new Keyv({ store: keyvRedis }) - : new Keyv({ namespace: 'search', ttl: expiration }); - router.use(requireJwtAuth); -router.get('/sync', async function (req, res) { - await Message.syncWithMeili(); - await Conversation.syncWithMeili(); - res.send('synced'); -}); - -router.get('/test', async function (req, res) { - const { q } = req.query; - const messages = ( - await Message.meiliSearch(q, { attributesToHighlight: ['text'] }, true) - ).hits.map((message) => { - const { _formatted, ...rest } = message; - return { ...rest, searchResult: true, text: _formatted.text }; - }); - res.send(messages); -}); - router.get('/enable', async function (req, res) { - let result = false; + if (!isEnabled(process.env.SEARCH)) { + return res.send(false); + } + try { const client = new MeiliSearch({ host: process.env.MEILI_HOST, @@ -42,8 +19,7 @@ router.get('/enable', async function (req, res) { }); const { status } = await client.health(); - result = status === 'available' && !!process.env.SEARCH; - return res.send(result); + return res.send(status === 'available'); } catch (error) { return res.send(false); } diff --git a/client/src/components/Chat/ChatView.tsx b/client/src/components/Chat/ChatView.tsx index a180bb92e..bce6974c1 100644 --- a/client/src/components/Chat/ChatView.tsx +++ b/client/src/components/Chat/ChatView.tsx @@ -1,4 +1,4 @@ -import { memo, useCallback } from 'react'; +import { memo, useMemo, useCallback } from 'react'; import { useRecoilValue } from 'recoil'; import { useForm } from 'react-hook-form'; import { useParams } from 'react-router-dom'; @@ -19,6 +19,16 @@ import Header from './Header'; import Footer from './Footer'; import store from '~/store'; +function LoadingSpinner() { + return ( +
+
+ +
+
+ ); +} + function ChatView({ index = 0 }: { index?: number }) { const { conversationId } = useParams(); const rootSubmission = useRecoilValue(store.submissionByIndex(index)); @@ -52,15 +62,12 @@ function ChatView({ index = 0 }: { index?: number }) { const isLandingPage = (!messagesTree || messagesTree.length === 0) && (conversationId === Constants.NEW_CONVO || !conversationId); + const isNavigating = (!messagesTree || messagesTree.length === 0) && conversationId != null; if (isLoading && conversationId !== Constants.NEW_CONVO) { - content = ( -
-
- -
-
- ); + content = ; + } else if ((isLoading || isNavigating) && !isLandingPage) { + content = ; } else if (!isLandingPage) { content = ; } else { diff --git a/client/src/components/Chat/Menus/HeaderNewChat.tsx b/client/src/components/Chat/Menus/HeaderNewChat.tsx index e417d6d6d..44e4384d3 100644 --- a/client/src/components/Chat/Menus/HeaderNewChat.tsx +++ b/client/src/components/Chat/Menus/HeaderNewChat.tsx @@ -20,6 +20,7 @@ export default function HeaderNewChat() { [QueryKeys.messages, conversation?.conversationId ?? Constants.NEW_CONVO], [], ); + queryClient.invalidateQueries([QueryKeys.messages]); newConversation(); }; diff --git a/client/src/components/Chat/Messages/MessagesView.tsx b/client/src/components/Chat/Messages/MessagesView.tsx index bf0016c63..bc6e285a5 100644 --- a/client/src/components/Chat/Messages/MessagesView.tsx +++ b/client/src/components/Chat/Messages/MessagesView.tsx @@ -14,10 +14,9 @@ export default function MessagesView({ messagesTree?: TMessage[] | null; }) { const localize = useLocalize(); - const scrollButtonPreference = useRecoilValue(store.showScrollButton); - const maximizeChatSpace = useRecoilValue(store.maximizeChatSpace); const fontSize = useRecoilValue(store.fontSize); const { screenshotTargetRef } = useScreenshot(); + const scrollButtonPreference = useRecoilValue(store.showScrollButton); const [currentEditId, setCurrentEditId] = useState(-1); const { diff --git a/client/src/components/Chat/Messages/SearchButtons.tsx b/client/src/components/Chat/Messages/SearchButtons.tsx index a8abb3afc..46804cefb 100644 --- a/client/src/components/Chat/Messages/SearchButtons.tsx +++ b/client/src/components/Chat/Messages/SearchButtons.tsx @@ -13,7 +13,7 @@ export default function SearchButtons({ message }: { message: TMessage }) { const localize = useLocalize(); const queryClient = useQueryClient(); const search = useRecoilValue(store.search); - const { navigateWithLastTools } = useNavigateToConvo(); + const { navigateToConvo } = useNavigateToConvo(); const conversationId = message.conversationId ?? ''; const clickHandler = async (event: React.MouseEvent) => { @@ -39,14 +39,13 @@ export default function SearchButtons({ message }: { message: TMessage }) { } document.title = title; - navigateWithLastTools( + navigateToConvo( cachedConvo ?? ({ conversationId, title, } as TConversation), - true, - true, + { resetLatestMessage: true }, ); }; diff --git a/client/src/components/Conversations/Convo.tsx b/client/src/components/Conversations/Convo.tsx index 825fc5403..afd14a4cb 100644 --- a/client/src/components/Conversations/Convo.tsx +++ b/client/src/components/Conversations/Convo.tsx @@ -31,11 +31,11 @@ export default function Conversation({ const params = useParams(); const localize = useLocalize(); const { showToast } = useToastContext(); + const { navigateToConvo } = useNavigateToConvo(); + const { data: endpointsConfig } = useGetEndpointsQuery(); const currentConvoId = useMemo(() => params.conversationId, [params.conversationId]); const updateConvoMutation = useUpdateConversationMutation(currentConvoId ?? ''); const activeConvos = useRecoilValue(store.allConversationsSelector); - const { data: endpointsConfig } = useGetEndpointsQuery(); - const { navigateWithLastTools } = useNavigateToConvo(); const isSmallScreen = useMediaQuery('(max-width: 768px)'); const { conversationId, title = '' } = conversation; @@ -118,10 +118,10 @@ export default function Conversation({ document.title = title; } - navigateWithLastTools( - conversation, - !(conversationId ?? '') || conversationId === Constants.NEW_CONVO, - ); + navigateToConvo(conversation, { + currentConvoId, + resetLatestMessage: !(conversationId ?? '') || conversationId === Constants.NEW_CONVO, + }); }; const convoOptionsProps = { diff --git a/client/src/components/Nav/MobileNav.tsx b/client/src/components/Nav/MobileNav.tsx index b25809a3b..e945ec5be 100644 --- a/client/src/components/Nav/MobileNav.tsx +++ b/client/src/components/Nav/MobileNav.tsx @@ -61,6 +61,7 @@ export default function MobileNav({ [QueryKeys.messages, conversation?.conversationId ?? Constants.NEW_CONVO], [], ); + queryClient.invalidateQueries([QueryKeys.messages]); newConversation(); }} > diff --git a/client/src/components/Nav/NewChat.tsx b/client/src/components/Nav/NewChat.tsx index 7b3129adb..ede40688c 100644 --- a/client/src/components/Nav/NewChat.tsx +++ b/client/src/components/Nav/NewChat.tsx @@ -40,6 +40,7 @@ export default function NewChat({ [QueryKeys.messages, conversation?.conversationId ?? Constants.NEW_CONVO], [], ); + queryClient.invalidateQueries([QueryKeys.messages]); newConvo(); navigate('/c/new', { state: { focusChat: true } }); if (isSmallScreen) { diff --git a/client/src/components/Share/MessagesView.tsx b/client/src/components/Share/MessagesView.tsx index 4233a5d6b..2d1105168 100644 --- a/client/src/components/Share/MessagesView.tsx +++ b/client/src/components/Share/MessagesView.tsx @@ -1,6 +1,7 @@ import { useState } from 'react'; import type { TMessage } from 'librechat-data-provider'; import MultiMessage from './MultiMessage'; +import { useLocalize } from '~/hooks'; export default function MessagesView({ messagesTree: _messagesTree, @@ -9,6 +10,7 @@ export default function MessagesView({ messagesTree?: TMessage[] | null; conversationId: string; }) { + const localize = useLocalize(); const [currentEditId, setCurrentEditId] = useState(-1); return (
@@ -23,7 +25,7 @@ export default function MessagesView({
{(_messagesTree && _messagesTree.length == 0) || _messagesTree === null ? (
- Nothing found + {localize('com_ui_nothing_found')}
) : ( <> diff --git a/client/src/components/SidePanel/Agents/AgentPanel.tsx b/client/src/components/SidePanel/Agents/AgentPanel.tsx index 7e0e80be7..23ede096a 100644 --- a/client/src/components/SidePanel/Agents/AgentPanel.tsx +++ b/client/src/components/SidePanel/Agents/AgentPanel.tsx @@ -4,6 +4,7 @@ import { useWatch, useForm, FormProvider } from 'react-hook-form'; import { useGetModelsQuery } from 'librechat-data-provider/react-query'; import { Tools, + Constants, SystemRoles, EModelEndpoint, isAssistantsEndpoint, @@ -45,7 +46,7 @@ export default function AgentPanel({ const modelsQuery = useGetModelsQuery(); const agentQuery = useGetAgentByIdQuery(current_agent_id ?? '', { - enabled: !!(current_agent_id ?? ''), + enabled: !!(current_agent_id ?? '') && current_agent_id !== Constants.EPHEMERAL_AGENT_ID, }); const models = useMemo(() => modelsQuery.data ?? {}, [modelsQuery.data]); diff --git a/client/src/hooks/Agents/useSelectAgent.ts b/client/src/hooks/Agents/useSelectAgent.ts index abeda4c17..bda268062 100644 --- a/client/src/hooks/Agents/useSelectAgent.ts +++ b/client/src/hooks/Agents/useSelectAgent.ts @@ -18,7 +18,7 @@ export default function useSelectAgent() { ); const agentQuery = useGetAgentByIdQuery(selectedAgentId ?? '', { - enabled: !!(selectedAgentId ?? ''), + enabled: !!(selectedAgentId ?? '') && selectedAgentId !== Constants.EPHEMERAL_AGENT_ID, }); const updateConversation = useCallback( diff --git a/client/src/hooks/Chat/index.ts b/client/src/hooks/Chat/index.ts index b9b286c8e..d70146a58 100644 --- a/client/src/hooks/Chat/index.ts +++ b/client/src/hooks/Chat/index.ts @@ -2,4 +2,5 @@ export { default as useChatHelpers } from './useChatHelpers'; export { default as useAddedHelpers } from './useAddedHelpers'; export { default as useAddedResponse } from './useAddedResponse'; export { default as useChatFunctions } from './useChatFunctions'; +export { default as useIdChangeEffect } from './useIdChangeEffect'; export { default as useFocusChatEffect } from './useFocusChatEffect'; diff --git a/client/src/hooks/Chat/useIdChangeEffect.ts b/client/src/hooks/Chat/useIdChangeEffect.ts new file mode 100644 index 000000000..77b066ef3 --- /dev/null +++ b/client/src/hooks/Chat/useIdChangeEffect.ts @@ -0,0 +1,21 @@ +import { useEffect, useRef } from 'react'; +import { useResetRecoilState } from 'recoil'; +import { logger } from '~/utils'; +import store from '~/store'; + +/** + * Hook to reset artifacts when the conversation ID changes + * @param conversationId - The current conversation ID + */ +export default function useIdChangeEffect(conversationId: string) { + const lastConvoId = useRef(null); + const resetArtifacts = useResetRecoilState(store.artifactsState); + + useEffect(() => { + if (conversationId !== lastConvoId.current) { + logger.log('conversation', 'Conversation ID change'); + resetArtifacts(); + } + lastConvoId.current = conversationId; + }, [conversationId, resetArtifacts]); +} diff --git a/client/src/hooks/Conversations/useNavigateToConvo.tsx b/client/src/hooks/Conversations/useNavigateToConvo.tsx index c31fb08a3..2b2981f41 100644 --- a/client/src/hooks/Conversations/useNavigateToConvo.tsx +++ b/client/src/hooks/Conversations/useNavigateToConvo.tsx @@ -1,13 +1,7 @@ +import { useSetRecoilState } from 'recoil'; import { useNavigate } from 'react-router-dom'; import { useQueryClient } from '@tanstack/react-query'; -import { useSetRecoilState, useResetRecoilState } from 'recoil'; -import { - QueryKeys, - Constants, - dataService, - EModelEndpoint, - LocalStorageKeys, -} from 'librechat-data-provider'; +import { QueryKeys, Constants } from 'librechat-data-provider'; import type { TConversation, TEndpointsConfig, TModelsConfig } from 'librechat-data-provider'; import { buildDefaultConvo, getDefaultEndpoint, getEndpointField, logger } from '~/utils'; import store from '~/store'; @@ -16,48 +10,28 @@ const useNavigateToConvo = (index = 0) => { const navigate = useNavigate(); const queryClient = useQueryClient(); const clearAllConversations = store.useClearConvoState(); - const resetArtifacts = useResetRecoilState(store.artifactsState); const setSubmission = useSetRecoilState(store.submissionByIndex(index)); const clearAllLatestMessages = store.useClearLatestMessages(`useNavigateToConvo ${index}`); const { hasSetConversation, setConversation } = store.useCreateConversationAtom(index); - const fetchFreshData = async (conversationId?: string | null) => { - if (!conversationId) { - return; - } - try { - const data = await queryClient.fetchQuery([QueryKeys.conversation, conversationId], () => - dataService.getConversationById(conversationId), - ); - logger.log('conversation', 'Fetched fresh conversation data', data); - await queryClient.invalidateQueries([QueryKeys.messages, conversationId]); - setConversation(data); - navigate(`/c/${conversationId ?? Constants.NEW_CONVO}`, { state: { focusChat: true } }); - } catch (error) { - console.error('Error fetching conversation data on navigation', error); - } - }; - const navigateToConvo = ( conversation?: TConversation | null, - _resetLatestMessage = true, - /** Likely need to remove this since it happens after fetching conversation data */ - invalidateMessages = false, + options?: { + resetLatestMessage?: boolean; + currentConvoId?: string; + }, ) => { if (!conversation) { logger.warn('conversation', 'Conversation not provided to `navigateToConvo`'); return; } + const { resetLatestMessage = true, currentConvoId } = options || {}; logger.log('conversation', 'Navigating to conversation', conversation); hasSetConversation.current = true; setSubmission(null); - if (_resetLatestMessage) { + if (resetLatestMessage) { clearAllLatestMessages(); } - if (invalidateMessages && conversation.conversationId != null && conversation.conversationId) { - queryClient.setQueryData([QueryKeys.messages, Constants.NEW_CONVO], []); - queryClient.invalidateQueries([QueryKeys.messages, conversation.conversationId]); - } let convo = { ...conversation }; const endpointsConfig = queryClient.getQueryData([QueryKeys.endpoints]); @@ -84,51 +58,13 @@ const useNavigateToConvo = (index = 0) => { }); } clearAllConversations(true); - resetArtifacts(); setConversation(convo); - if (convo.conversationId !== Constants.NEW_CONVO && convo.conversationId) { - queryClient.invalidateQueries([QueryKeys.conversation, convo.conversationId]); - fetchFreshData(convo.conversationId); - } else { - navigate(`/c/${convo.conversationId ?? Constants.NEW_CONVO}`, { state: { focusChat: true } }); - } - }; - - const navigateWithLastTools = ( - conversation?: TConversation | null, - _resetLatestMessage?: boolean, - invalidateMessages?: boolean, - ) => { - if (!conversation) { - logger.warn('conversation', 'Conversation not provided to `navigateToConvo`'); - return; - } - // set conversation to the new conversation - if (conversation.endpoint === EModelEndpoint.gptPlugins) { - let lastSelectedTools = []; - try { - lastSelectedTools = - JSON.parse(localStorage.getItem(LocalStorageKeys.LAST_TOOLS) ?? '') ?? []; - } catch (e) { - logger.error('conversation', 'Error parsing last selected tools', e); - } - const hasTools = (conversation.tools?.length ?? 0) > 0; - navigateToConvo( - { - ...conversation, - tools: hasTools ? conversation.tools : lastSelectedTools, - }, - _resetLatestMessage, - invalidateMessages, - ); - } else { - navigateToConvo(conversation, _resetLatestMessage, invalidateMessages); - } + queryClient.setQueryData([QueryKeys.messages, currentConvoId], []); + navigate(`/c/${convo.conversationId ?? Constants.NEW_CONVO}`, { state: { focusChat: true } }); }; return { navigateToConvo, - navigateWithLastTools, }; }; diff --git a/client/src/hooks/ThemeContext.tsx b/client/src/hooks/ThemeContext.tsx index df516eec4..76b272598 100644 --- a/client/src/hooks/ThemeContext.tsx +++ b/client/src/hooks/ThemeContext.tsx @@ -58,10 +58,14 @@ export const ThemeProvider = ({ initialTheme, children }) => { if (fontSize == null) { setFontSize('text-base'); applyFontSize('text-base'); - localStorage.setItem('fontSize', 'text-base'); + localStorage.setItem('fontSize', JSON.stringify('text-base')); return; } - applyFontSize(JSON.parse(fontSize)); + try { + applyFontSize(JSON.parse(fontSize)); + } catch (error) { + console.log(error); + } // Reason: This effect should only run once, and `setFontSize` is a stable function // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/client/src/routes/ChatRoute.tsx b/client/src/routes/ChatRoute.tsx index 48382fc54..3ae8eb853 100644 --- a/client/src/routes/ChatRoute.tsx +++ b/client/src/routes/ChatRoute.tsx @@ -9,8 +9,8 @@ import { useGetStartupConfig, useGetEndpointsQuery, } from '~/data-provider'; +import { useNewConvo, useAppStartup, useAssistantListMap, useIdChangeEffect } from '~/hooks'; import { getDefaultModelSpec, getModelSpecPreset, logger } from '~/utils'; -import { useNewConvo, useAppStartup, useAssistantListMap } from '~/hooks'; import { ToolCallsMapProvider } from '~/Providers'; import ChatView from '~/components/Chat/ChatView'; import useAuthRedirect from './useAuthRedirect'; @@ -34,7 +34,7 @@ export default function ChatRoute() { const index = 0; const { conversationId = '' } = useParams(); - + useIdChangeEffect(conversationId); const { hasSetConversation, conversation } = store.useCreateConversationAtom(index); const { newConversation } = useNewConvo(); diff --git a/client/src/utils/getDefaultEndpoint.ts b/client/src/utils/getDefaultEndpoint.ts index a9267b339..5a93125d3 100644 --- a/client/src/utils/getDefaultEndpoint.ts +++ b/client/src/utils/getDefaultEndpoint.ts @@ -20,7 +20,7 @@ const getEndpointFromSetup = ( if (targetEndpoint && endpointsConfig?.[targetEndpoint]) { return targetEndpoint as EModelEndpoint; } else if (targetEndpoint) { - console.warn(`Illegal target endpoint ${targetEndpoint} ${endpointsConfig}`); + console.warn(`Illegal target endpoint ${targetEndpoint}`, endpointsConfig); } return null; };