diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index 17b78dff12..6c3825f821 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -411,7 +411,7 @@ const ChatForm = memo(function ChatForm({ const isMoreThanThreeRows = visualRowCount > 3; - const composerItems = useComposerItems(conversationId); + const composerItems = useComposerItems(conversationId, quotesEnabled); const attachTarget = useAttachTarget(conversation, disableInputs); const dictation = useDictation({ ask: submitMessage, methods, isSubmitting }); const uploadingCount = useMemo(() => { diff --git a/client/src/components/Chat/Input/Composer/Hints.tsx b/client/src/components/Chat/Input/Composer/Hints.tsx index 846cdbfddf..b4a705cbac 100644 --- a/client/src/components/Chat/Input/Composer/Hints.tsx +++ b/client/src/components/Chat/Input/Composer/Hints.tsx @@ -22,8 +22,9 @@ export const COMPOSER_HINT_ID = 'composer-hint'; * here would re-announce on every keystroke as the hint flips between idle and * typing, so the description channel carries it instead. */ -function Hints(state: ComposerHintState) { - const hint = useComposerHint(state); +function Hints(state: Omit) { + const enterToSend = useRecoilValue(store.enterToSend); + const hint = useComposerHint({ ...state, enterToSend }); const showTips = useRecoilValue(store.showComposerTips); const visible = showTips || hint.kind === 'state'; diff --git a/client/src/components/Chat/Input/Composer/Palette.tsx b/client/src/components/Chat/Input/Composer/Palette.tsx index e90b6b05b8..a25f6e5362 100644 --- a/client/src/components/Chat/Input/Composer/Palette.tsx +++ b/client/src/components/Chat/Input/Composer/Palette.tsx @@ -896,6 +896,9 @@ function Palette({ disclosure's own click handler and the popover never opened. */} (null); diff --git a/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx b/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx index d4cb0f95e4..0a46cacdc9 100644 --- a/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx +++ b/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { act, render, renderHook } from '@testing-library/react'; -import { RecoilRoot, useRecoilValue, type MutableSnapshot } from 'recoil'; +import { RecoilRoot, useRecoilValue, useSetRecoilState, type MutableSnapshot } from 'recoil'; import useSteerRecovery from '../useSteerRecovery'; import store from '~/store'; @@ -16,9 +16,25 @@ const flush = () => act(async () => undefined); const CONVO_ID = 'convo-steer-recovery'; +/** The hook resolves "is the run over" from the conversation's own submitting + * state, not from its own unmount, so every case has to place the + * conversation in the store the way `ChatView` does. */ +const seedRun = (snapshot: MutableSnapshot, submitting: boolean) => { + snapshot.set(store.conversationKeysAtom, [0]); + snapshot.set(store.conversationByIndex(0), { conversationId: CONVO_ID } as never); + snapshot.set(store.isSubmittingFamily(0), submitting); +}; + function setup(initialize?: (snapshot: MutableSnapshot) => void) { const wrapper = ({ children }: { children: React.ReactNode }) => ( - {children} + { + seedRun(snapshot, true); + initialize?.(snapshot); + }} + > + {children} + ); return renderHook( () => ({ @@ -125,17 +141,15 @@ describe('useSteerRecovery', () => { expect(result.current.queue).toEqual([]); }); - /* The block this hook lives in unmounts the moment the run ends, which is - exactly when a retry's ack tends to land. It has to survive that: the - words go to the queue rather than leaving the chip saying `sending`. */ - /* The block this hook lives in unmounts the moment the run ends, which is - exactly when a retry's ack tends to land. It has to survive that: the - words go to the queue rather than leaving the chip saying `sending`. */ - it('queues a retry whose ack lands after the run ended', async () => { + /* A retry's ack tends to land right as the run ends, which is also when + the block this hook lives in unmounts. Whether the words go to the queue + turns on the run, not on the unmount. */ + const lateAck = async (endRun: boolean) => { let settle: (value: unknown) => void = () => undefined; mockMutateAsync.mockReturnValue(new Promise((resolve) => (settle = resolve))); let recovery: ReturnType | undefined; + let setSubmitting: ((value: boolean) => void) | undefined; let chips: unknown[] = []; let queue: unknown[] = []; const Recovery = () => { @@ -147,12 +161,14 @@ describe('useSteerRecovery', () => { const Observer = () => { chips = useRecoilValue(store.pendingSteersByConvoId(CONVO_ID)); queue = useRecoilValue(store.queuedMessagesByConvoId(CONVO_ID)); + setSubmitting = useSetRecoilState(store.isSubmittingFamily(0)); return null; }; const Tree = ({ live }: { live: boolean }) => ( { - set(store.pendingSteersByConvoId(CONVO_ID), [ + initializeState={(snapshot) => { + seedRun(snapshot, true); + snapshot.set(store.pendingSteersByConvoId(CONVO_ID), [ { steerId: 'local-late', text: 'landed too late', status: 'failed', createdAt: 3 }, ]); }} @@ -168,15 +184,32 @@ describe('useSteerRecovery', () => { }); expect(chips).toEqual([expect.objectContaining({ status: 'sending' })]); + if (endRun) { + act(() => setSubmitting?.(false)); + } rerender(); await act(async () => { settle({ steerId: 'srv-late', status: 'queued', position: 1, conversationId: CONVO_ID }); }); + return { chips, queue }; + }; + + it('queues a retry whose ack lands after the run ended', async () => { + const { chips, queue } = await lateAck(true); expect(chips).toEqual([]); expect(queue).toEqual([expect.objectContaining({ id: 'srv-late', text: 'landed too late' })]); }); + /* Navigating away unmounts the same block while the resumable run carries + on, and the server will still inject the accepted steer. Queueing it as + a follow-up here is what sent the same words twice. */ + it('leaves the ack pending when the block unmounts on a still-live run', async () => { + const { chips, queue } = await lateAck(false); + expect(chips).toEqual([expect.objectContaining({ steerId: 'srv-late', status: 'pending' })]); + expect(queue).toEqual([]); + }); + /* The picks the message was written with have to survive the retry, or the words are re-sent without the quotes and skills they referred to. */ it('carries the quotes and skills the steer was written with', async () => { diff --git a/client/src/hooks/Chat/useSteerRecovery.ts b/client/src/hooks/Chat/useSteerRecovery.ts index 2a24f03272..14b37b8a0d 100644 --- a/client/src/hooks/Chat/useSteerRecovery.ts +++ b/client/src/hooks/Chat/useSteerRecovery.ts @@ -1,5 +1,6 @@ -import { useRef, useEffect, useCallback } from 'react'; +import { useCallback } from 'react'; import { useRecoilCallback } from 'recoil'; +import type { Snapshot } from 'recoil'; import type { PendingSteer } from '~/store/families'; import { getSteerErrorCode, resolveAcknowledgedSteer } from '~/hooks/Chat/useSteering'; import useSteerConvert from '~/hooks/Chat/useSteerConvert'; @@ -16,16 +17,24 @@ export default function useSteerRecovery(conversationId: string) { const { mutateAsync: steerMessage } = useSteerMessageMutation(); const convertSteersToQueued = useSteerConvert(); - /** `PendingSteers` only renders while `isLast && isSubmitting` is true, so - * its unmount IS the run ending. A retry's ack can resolve afterward; - * mirrors `composerMountedRef` in `ChatForm`, which guards the equivalent - * race for a reclaimed steer whose restore resolves post-unmount. */ - const mountedRef = useRef(true); - useEffect( - () => () => { - mountedRef.current = false; + /** Whether the run this steer belongs to has finished, read live at ack time. + * Unmounting is not the same signal: `PendingSteers` also unmounts when the + * user navigates away from a run that is still going, and treating that as + * the end queued the accepted steer as a follow-up on top of the injection + * the server was already making. */ + const isRunOver = useCallback( + (snapshot: Snapshot) => { + const keys = snapshot.getLoadable(store.conversationKeysAtom).getValue(); + for (const key of keys) { + const convo = snapshot.getLoadable(store.conversationByIndex(key)).getValue(); + if (convo?.conversationId !== conversationId) { + continue; + } + return snapshot.getLoadable(store.isSubmittingFamily(key)).getValue() !== true; + } + return true; }, - [], + [conversationId], ); const markStatus = useRecoilCallback( @@ -39,10 +48,16 @@ export default function useSteerRecovery(conversationId: string) { ); const acknowledgeRetry = useRecoilCallback( - (cbInterface) => (localId: string, steer: PendingSteer, runOver: boolean) => { - resolveAcknowledgedSteer(cbInterface, conversationId, localId, steer, runOver); + (cbInterface) => (localId: string, steer: PendingSteer) => { + resolveAcknowledgedSteer( + cbInterface, + conversationId, + localId, + steer, + isRunOver(cbInterface.snapshot), + ); }, - [conversationId], + [conversationId, isRunOver], ); /** Routes a steer straight into the queue: reused for a retry that degrades @@ -84,11 +99,7 @@ export default function useSteerRecovery(conversationId: string) { rest of the conversation with the words neither sent nor queued. */ steerMessage({ conversationId, text: steer.text, files: steer.files }) .then((response) => { - acknowledgeRetry( - steerId, - { ...steer, steerId: response.steerId, status: 'pending' }, - !mountedRef.current, - ); + acknowledgeRetry(steerId, { ...steer, steerId: response.steerId, status: 'pending' }); }) .catch((error: unknown) => { const code = getSteerErrorCode(error); diff --git a/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx b/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx index 4c38c3684f..16716abf34 100644 --- a/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx +++ b/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx @@ -186,6 +186,22 @@ describe('useAttachItems', () => { ).toContain('local:image'); }); + /* Assistants carry their own file configuration and the old menu handed + them an unfiltered picker; scoping them to the image capability, which + the provider check would, silently dropped PDF support. */ + it.each([EModelEndpoint.assistants, EModelEndpoint.azureAssistants])( + 'offers one unfiltered upload for %s and no tool destinations', + (endpoint) => { + const ids = renderEntries({ + endpoint, + contextEnabled: true, + fileSearchEnabled: true, + codeEnabled: true, + }); + expect(ids).toEqual(['local:assistants']); + }, + ); + it('reads a provider whatever its casing, which OpenRouter arrives in', () => { expect(renderEntries({ provider: 'OpenRouter' })).toContain('local:provider'); }); diff --git a/client/src/hooks/Input/__tests__/useComposerHint.spec.ts b/client/src/hooks/Input/__tests__/useComposerHint.spec.ts index d723e7b2d1..4cc7f028ac 100644 --- a/client/src/hooks/Input/__tests__/useComposerHint.spec.ts +++ b/client/src/hooks/Input/__tests__/useComposerHint.spec.ts @@ -12,6 +12,7 @@ const baseState: ComposerHintState = { duringRunAction: 'queue' as const, answerModeActive: false, uploadingCount: 0, + enterToSend: true, }; const hint = (overrides: Partial, isMac = true) => @@ -68,6 +69,29 @@ describe('composeHint', () => { }); }); + describe('with Enter bound to a newline', () => { + it('names the chord as the send key while typing', () => { + const result = hint({ hasText: true, enterToSend: false }); + expect(result).toContain('⌘⏎'); + expect(result).toContain('com_ui_composer_hint_send'); + expect(result).toContain('com_ui_composer_hint_newline'); + expect(result).not.toContain('com_ui_composer_hint_typing'); + }); + + it('drops the alternate during-run action, which the chord no longer reaches', () => { + const result = hint({ + duringRunActive: true, + hasText: true, + isSubmitting: true, + duringRunAction: 'steer', + enterToSend: false, + }); + expect(result).toContain('⌘⏎ com_ui_composer_hint_steer_verb'); + expect(result).toContain('com_ui_composer_hint_interrupt'); + expect(result).not.toContain('com_ui_composer_hint_queue'); + }); + }); + describe('kind', () => { it('marks the ambient copy as a tip, so a conversation can drop it', () => { expect(kindOf({})).toBe('tip'); diff --git a/client/src/hooks/Input/__tests__/useDictation.spec.tsx b/client/src/hooks/Input/__tests__/useDictation.spec.tsx new file mode 100644 index 0000000000..b191b1446f --- /dev/null +++ b/client/src/hooks/Input/__tests__/useDictation.spec.tsx @@ -0,0 +1,180 @@ +import React from 'react'; +import { RecoilRoot } from 'recoil'; +import { act, renderHook } from '@testing-library/react'; +import type { TAskFunction } from '~/common'; +import useDictation from '../useDictation'; +import store from '~/store'; + +/** + * The engines only report a completed transcription when Auto Send Text is + * configured, so the composer's own stop-and-send cannot be built on that + * callback alone. These cases pin the three ways a take can be spent — sent on + * request, sent by the setting, or thrown away — against both engines. + */ + +let mockSpeechEndpoint: 'browser' | 'external'; +let mockSetTextCallback: (text: string) => void; +let mockOnTranscriptionComplete: (text: string) => void; +const mockStart = jest.fn(); +const mockStop = jest.fn(); +const mockAbort = jest.fn(); +let mockIsListening: boolean; +let mockIsLoading: boolean; + +jest.mock('../useSpeechToText', () => ({ + __esModule: true, + default: (setText: (text: string) => void, complete: (text: string) => void) => { + mockSetTextCallback = setText; + mockOnTranscriptionComplete = complete; + return { + isListening: mockIsListening, + isLoading: mockIsLoading, + startRecording: mockStart, + stopRecording: mockStop, + abortRecording: mockAbort, + }; + }, +})); + +jest.mock('../useGetAudioSettings', () => ({ + __esModule: true, + default: () => ({ speechToTextEndpoint: mockSpeechEndpoint }), +})); + +jest.mock('@librechat/client', () => ({ + useToastContext: () => ({ showToast: jest.fn() }), +})); + +jest.mock('../../useLocalize', () => ({ + __esModule: true, + default: () => (key: string) => key, +})); + +const ask = jest.fn(() => true) as unknown as jest.Mock & TAskFunction; + +function setup({ autoSendText = -1, draft = '' }: { autoSendText?: number; draft?: string } = {}) { + let text = draft; + const methods = { + setValue: jest.fn((_name: string, value: string) => { + text = value; + }), + reset: jest.fn((values: { text: string }) => { + text = values.text; + }), + getValues: jest.fn(() => text), + }; + const wrapper = ({ children }: { children: React.ReactNode }) => ( + set(store.autoSendText, autoSendText)}> + {children} + + ); + const view = renderHook( + () => + useDictation({ + ask: ask as unknown as TAskFunction, + methods: methods as never, + isSubmitting: false, + }), + { wrapper }, + ); + return { ...view, methods, currentText: () => text }; +} + +/** Ends the take the way an engine does: listening clears, then the settle + * backstop expires. */ +const settle = async (rerender: () => void) => { + mockIsListening = false; + act(() => { + rerender(); + }); + await act(async () => { + jest.advanceTimersByTime(600); + }); +}; + +describe('useDictation', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + mockSpeechEndpoint = 'browser'; + mockIsListening = false; + mockIsLoading = false; + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('sends on stop-and-send even with Auto Send Text off, which reports no transcription', async () => { + const { result, rerender, currentText } = setup(); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => mockSetTextCallback('take me somewhere')); + act(() => result.current.stopAndSend()); + await settle(rerender); + + expect(ask).toHaveBeenCalledWith({ text: 'take me somewhere' }); + expect(currentText()).toBe(''); + }); + + it('leaves a plain stop in the composer', async () => { + const { result, rerender, currentText } = setup(); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => mockSetTextCallback('just a draft')); + act(() => result.current.stopToComposer()); + await settle(rerender); + + expect(ask).not.toHaveBeenCalled(); + expect(currentText()).toBe('just a draft'); + }); + + it('still honours Auto Send Text on a plain stop', async () => { + const { result, rerender } = setup({ autoSendText: 0 }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => result.current.stopToComposer()); + await settle(rerender); + act(() => mockOnTranscriptionComplete('send this for me')); + + expect(ask).toHaveBeenCalledWith({ text: 'send this for me' }); + }); + + it('spends a take once when the setting and the send button both reach it', async () => { + const { result, rerender } = setup({ autoSendText: 0 }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => mockSetTextCallback('only once')); + act(() => result.current.stopAndSend()); + await settle(rerender); + act(() => mockOnTranscriptionComplete('only once')); + + expect(ask).toHaveBeenCalledTimes(1); + }); + + it('keeps the draft when a cancelled external transcription lands anyway', async () => { + mockSpeechEndpoint = 'external'; + const { result, rerender, currentText } = setup({ draft: 'my unsent draft' }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => result.current.cancel()); + expect(currentText()).toBe('my unsent draft'); + + /* The mutation was already in flight, so aborting the recorder cannot stop + its success callback from arriving. */ + act(() => mockOnTranscriptionComplete('words nobody asked for')); + + expect(ask).not.toHaveBeenCalled(); + expect(currentText()).toBe('my unsent draft'); + }); +}); diff --git a/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx b/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx index 493365696b..33fa809662 100644 --- a/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx +++ b/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx @@ -18,11 +18,13 @@ let mockContext: Record | null; let mockSkills: Array>; let mockAgentsMap: Record>; let mockSkillsActive: boolean; +let mockUser: { personalization?: { memories?: boolean } } | undefined; jest.mock('~/hooks', () => ({ useHasAccess: ({ permissionType }: { permissionType: string }) => mockPermissions[permissionType] ?? false, useHasMemoryAccess: () => mockMemoryAccess, + useAuthContext: () => ({ user: mockUser }), useAgentCapabilities: () => mockCapabilities, /* The real popover filter is used here, and it drops anything the user cannot invoke or that is not active for them. */ @@ -125,6 +127,7 @@ describe('usePaletteEntries', () => { beforeEach(() => { mockPermissions = { ...allPermissions }; mockMemoryAccess = true; + mockUser = { personalization: { memories: true } }; mockCapabilities = { ...allCapabilities }; mockContext = fullContext(); mockSkills = []; @@ -178,6 +181,11 @@ describe('usePaletteEntries', () => { mockMemoryAccess = false; expect(keysOf(entries().result)).not.toContain('builtin:memory'); }); + + it('withholds memory from a user who opted out in personalization', () => { + mockUser = { personalization: { memories: false } }; + expect(keysOf(entries().result)).not.toContain('builtin:memory'); + }); }); describe('on state', () => { diff --git a/client/src/hooks/Input/useAttachItems.tsx b/client/src/hooks/Input/useAttachItems.tsx index f29a527547..0770a046ba 100644 --- a/client/src/hooks/Input/useAttachItems.tsx +++ b/client/src/hooks/Input/useAttachItems.tsx @@ -14,6 +14,7 @@ import { EModelEndpoint, getConfiguredMimeAccept, bedrockDocumentMimeTypes, + isAssistantsEndpoint, defaultAgentCapabilities, bedrockDocumentExtensions, isDocumentSupportedProvider, @@ -201,6 +202,25 @@ export default function useAttachItems({ const build = (onAction: (fileType?: FileUploadType) => void, prefix: string) => { const items: AttachEntry[] = []; + + /* Assistants own their own file handling: whatever the assistant is + configured to accept goes up unfiltered, and none of the tool + destinations below apply. Scoping this to the image capability, as the + provider check would, silently dropped PDF support. */ + if (isAssistantsEndpoint(endpoint)) { + items.push({ + id: `${prefix}:assistants`, + label: localize('com_sidepanel_attach_files'), + primary: true, + icon: