mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-16 16:30:15 +01:00
🎯 refactor: Centralize Agent Model Handling Across Conversation Lifecycle (#10956)
* refactor: Implement clearModelForNonEphemeralAgent utility for improved agent handling - Introduced clearModelForNonEphemeralAgent function to manage model state for non-ephemeral agents across various components. - Updated ModelSelectorContext to initialize model based on agent type. - Enhanced useNavigateToConvo, useQueryParams, and useSelectMention hooks to clear model for non-ephemeral agents. - Refactored buildDefaultConvo and endpoints utility to ensure proper handling of agent_id and model state. - Improved overall conversation logic and state management for better performance and reliability. * refactor: Enhance useNewConvo hook to improve agent conversation handling - Added logic to skip access checks for existing agent conversations, utilizing localStorage to restore conversations after refresh. - Improved handling of default endpoints for agents based on user access and existing conversation state, ensuring more reliable conversation initialization. * refactor: Update ChatRoute and useAuthRedirect to include user roles - Enhanced ChatRoute to utilize user roles for improved conversation state management. - Modified useAuthRedirect to return user roles alongside authentication status, ensuring roles are available for conditional logic in ChatRoute. - Adjusted conversation initialization logic to depend on the loaded roles, enhancing the reliability of the conversation setup. * refactor: Update BaseClient to handle non-ephemeral agents in conversation logic - Added a check for non-ephemeral agents in BaseClient, modifying the exceptions set to include 'model' when applicable. - Enhanced conversation handling to improve flexibility based on agent type. * test: Add mock for clearModelForNonEphemeralAgent in useQueryParams tests - Introduced a mock for clearModelForNonEphemeralAgent to enhance testing of query parameters related to non-ephemeral agents. - This addition supports improved test coverage and ensures proper handling of model state in relevant scenarios. * refactor: Simplify mocks in useQueryParams tests for improved clarity - Updated the mocking strategy for utilities in useQueryParams tests to use actual implementations where possible, while still suppressing test output for the logger. - Enhanced the mock for tQueryParamsSchema to minimize complexity and avoid unnecessary validation during tests, improving test reliability and maintainability. * refactor: Enhance agent identification logic in BaseClient for improved clarity * chore: Import Constants in families.ts for enhanced functionality
This commit is contained in:
parent
06719794f6
commit
b5ab32c5ae
12 changed files with 161 additions and 49 deletions
|
|
@ -966,6 +966,13 @@ class BaseClient {
|
|||
|
||||
const unsetFields = {};
|
||||
const exceptions = new Set(['spec', 'iconURL']);
|
||||
const hasNonEphemeralAgent =
|
||||
isAgentsEndpoint(this.options.endpoint) &&
|
||||
endpointOptions?.agent_id &&
|
||||
endpointOptions.agent_id !== Constants.EPHEMERAL_AGENT_ID;
|
||||
if (hasNonEphemeralAgent) {
|
||||
exceptions.add('model');
|
||||
}
|
||||
if (existingConvo != null) {
|
||||
this.fetchedConvo = true;
|
||||
for (const key in existingConvo) {
|
||||
|
|
|
|||
|
|
@ -104,10 +104,18 @@ export function ModelSelectorProvider({ children, startupConfig }: ModelSelector
|
|||
});
|
||||
|
||||
// State
|
||||
const [selectedValues, setSelectedValues] = useState<SelectedValues>({
|
||||
endpoint: endpoint || '',
|
||||
model: model || '',
|
||||
modelSpec: spec || '',
|
||||
const [selectedValues, setSelectedValues] = useState<SelectedValues>(() => {
|
||||
let initialModel = model || '';
|
||||
if (isAgentsEndpoint(endpoint) && agent_id) {
|
||||
initialModel = agent_id;
|
||||
} else if (isAssistantsEndpoint(endpoint) && assistant_id) {
|
||||
initialModel = assistant_id;
|
||||
}
|
||||
return {
|
||||
endpoint: endpoint || '',
|
||||
model: initialModel,
|
||||
modelSpec: spec || '',
|
||||
};
|
||||
});
|
||||
useSelectorEffects({
|
||||
agentsMap,
|
||||
|
|
|
|||
|
|
@ -9,7 +9,13 @@ import type {
|
|||
TModelsConfig,
|
||||
TConversation,
|
||||
} from 'librechat-data-provider';
|
||||
import { getDefaultEndpoint, clearMessagesCache, buildDefaultConvo, logger } from '~/utils';
|
||||
import {
|
||||
clearModelForNonEphemeralAgent,
|
||||
getDefaultEndpoint,
|
||||
clearMessagesCache,
|
||||
buildDefaultConvo,
|
||||
logger,
|
||||
} from '~/utils';
|
||||
import { useApplyModelSpecEffects } from '~/hooks/Agents';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -49,7 +55,10 @@ const useNavigateToConvo = (index = 0) => {
|
|||
dataService.getConversationById(conversationId),
|
||||
);
|
||||
logger.log('conversation', 'Fetched fresh conversation data', data);
|
||||
setConversation(data);
|
||||
|
||||
const convoData = { ...data };
|
||||
clearModelForNonEphemeralAgent(convoData);
|
||||
setConversation(convoData);
|
||||
navigate(`/c/${conversationId ?? Constants.NEW_CONVO}`, { state: { focusChat: true } });
|
||||
} catch (error) {
|
||||
console.error('Error fetching conversation data on navigation', error);
|
||||
|
|
|
|||
|
|
@ -65,38 +65,33 @@ jest.mock('~/hooks/Agents/useAgentDefaultPermissionLevel', () => ({
|
|||
default: jest.fn(() => ({})),
|
||||
}));
|
||||
|
||||
jest.mock('~/utils', () => ({
|
||||
getConvoSwitchLogic: jest.fn(() => ({
|
||||
template: {},
|
||||
shouldSwitch: false,
|
||||
isNewModular: false,
|
||||
newEndpointType: null,
|
||||
isCurrentModular: false,
|
||||
isExistingConversation: false,
|
||||
})),
|
||||
getModelSpecIconURL: jest.fn(() => 'icon-url'),
|
||||
removeUnavailableTools: jest.fn((preset) => preset),
|
||||
logger: { log: jest.fn() },
|
||||
getInitialTheme: jest.fn(() => 'light'),
|
||||
applyFontSize: jest.fn(),
|
||||
}));
|
||||
jest.mock('~/utils', () => {
|
||||
const actualUtils = jest.requireActual('~/utils');
|
||||
return {
|
||||
...actualUtils,
|
||||
// Only mock logger to suppress test output
|
||||
logger: { log: jest.fn(), warn: jest.fn(), error: jest.fn() },
|
||||
// Mock theme utilities that interact with DOM
|
||||
getInitialTheme: jest.fn(() => 'light'),
|
||||
applyFontSize: jest.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
// Mock the tQueryParamsSchema
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
...jest.requireActual('librechat-data-provider'),
|
||||
tQueryParamsSchema: {
|
||||
shape: {
|
||||
model: { parse: jest.fn((value) => value) },
|
||||
endpoint: { parse: jest.fn((value) => value) },
|
||||
temperature: { parse: jest.fn((value) => value) },
|
||||
// Add other schema shapes as needed
|
||||
// Use actual librechat-data-provider with minimal overrides
|
||||
jest.mock('librechat-data-provider', () => {
|
||||
const actual = jest.requireActual('librechat-data-provider');
|
||||
return {
|
||||
...actual,
|
||||
// Override schema to avoid complex validation in tests
|
||||
tQueryParamsSchema: {
|
||||
shape: {
|
||||
model: { parse: jest.fn((value) => value) },
|
||||
endpoint: { parse: jest.fn((value) => value) },
|
||||
temperature: { parse: jest.fn((value) => value) },
|
||||
},
|
||||
},
|
||||
},
|
||||
isAgentsEndpoint: jest.fn(() => false),
|
||||
isAssistantsEndpoint: jest.fn(() => false),
|
||||
QueryKeys: { startupConfig: 'startupConfig', endpoints: 'endpoints' },
|
||||
EModelEndpoint: { custom: 'custom', assistants: 'assistants', agents: 'agents' },
|
||||
}));
|
||||
};
|
||||
});
|
||||
|
||||
// Mock data-provider hooks
|
||||
jest.mock('~/data-provider', () => ({
|
||||
|
|
|
|||
|
|
@ -11,13 +11,19 @@ import {
|
|||
PermissionBits,
|
||||
} from 'librechat-data-provider';
|
||||
import type {
|
||||
TPreset,
|
||||
AgentListResponse,
|
||||
TEndpointsConfig,
|
||||
TStartupConfig,
|
||||
AgentListResponse,
|
||||
TPreset,
|
||||
} from 'librechat-data-provider';
|
||||
import type { ZodAny } from 'zod';
|
||||
import { getConvoSwitchLogic, getModelSpecIconURL, removeUnavailableTools, logger } from '~/utils';
|
||||
import {
|
||||
clearModelForNonEphemeralAgent,
|
||||
removeUnavailableTools,
|
||||
getModelSpecIconURL,
|
||||
getConvoSwitchLogic,
|
||||
logger,
|
||||
} from '~/utils';
|
||||
import { useAuthContext, useAgentsMap, useDefaultConvo, useSubmitMessage } from '~/hooks';
|
||||
import { useChatContext, useChatFormContext } from '~/Providers';
|
||||
import { useGetAgentByIdQuery } from '~/data-provider';
|
||||
|
|
@ -194,6 +200,12 @@ export default function useQueryParams({
|
|||
newPreset = { ...newPreset, ...resetParams };
|
||||
}
|
||||
|
||||
// Sync agent_id from newPreset to template, then clear model if non-ephemeral agent
|
||||
if (newPreset.agent_id) {
|
||||
template.agent_id = newPreset.agent_id;
|
||||
}
|
||||
clearModelForNonEphemeralAgent(template);
|
||||
|
||||
const isModular = isCurrentModular && isNewModular && shouldSwitch;
|
||||
if (isExistingConversation && isModular) {
|
||||
template.endpointType = newEndpointType as EModelEndpoint | undefined;
|
||||
|
|
|
|||
|
|
@ -9,7 +9,13 @@ import type {
|
|||
TEndpointsConfig,
|
||||
} from 'librechat-data-provider';
|
||||
import type { MentionOption, ConvoGenerator } from '~/common';
|
||||
import { getConvoSwitchLogic, getModelSpecIconURL, removeUnavailableTools, logger } from '~/utils';
|
||||
import {
|
||||
clearModelForNonEphemeralAgent,
|
||||
removeUnavailableTools,
|
||||
getModelSpecIconURL,
|
||||
getConvoSwitchLogic,
|
||||
logger,
|
||||
} from '~/utils';
|
||||
import { useDefaultConvo } from '~/hooks';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -154,6 +160,7 @@ export default function useSelectMention({
|
|||
if (agent_id) {
|
||||
template.agent_id = agent_id;
|
||||
}
|
||||
clearModelForNonEphemeralAgent(template);
|
||||
|
||||
template.spec = null;
|
||||
template.iconURL = null;
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import type {
|
|||
import type { AssistantListItem } from '~/common';
|
||||
import {
|
||||
updateLastSelectedModel,
|
||||
getLocalStorageItems,
|
||||
getDefaultModelSpec,
|
||||
getDefaultEndpoint,
|
||||
getModelSpecPreset,
|
||||
|
|
@ -112,7 +113,21 @@ const useNewConvo = (index = 0) => {
|
|||
});
|
||||
|
||||
// If the selected endpoint is agents but user doesn't have access, find an alternative
|
||||
if (defaultEndpoint && isAgentsEndpoint(defaultEndpoint) && !hasAgentAccess) {
|
||||
// Skip this check for existing agent conversations (they have agent_id set)
|
||||
// Also check localStorage for new conversations restored after refresh
|
||||
const { lastConversationSetup } = getLocalStorageItems();
|
||||
const storedAgentId =
|
||||
isAgentsEndpoint(lastConversationSetup?.endpoint) && lastConversationSetup?.agent_id;
|
||||
const isExistingAgentConvo =
|
||||
isAgentsEndpoint(defaultEndpoint) &&
|
||||
((conversation.agent_id && conversation.agent_id !== Constants.EPHEMERAL_AGENT_ID) ||
|
||||
(storedAgentId && storedAgentId !== Constants.EPHEMERAL_AGENT_ID));
|
||||
if (
|
||||
defaultEndpoint &&
|
||||
isAgentsEndpoint(defaultEndpoint) &&
|
||||
!hasAgentAccess &&
|
||||
!isExistingAgentConvo
|
||||
) {
|
||||
defaultEndpoint = Object.keys(endpointsConfig ?? {}).find(
|
||||
(ep) => !isAgentsEndpoint(ep as EModelEndpoint) && endpointsConfig?.[ep],
|
||||
) as EModelEndpoint | undefined;
|
||||
|
|
@ -121,7 +136,11 @@ const useNewConvo = (index = 0) => {
|
|||
if (!defaultEndpoint) {
|
||||
// Find first available endpoint that's not agents (if no access) or any endpoint
|
||||
defaultEndpoint = Object.keys(endpointsConfig ?? {}).find((ep) => {
|
||||
if (isAgentsEndpoint(ep as EModelEndpoint) && !hasAgentAccess) {
|
||||
if (
|
||||
isAgentsEndpoint(ep as EModelEndpoint) &&
|
||||
!hasAgentAccess &&
|
||||
!isExistingAgentConvo
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return !!endpointsConfig?.[ep];
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import store from '~/store';
|
|||
|
||||
export default function ChatRoute() {
|
||||
const { data: startupConfig } = useGetStartupConfig();
|
||||
const { isAuthenticated, user } = useAuthRedirect();
|
||||
const { isAuthenticated, user, roles } = useAuthRedirect();
|
||||
|
||||
const defaultTemporaryChat = useRecoilValue(temporaryStore.defaultTemporaryChat);
|
||||
const setIsTemporary = useRecoilCallback(
|
||||
|
|
@ -61,8 +61,11 @@ export default function ChatRoute() {
|
|||
* Adjusting this may have unintended consequences on the conversation state.
|
||||
*/
|
||||
useEffect(() => {
|
||||
// Wait for roles to load so hasAgentAccess has a definitive value in useNewConvo
|
||||
const rolesLoaded = roles?.USER != null;
|
||||
const shouldSetConvo =
|
||||
(startupConfig && !hasSetConversation.current && !modelsQuery.data?.initial) ?? false;
|
||||
(startupConfig && rolesLoaded && !hasSetConversation.current && !modelsQuery.data?.initial) ??
|
||||
false;
|
||||
/* Early exit if startupConfig is not loaded and conversation is already set and only initial models have loaded */
|
||||
if (!shouldSetConvo) {
|
||||
return;
|
||||
|
|
@ -119,6 +122,7 @@ export default function ChatRoute() {
|
|||
/* Creates infinite render if all dependencies included due to newConversation invocations exceeding call stack before hasSetConversation.current becomes truthy */
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [
|
||||
roles,
|
||||
startupConfig,
|
||||
initialConvoQuery.data,
|
||||
endpointsQuery.data,
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { useNavigate } from 'react-router-dom';
|
|||
import { useAuthContext } from '~/hooks';
|
||||
|
||||
export default function useAuthRedirect() {
|
||||
const { user, isAuthenticated } = useAuthContext();
|
||||
const { user, roles, isAuthenticated } = useAuthContext();
|
||||
const navigate = useNavigate();
|
||||
|
||||
useEffect(() => {
|
||||
|
|
@ -20,6 +20,7 @@ export default function useAuthRedirect() {
|
|||
|
||||
return {
|
||||
user,
|
||||
roles,
|
||||
isAuthenticated,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { useEffect } from 'react';
|
||||
import { createSearchParams } from 'react-router-dom';
|
||||
import {
|
||||
atom,
|
||||
selector,
|
||||
|
|
@ -13,9 +14,13 @@ import {
|
|||
import { LocalStorageKeys, Constants } from 'librechat-data-provider';
|
||||
import type { TMessage, TPreset, TConversation, TSubmission } from 'librechat-data-provider';
|
||||
import type { TOptionSettings, ExtendedFile } from '~/common';
|
||||
import {
|
||||
clearModelForNonEphemeralAgent,
|
||||
createChatSearchParams,
|
||||
storeEndpointSettings,
|
||||
logger,
|
||||
} from '~/utils';
|
||||
import { useSetConvoContext } from '~/Providers/SetConvoContext';
|
||||
import { storeEndpointSettings, logger, createChatSearchParams } from '~/utils';
|
||||
import { createSearchParams } from 'react-router-dom';
|
||||
|
||||
const latestMessageKeysAtom = atom<(string | number)[]>({
|
||||
key: 'latestMessageKeys',
|
||||
|
|
@ -101,9 +106,12 @@ const conversationByIndex = atomFamily<TConversation | null, string | number>({
|
|||
}
|
||||
|
||||
storeEndpointSettings(newValue);
|
||||
|
||||
const convoToStore = { ...newValue };
|
||||
clearModelForNonEphemeralAgent(convoToStore);
|
||||
localStorage.setItem(
|
||||
`${LocalStorageKeys.LAST_CONVO_SETUP}_${index}`,
|
||||
JSON.stringify(newValue),
|
||||
JSON.stringify(convoToStore),
|
||||
);
|
||||
|
||||
const disableParams = newValue.disableParams === true;
|
||||
|
|
|
|||
|
|
@ -1,10 +1,12 @@
|
|||
import {
|
||||
Constants,
|
||||
parseConvo,
|
||||
EModelEndpoint,
|
||||
isAssistantsEndpoint,
|
||||
isAgentsEndpoint,
|
||||
} from 'librechat-data-provider';
|
||||
import type { TConversation, EndpointSchemaKey } from 'librechat-data-provider';
|
||||
import { clearModelForNonEphemeralAgent } from './endpoints';
|
||||
import { getLocalStorageItems } from './localStorage';
|
||||
|
||||
const buildDefaultConvo = ({
|
||||
|
|
@ -66,10 +68,17 @@ const buildDefaultConvo = ({
|
|||
// Ensures agent_id is always defined
|
||||
const agentId = convo?.agent_id ?? '';
|
||||
const defaultAgentId = lastConversationSetup?.agent_id ?? '';
|
||||
if (isAgentsEndpoint(endpoint) && !defaultAgentId && agentId) {
|
||||
if (
|
||||
isAgentsEndpoint(endpoint) &&
|
||||
agentId &&
|
||||
(!defaultAgentId || defaultAgentId === Constants.EPHEMERAL_AGENT_ID)
|
||||
) {
|
||||
defaultConvo.agent_id = agentId;
|
||||
}
|
||||
|
||||
// Clear model for non-ephemeral agents - agents use their configured model internally
|
||||
clearModelForNonEphemeralAgent(defaultConvo);
|
||||
|
||||
defaultConvo.tools = lastConversationSetup?.tools ?? lastSelectedTools ?? defaultConvo.tools;
|
||||
|
||||
return defaultConvo;
|
||||
|
|
|
|||
|
|
@ -11,6 +11,27 @@ import {
|
|||
import type * as t from 'librechat-data-provider';
|
||||
import type { LocalizeFunction, IconsRecord } from '~/common';
|
||||
|
||||
/**
|
||||
* Clears model for non-ephemeral agent conversations.
|
||||
* Agents use their configured model internally, so the conversation model should be undefined.
|
||||
* Mutates the template in place.
|
||||
*/
|
||||
export function clearModelForNonEphemeralAgent<
|
||||
T extends {
|
||||
endpoint?: EModelEndpoint | string | null;
|
||||
agent_id?: string | null;
|
||||
model?: string | null;
|
||||
},
|
||||
>(template: T): void {
|
||||
if (
|
||||
isAgentsEndpoint(template.endpoint) &&
|
||||
template.agent_id &&
|
||||
template.agent_id !== Constants.EPHEMERAL_AGENT_ID
|
||||
) {
|
||||
template.model = undefined as T['model'];
|
||||
}
|
||||
}
|
||||
|
||||
export const getEntityName = ({
|
||||
name = '',
|
||||
localize,
|
||||
|
|
@ -125,6 +146,18 @@ export function getConvoSwitchLogic(params: ConversationInitParams): InitiatedTe
|
|||
conversationId: 'new',
|
||||
};
|
||||
|
||||
// Reset agent_id if switching to a non-agents endpoint but template has a non-ephemeral agent_id
|
||||
if (
|
||||
!isAgentsEndpoint(newEndpoint) &&
|
||||
template.agent_id &&
|
||||
template.agent_id !== Constants.EPHEMERAL_AGENT_ID
|
||||
) {
|
||||
template.agent_id = Constants.EPHEMERAL_AGENT_ID;
|
||||
}
|
||||
|
||||
// Clear model for non-ephemeral agents - agents use their configured model internally
|
||||
clearModelForNonEphemeralAgent(template);
|
||||
|
||||
const isAssistantSwitch =
|
||||
isAssistantsEndpoint(newEndpoint) &&
|
||||
isAssistantsEndpoint(currentEndpoint) &&
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue