mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🧠 fix: Restore Agent Memory Scope Control in Unified Builder (#14292)
* 🧠 fix: Restore Agent Memory Scope Control in Unified Builder The Agent Builder redesign (#13952) surfaced memory as a Tools marketplace item and stopped rendering SidePanel/Agents/Memory.tsx, orphaning the file and removing the 'Keep memories separate for this agent' control shipped in #14084. Only the render was lost: the locale keys, memory_scope on AgentForm, the AgentPanel save path, and AgentSelect hydration all survived, which is why setting memory_scope directly on the agent document still worked. Restore it as a builtin item setting (the seam the new builder uses for per-tool config): mark memory configurable so its row gets a cog, and render a MemoryConfig branch in BuiltinSection mirroring ArtifactsConfig. Delete the orphaned component so there is a single source of truth. Resolves #14287 * 🧹 chore: Remove Orphaned com_agents_enable_memory i18n Key The key labeled the enable checkbox in the deleted SidePanel/Agents/Memory.tsx. The unified builder labels the catalog item via com_ui_memory, so it has no remaining consumer and detect-unused-i18n-keys fails on it.
This commit is contained in:
parent
87b3557f11
commit
21766b5b3c
6 changed files with 131 additions and 124 deletions
|
|
@ -1,118 +0,0 @@
|
|||
import { memo } from 'react';
|
||||
import { useFormContext, Controller, useWatch } from 'react-hook-form';
|
||||
import { MemoryScope, AgentCapabilities } from 'librechat-data-provider';
|
||||
import {
|
||||
Checkbox,
|
||||
HoverCard,
|
||||
HoverCardContent,
|
||||
HoverCardPortal,
|
||||
HoverCardTrigger,
|
||||
CircleHelpIcon,
|
||||
} from '@librechat/client';
|
||||
import type { AgentForm } from '~/common';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { ESide } from '~/common';
|
||||
|
||||
function Memory() {
|
||||
const localize = useLocalize();
|
||||
const methods = useFormContext<AgentForm>();
|
||||
const { control } = methods;
|
||||
const memoryEnabled = useWatch({ control, name: AgentCapabilities.memory });
|
||||
|
||||
return (
|
||||
<div>
|
||||
<HoverCard openDelay={50}>
|
||||
<div className="my-2 flex items-center">
|
||||
<Controller
|
||||
name={AgentCapabilities.memory}
|
||||
control={control}
|
||||
render={({ field }) => (
|
||||
<Checkbox
|
||||
{...field}
|
||||
id="memory-checkbox"
|
||||
checked={field.value === true}
|
||||
onCheckedChange={field.onChange}
|
||||
className="relative float-left mr-2 inline-flex h-4 w-4 cursor-pointer"
|
||||
value={(field.value === true).toString()}
|
||||
aria-labelledby="memory-label"
|
||||
/>
|
||||
)}
|
||||
/>
|
||||
<label
|
||||
id="memory-label"
|
||||
htmlFor="memory-checkbox"
|
||||
className="form-check-label text-token-text-primary cursor-pointer text-sm"
|
||||
>
|
||||
{localize('com_agents_enable_memory')}
|
||||
</label>
|
||||
<HoverCardTrigger asChild className="ml-2">
|
||||
<button
|
||||
type="button"
|
||||
className="inline-flex items-center"
|
||||
aria-label={localize('com_agents_memory_info')}
|
||||
>
|
||||
<CircleHelpIcon className="h-4 w-4 text-text-tertiary" />
|
||||
</button>
|
||||
</HoverCardTrigger>
|
||||
<HoverCardPortal>
|
||||
<HoverCardContent side={ESide.Top} className="w-80">
|
||||
<div className="space-y-2">
|
||||
<p className="text-sm text-text-secondary">{localize('com_agents_memory_info')}</p>
|
||||
</div>
|
||||
</HoverCardContent>
|
||||
</HoverCardPortal>
|
||||
</div>
|
||||
</HoverCard>
|
||||
{memoryEnabled === true && (
|
||||
<HoverCard openDelay={50}>
|
||||
<div className="my-2 ml-6 flex items-center">
|
||||
<Controller
|
||||
name="memory_scope"
|
||||
control={control}
|
||||
render={({ field }) => (
|
||||
<Checkbox
|
||||
{...field}
|
||||
id="memory-scope-checkbox"
|
||||
checked={field.value === MemoryScope.agent}
|
||||
onCheckedChange={(checked) =>
|
||||
field.onChange(checked === true ? MemoryScope.agent : MemoryScope.user)
|
||||
}
|
||||
className="relative float-left mr-2 inline-flex h-4 w-4 cursor-pointer"
|
||||
value={(field.value === MemoryScope.agent).toString()}
|
||||
aria-labelledby="memory-scope-label"
|
||||
/>
|
||||
)}
|
||||
/>
|
||||
<label
|
||||
id="memory-scope-label"
|
||||
htmlFor="memory-scope-checkbox"
|
||||
className="form-check-label text-token-text-primary cursor-pointer text-sm"
|
||||
>
|
||||
{localize('com_agents_memory_scope')}
|
||||
</label>
|
||||
<HoverCardTrigger asChild className="ml-2">
|
||||
<button
|
||||
type="button"
|
||||
className="inline-flex items-center"
|
||||
aria-label={localize('com_agents_memory_scope_info')}
|
||||
>
|
||||
<CircleHelpIcon className="h-4 w-4 text-text-tertiary" />
|
||||
</button>
|
||||
</HoverCardTrigger>
|
||||
<HoverCardPortal>
|
||||
<HoverCardContent side={ESide.Top} className="w-80">
|
||||
<div className="space-y-2">
|
||||
<p className="text-sm text-text-secondary">
|
||||
{localize('com_agents_memory_scope_info')}
|
||||
</p>
|
||||
</div>
|
||||
</HoverCardContent>
|
||||
</HoverCardPortal>
|
||||
</div>
|
||||
</HoverCard>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export default memo(Memory);
|
||||
|
|
@ -0,0 +1,80 @@
|
|||
import '@testing-library/jest-dom/extend-expect';
|
||||
import { MemoryScope } from 'librechat-data-provider';
|
||||
import { useForm, FormProvider, useWatch } from 'react-hook-form';
|
||||
import { render, screen, fireEvent } from '@testing-library/react';
|
||||
import type { ReactNode } from 'react';
|
||||
import type { AgentForm } from '~/common';
|
||||
import BuiltinSection from '../sections/BuiltinSection';
|
||||
|
||||
jest.mock('~/hooks', () => ({ useLocalize: () => (key: string) => key }));
|
||||
jest.mock('~/data-provider', () => ({ useVerifyAgentToolAuth: () => ({ data: undefined }) }));
|
||||
jest.mock('../../../Search/Action', () => ({ __esModule: true, default: () => <div /> }));
|
||||
jest.mock('../../../FileContext', () => ({ __esModule: true, default: () => <div /> }));
|
||||
jest.mock('../../../FileSearch', () => ({ __esModule: true, default: () => <div /> }));
|
||||
jest.mock('../../../Code/Files', () => ({ __esModule: true, default: () => <div /> }));
|
||||
|
||||
function ScopeProbe() {
|
||||
const value = useWatch<AgentForm>({ name: 'memory_scope' });
|
||||
return <span data-testid="scope">{String(value)}</span>;
|
||||
}
|
||||
|
||||
function renderSection(builtinId: string, defaultValues: Partial<AgentForm> = {}) {
|
||||
function Wrapper({ children }: { children: ReactNode }) {
|
||||
const methods = useForm<AgentForm>({ defaultValues: defaultValues as AgentForm });
|
||||
return (
|
||||
<FormProvider {...methods}>
|
||||
{children}
|
||||
<ScopeProbe />
|
||||
</FormProvider>
|
||||
);
|
||||
}
|
||||
|
||||
return render(
|
||||
<BuiltinSection
|
||||
builtinId={builtinId as never}
|
||||
agentId="a1"
|
||||
contextFiles={[]}
|
||||
knowledgeFiles={[]}
|
||||
codeFiles={[]}
|
||||
description="com_agents_memory_info"
|
||||
/>,
|
||||
{ wrapper: Wrapper },
|
||||
);
|
||||
}
|
||||
|
||||
const scopeCheckbox = () => screen.getByRole('checkbox', { name: 'com_agents_memory_scope' });
|
||||
|
||||
describe('BuiltinSection memory scope', () => {
|
||||
test('renders the scope control unchecked when memory is on the shared pool', () => {
|
||||
renderSection('memory', { memory: true, memory_scope: MemoryScope.user });
|
||||
expect(scopeCheckbox()).not.toBeChecked();
|
||||
expect(screen.getByText('com_agents_memory_scope_info')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('defaults to the shared pool when the agent has no saved scope', () => {
|
||||
renderSection('memory', { memory: true });
|
||||
expect(scopeCheckbox()).not.toBeChecked();
|
||||
});
|
||||
|
||||
test('renders checked for an agent already partitioned to its own memories', () => {
|
||||
renderSection('memory', { memory: true, memory_scope: MemoryScope.agent });
|
||||
expect(scopeCheckbox()).toBeChecked();
|
||||
});
|
||||
|
||||
test('toggling writes the scope back to the form in both directions', () => {
|
||||
renderSection('memory', { memory: true, memory_scope: MemoryScope.user });
|
||||
|
||||
fireEvent.click(scopeCheckbox());
|
||||
expect(screen.getByTestId('scope')).toHaveTextContent(MemoryScope.agent);
|
||||
expect(scopeCheckbox()).toBeChecked();
|
||||
|
||||
fireEvent.click(scopeCheckbox());
|
||||
expect(screen.getByTestId('scope')).toHaveTextContent(MemoryScope.user);
|
||||
expect(scopeCheckbox()).not.toBeChecked();
|
||||
});
|
||||
|
||||
test('other builtins do not render the scope control', () => {
|
||||
renderSection('execute_code', { memory: true, memory_scope: MemoryScope.agent });
|
||||
expect(screen.queryByRole('checkbox', { name: 'com_agents_memory_scope' })).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { Radio } from '@librechat/client';
|
||||
import { Radio, Checkbox } from '@librechat/client';
|
||||
import { useFormContext, useWatch } from 'react-hook-form';
|
||||
import { Tools, ArtifactModes, AgentCapabilities } from 'librechat-data-provider';
|
||||
import { Tools, MemoryScope, ArtifactModes, AgentCapabilities } from 'librechat-data-provider';
|
||||
import type { TranslationKeys } from '~/hooks/useLocalize';
|
||||
import type { AgentForm, ExtendedFile } from '~/common';
|
||||
import type { BuiltinId } from '../../items/types';
|
||||
|
|
@ -74,6 +74,43 @@ function ArtifactsConfig({ value, onChange }: ArtifactsConfigProps) {
|
|||
);
|
||||
}
|
||||
|
||||
interface MemoryConfigProps {
|
||||
value: string;
|
||||
onChange: (next: MemoryScope) => void;
|
||||
}
|
||||
|
||||
function MemoryConfig({ value, onChange }: MemoryConfigProps) {
|
||||
const localize = useLocalize();
|
||||
const isolated = value === MemoryScope.agent;
|
||||
|
||||
return (
|
||||
<div className="flex flex-col gap-3">
|
||||
<div className="flex items-center">
|
||||
<Checkbox
|
||||
id="memory-scope-checkbox"
|
||||
checked={isolated}
|
||||
onCheckedChange={(checked) =>
|
||||
onChange(checked === true ? MemoryScope.agent : MemoryScope.user)
|
||||
}
|
||||
className="relative float-left mr-2 inline-flex h-4 w-4 cursor-pointer"
|
||||
value={isolated.toString()}
|
||||
aria-labelledby="memory-scope-label"
|
||||
/>
|
||||
<label
|
||||
id="memory-scope-label"
|
||||
htmlFor="memory-scope-checkbox"
|
||||
className="cursor-pointer text-sm font-medium text-text-primary"
|
||||
>
|
||||
{localize('com_agents_memory_scope')}
|
||||
</label>
|
||||
</div>
|
||||
<p className="text-sm leading-relaxed text-text-secondary">
|
||||
{localize('com_agents_memory_scope_info')}
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function WebSearchConfig() {
|
||||
const { data } = useVerifyAgentToolAuth({ toolId: Tools.web_search }, { retry: 1 });
|
||||
return <SearchAction authTypes={data?.authTypes} isToolAuthenticated={data?.authenticated} />;
|
||||
|
|
@ -91,6 +128,7 @@ export default function BuiltinSection({
|
|||
const { control, setValue } = useFormContext<AgentForm>();
|
||||
|
||||
const artifactsValue = (useWatch({ control, name: AgentCapabilities.artifacts }) ?? '') as string;
|
||||
const memoryScope = (useWatch({ control, name: 'memory_scope' }) ?? MemoryScope.user) as string;
|
||||
|
||||
let body: React.ReactNode = null;
|
||||
|
||||
|
|
@ -109,6 +147,13 @@ export default function BuiltinSection({
|
|||
);
|
||||
} else if (builtinId === 'context') {
|
||||
body = <FileContext agent_id={agentId} files={contextFiles} showHeader={false} />;
|
||||
} else if (builtinId === 'memory') {
|
||||
body = (
|
||||
<MemoryConfig
|
||||
value={memoryScope}
|
||||
onChange={(next) => setValue('memory_scope', next, { shouldDirty: true })}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const localizedDescription = description ? localize(description as TranslationKeys) : '';
|
||||
|
|
|
|||
|
|
@ -6,15 +6,15 @@ const builtin = (id: string, extra: Record<string, unknown> = {}): AgentItem =>
|
|||
({ kind: 'builtin', id, name: '', description: '', iconKey: id, ...extra }) as AgentItem;
|
||||
|
||||
describe('hasConfigurableSettings', () => {
|
||||
test('artifacts, file_search, and context builtins are configurable', () => {
|
||||
test('artifacts, file_search, context, and memory builtins are configurable', () => {
|
||||
expect(hasConfigurableSettings(builtin('artifacts'))).toBe(true);
|
||||
expect(hasConfigurableSettings(builtin('file_search'))).toBe(true);
|
||||
expect(hasConfigurableSettings(builtin('context'))).toBe(true);
|
||||
expect(hasConfigurableSettings(builtin('memory'))).toBe(true);
|
||||
});
|
||||
|
||||
test('execute_code and memory builtins are not configurable', () => {
|
||||
test('execute_code builtin is not configurable', () => {
|
||||
expect(hasConfigurableSettings(builtin('execute_code'))).toBe(false);
|
||||
expect(hasConfigurableSettings(builtin('memory'))).toBe(false);
|
||||
});
|
||||
|
||||
test('web_search is configurable only when auth is user-provided', () => {
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ export function hasConfigurableSettings(item: AgentItem): boolean {
|
|||
item.id === 'artifacts' ||
|
||||
item.id === 'file_search' ||
|
||||
item.id === 'context' ||
|
||||
item.id === 'memory' ||
|
||||
(item.id === 'web_search' && item.userProvidedAuth === true)
|
||||
);
|
||||
case 'tool':
|
||||
|
|
|
|||
|
|
@ -41,7 +41,6 @@
|
|||
"com_agents_description_card": "Description: {{description}}",
|
||||
"com_agents_description_placeholder": "What this agent does",
|
||||
"com_agents_empty_state_heading": "No agents found",
|
||||
"com_agents_enable_memory": "Enable Memory",
|
||||
"com_agents_error_bad_request_message": "The request could not be processed.",
|
||||
"com_agents_error_bad_request_suggestion": "Please check your input and try again.",
|
||||
"com_agents_error_category_title": "Category Error",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue