mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-03 14:50:19 +01:00
👥 fix: Duplicate Indicators for Model Specs (#11946)
* fix: key checkmark by endpoint , not just model name * fix: model spec endpoint collision for checkmark indicators * chore: fix formatting * refactor: move isSelected into EndpointModelItem, fix SearchResults, add tests Address PR review feedback: - Move isSelected computation from renderEndpointModels into EndpointModelItem via useModelSelectorContext, eliminating fragile positional params - Add !selectedSpec guard to SearchResults.tsx for both model and endpoint checks - Add unit tests for EndpointModelItem selection logic * test: update EndpointModelItem tests and add SearchResults tests - Update EndpointModelItem tests to replace null modelSpec with an empty string for consistency in rendering logic. - Introduce new SearchResults tests to validate selection behavior based on endpoint and model matching, including scenarios with and without active specs. --------- Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
b18915a96b
commit
93560f5f5b
5 changed files with 212 additions and 18 deletions
|
|
@ -96,7 +96,7 @@ function EndpointMenuContent({
|
|||
const localize = useLocalize();
|
||||
const { agentsMap, assistantsMap, modelSpecs, selectedValues, endpointSearchValues } =
|
||||
useModelSelectorContext();
|
||||
const { model: selectedModel, modelSpec: selectedSpec } = selectedValues;
|
||||
const { modelSpec: selectedSpec } = selectedValues;
|
||||
const searchValue = endpointSearchValues[endpoint.value] || '';
|
||||
|
||||
const endpointSpecs = useMemo(() => {
|
||||
|
|
@ -134,15 +134,9 @@ function EndpointMenuContent({
|
|||
<ModelSpecItem key={spec.name} spec={spec} isSelected={selectedSpec === spec.name} />
|
||||
))}
|
||||
{filteredModels
|
||||
? renderEndpointModels(
|
||||
endpoint,
|
||||
endpoint.models || [],
|
||||
selectedModel,
|
||||
filteredModels,
|
||||
endpointIndex,
|
||||
)
|
||||
? renderEndpointModels(endpoint, endpoint.models || [], filteredModels, endpointIndex)
|
||||
: endpoint.models &&
|
||||
renderEndpointModels(endpoint, endpoint.models, selectedModel, undefined, endpointIndex)}
|
||||
renderEndpointModels(endpoint, endpoint.models, undefined, endpointIndex)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
|
@ -157,7 +151,7 @@ export function EndpointItem({ endpoint, endpointIndex }: EndpointItemProps) {
|
|||
setEndpointSearchValue,
|
||||
endpointRequiresUserKey,
|
||||
} = useModelSelectorContext();
|
||||
const { endpoint: selectedEndpoint } = selectedValues;
|
||||
const { endpoint: selectedEndpoint, modelSpec: selectedSpec } = selectedValues;
|
||||
|
||||
const searchValue = endpointSearchValues[endpoint.value] || '';
|
||||
const isUserProvided = useMemo(
|
||||
|
|
@ -179,7 +173,7 @@ export function EndpointItem({ endpoint, endpointIndex }: EndpointItemProps) {
|
|||
</div>
|
||||
);
|
||||
|
||||
const isEndpointSelected = selectedEndpoint === endpoint.value;
|
||||
const isEndpointSelected = !selectedSpec && selectedEndpoint === endpoint.value;
|
||||
|
||||
if (endpoint.hasModels) {
|
||||
const placeholder =
|
||||
|
|
|
|||
|
|
@ -11,12 +11,18 @@ import { cn } from '~/utils';
|
|||
interface EndpointModelItemProps {
|
||||
modelId: string | null;
|
||||
endpoint: Endpoint;
|
||||
isSelected: boolean;
|
||||
}
|
||||
|
||||
export function EndpointModelItem({ modelId, endpoint, isSelected }: EndpointModelItemProps) {
|
||||
export function EndpointModelItem({ modelId, endpoint }: EndpointModelItemProps) {
|
||||
const localize = useLocalize();
|
||||
const { handleSelectModel } = useModelSelectorContext();
|
||||
const { handleSelectModel, selectedValues } = useModelSelectorContext();
|
||||
const {
|
||||
endpoint: selectedEndpoint,
|
||||
model: selectedModel,
|
||||
modelSpec: selectedSpec,
|
||||
} = selectedValues;
|
||||
const isSelected =
|
||||
!selectedSpec && selectedEndpoint === endpoint.value && selectedModel === modelId;
|
||||
const { isFavoriteModel, toggleFavoriteModel, isFavoriteAgent, toggleFavoriteAgent } =
|
||||
useFavorites();
|
||||
|
||||
|
|
@ -147,7 +153,6 @@ export function EndpointModelItem({ modelId, endpoint, isSelected }: EndpointMod
|
|||
export function renderEndpointModels(
|
||||
endpoint: Endpoint | null,
|
||||
models: Array<{ name: string; isGlobal?: boolean }>,
|
||||
selectedModel: string | null,
|
||||
filteredModels?: string[],
|
||||
endpointIndex?: number,
|
||||
) {
|
||||
|
|
@ -161,7 +166,6 @@ export function renderEndpointModels(
|
|||
key={`${endpoint.value}${indexSuffix}-${modelId}-${modelIndex}`}
|
||||
modelId={modelId}
|
||||
endpoint={endpoint}
|
||||
isSelected={selectedModel === modelId}
|
||||
/>
|
||||
),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -160,7 +160,9 @@ export function SearchResults({ results, localize, searchValue }: SearchResultsP
|
|||
}
|
||||
|
||||
const isModelSelected =
|
||||
selectedEndpoint === endpoint.value && selectedModel === modelId;
|
||||
!selectedSpec &&
|
||||
selectedEndpoint === endpoint.value &&
|
||||
selectedModel === modelId;
|
||||
return (
|
||||
<MenuItem
|
||||
key={`${endpoint.value}-${modelId}-search-${i}`}
|
||||
|
|
@ -199,7 +201,7 @@ export function SearchResults({ results, localize, searchValue }: SearchResultsP
|
|||
);
|
||||
} else {
|
||||
// Endpoints with no models
|
||||
const isEndpointSelected = selectedEndpoint === endpoint.value;
|
||||
const isEndpointSelected = !selectedSpec && selectedEndpoint === endpoint.value;
|
||||
return (
|
||||
<MenuItem
|
||||
key={`endpoint-${endpoint.value}-search-item`}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,85 @@
|
|||
import { render, screen } from '@testing-library/react';
|
||||
import type { Endpoint, SelectedValues } from '~/common';
|
||||
import { EndpointModelItem } from '../EndpointModelItem';
|
||||
|
||||
const mockHandleSelectModel = jest.fn();
|
||||
let mockSelectedValues: SelectedValues;
|
||||
|
||||
jest.mock('~/components/Chat/Menus/Endpoints/ModelSelectorContext', () => ({
|
||||
useModelSelectorContext: () => ({
|
||||
handleSelectModel: mockHandleSelectModel,
|
||||
selectedValues: mockSelectedValues,
|
||||
}),
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Menus/Endpoints/CustomMenu', () => {
|
||||
const React = jest.requireActual<typeof import('react')>('react');
|
||||
return {
|
||||
CustomMenuItem: React.forwardRef(function MockMenuItem(
|
||||
{ children, ...rest }: { children?: React.ReactNode },
|
||||
ref: React.Ref<HTMLDivElement>,
|
||||
) {
|
||||
return React.createElement('div', { ref, role: 'menuitem', ...rest }, children);
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useLocalize: () => (key: string) => key,
|
||||
useFavorites: () => ({
|
||||
isFavoriteModel: () => false,
|
||||
toggleFavoriteModel: jest.fn(),
|
||||
isFavoriteAgent: () => false,
|
||||
toggleFavoriteAgent: jest.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
const baseEndpoint: Endpoint = {
|
||||
value: 'anthropic',
|
||||
label: 'Anthropic',
|
||||
hasModels: true,
|
||||
models: [{ name: 'claude-opus-4-6' }],
|
||||
icon: null,
|
||||
};
|
||||
|
||||
describe('EndpointModelItem', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('renders checkmark when model and endpoint match with no active spec', () => {
|
||||
mockSelectedValues = { endpoint: 'anthropic', model: 'claude-opus-4-6', modelSpec: '' };
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).toHaveAttribute('aria-selected', 'true');
|
||||
});
|
||||
|
||||
it('does NOT render checkmark when a model spec is active even if endpoint and model match', () => {
|
||||
mockSelectedValues = {
|
||||
endpoint: 'anthropic',
|
||||
model: 'claude-opus-4-6',
|
||||
modelSpec: 'my-anthropic-spec',
|
||||
};
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).not.toHaveAttribute('aria-selected');
|
||||
});
|
||||
|
||||
it('does NOT render checkmark when model matches but endpoint differs', () => {
|
||||
mockSelectedValues = { endpoint: 'openai', model: 'claude-opus-4-6', modelSpec: '' };
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).not.toHaveAttribute('aria-selected');
|
||||
});
|
||||
|
||||
it('does NOT render checkmark when endpoint matches but model differs', () => {
|
||||
mockSelectedValues = { endpoint: 'anthropic', model: 'claude-sonnet-4-5', modelSpec: '' };
|
||||
render(<EndpointModelItem modelId="claude-opus-4-6" endpoint={baseEndpoint} />);
|
||||
|
||||
const menuItem = screen.getByRole('menuitem');
|
||||
expect(menuItem).not.toHaveAttribute('aria-selected');
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,109 @@
|
|||
import { render, screen } from '@testing-library/react';
|
||||
import type { Endpoint, SelectedValues } from '~/common';
|
||||
import { SearchResults } from '../SearchResults';
|
||||
|
||||
const mockHandleSelectSpec = jest.fn();
|
||||
const mockHandleSelectModel = jest.fn();
|
||||
const mockHandleSelectEndpoint = jest.fn();
|
||||
let mockSelectedValues: SelectedValues;
|
||||
|
||||
jest.mock('~/components/Chat/Menus/Endpoints/ModelSelectorContext', () => ({
|
||||
useModelSelectorContext: () => ({
|
||||
selectedValues: mockSelectedValues,
|
||||
handleSelectSpec: mockHandleSelectSpec,
|
||||
handleSelectModel: mockHandleSelectModel,
|
||||
handleSelectEndpoint: mockHandleSelectEndpoint,
|
||||
endpointsConfig: {},
|
||||
}),
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Menus/Endpoints/CustomMenu', () => {
|
||||
const React = jest.requireActual<typeof import('react')>('react');
|
||||
return {
|
||||
CustomMenuItem: React.forwardRef(function MockMenuItem(
|
||||
{ children, ...rest }: { children?: React.ReactNode },
|
||||
ref: React.Ref<HTMLDivElement>,
|
||||
) {
|
||||
return React.createElement('div', { ref, role: 'menuitem', ...rest }, children);
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('../SpecIcon', () => {
|
||||
const React = jest.requireActual<typeof import('react')>('react');
|
||||
return {
|
||||
__esModule: true,
|
||||
default: () => React.createElement('span', null, 'icon'),
|
||||
};
|
||||
});
|
||||
|
||||
const localize = (key: string) => key;
|
||||
|
||||
const anthropicEndpoint: Endpoint = {
|
||||
value: 'anthropic',
|
||||
label: 'Anthropic',
|
||||
hasModels: true,
|
||||
models: [{ name: 'claude-opus-4-6' }, { name: 'claude-sonnet-4-5' }],
|
||||
icon: null,
|
||||
};
|
||||
|
||||
const noModelsEndpoint: Endpoint = {
|
||||
value: 'custom',
|
||||
label: 'Custom',
|
||||
hasModels: false,
|
||||
icon: null,
|
||||
};
|
||||
|
||||
describe('SearchResults', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('marks model as selected when endpoint and model match with no active spec', () => {
|
||||
mockSelectedValues = { endpoint: 'anthropic', model: 'claude-opus-4-6', modelSpec: '' };
|
||||
render(
|
||||
<SearchResults results={[anthropicEndpoint]} localize={localize} searchValue="claude" />,
|
||||
);
|
||||
|
||||
const items = screen.getAllByRole('menuitem');
|
||||
const selectedItem = items.find((el) => el.getAttribute('aria-selected') === 'true');
|
||||
expect(selectedItem).toBeDefined();
|
||||
expect(selectedItem).toHaveTextContent('claude-opus-4-6');
|
||||
});
|
||||
|
||||
it('does not mark model as selected when a spec is active', () => {
|
||||
mockSelectedValues = {
|
||||
endpoint: 'anthropic',
|
||||
model: 'claude-opus-4-6',
|
||||
modelSpec: 'my-spec',
|
||||
};
|
||||
render(
|
||||
<SearchResults results={[anthropicEndpoint]} localize={localize} searchValue="claude" />,
|
||||
);
|
||||
|
||||
const items = screen.getAllByRole('menuitem');
|
||||
for (const item of items) {
|
||||
expect(item).not.toHaveAttribute('aria-selected');
|
||||
}
|
||||
});
|
||||
|
||||
it('does not mark endpoint as selected when a spec is active', () => {
|
||||
mockSelectedValues = {
|
||||
endpoint: 'custom',
|
||||
model: '',
|
||||
modelSpec: 'my-spec',
|
||||
};
|
||||
render(<SearchResults results={[noModelsEndpoint]} localize={localize} searchValue="custom" />);
|
||||
|
||||
const item = screen.getByRole('menuitem');
|
||||
expect(item).not.toHaveAttribute('aria-selected');
|
||||
});
|
||||
|
||||
it('marks endpoint as selected when no spec is active and endpoint matches', () => {
|
||||
mockSelectedValues = { endpoint: 'custom', model: '', modelSpec: '' };
|
||||
render(<SearchResults results={[noModelsEndpoint]} localize={localize} searchValue="custom" />);
|
||||
|
||||
const item = screen.getByRole('menuitem');
|
||||
expect(item).toHaveAttribute('aria-selected', 'true');
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue