mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: memoize tool toggles and drop dead steering code
The composer palette derives its whole catalog from the badge-row context, and `useToolToggle` returned a fresh object every render, so memoizing that context value changed nothing: six new identities per render kept it invalidating on every keystroke. Memoize the hook's own return instead. Also removes four steer helpers, the run-end map they read and the drain re-arm they called, none of which have had a caller since the pending steers moved into the thread; tells the user why a queued message would not come back to an occupied composer instead of leaving the button looking broken; and refuses a stop once the take is already being transcribed, where it could only rewrite how a committed take gets spent.
This commit is contained in:
parent
2c924bf11a
commit
ea177022df
9 changed files with 95 additions and 152 deletions
|
|
@ -428,7 +428,12 @@ function Bar({
|
|||
<RoundButton
|
||||
label={dictating ? localize('com_ui_stop') : localize('com_ui_use_micrphone')}
|
||||
onClick={dictating ? dictation.stopToComposer : dictation.start}
|
||||
disabled={speechDisabled && !dictating}
|
||||
/* Once the take is handed to the transcriber there is nothing left
|
||||
to stop, and pressing either of these again would only rewrite
|
||||
how a take that has already been committed gets spent. Cancel,
|
||||
on the `+`, stays live: an external transcription in flight can
|
||||
still be thrown away. */
|
||||
disabled={(speechDisabled && !dictating) || dictation.transcribing}
|
||||
>
|
||||
{dictating ? (
|
||||
<Square className="size-4 fill-current" aria-hidden="true" />
|
||||
|
|
@ -442,6 +447,7 @@ function Bar({
|
|||
primary
|
||||
label={localize('com_nav_send_message')}
|
||||
onClick={dictation.stopAndSend}
|
||||
disabled={dictation.transcribing}
|
||||
>
|
||||
<SendIcon size={18} />
|
||||
</RoundButton>
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
import { memo, useRef, useMemo, useState, useCallback } from 'react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import { useDrag, useDrop } from 'react-dnd';
|
||||
import { useMediaQuery } from '@librechat/client';
|
||||
import { X, Pencil, GripVertical } from 'lucide-react';
|
||||
import { useMediaQuery, useToastContext } from '@librechat/client';
|
||||
import type { TMessage } from 'librechat-data-provider';
|
||||
import type { SteeringControls, QueuedMessageContext } from '~/hooks/Chat/useSteering';
|
||||
import type { QueuedMessage } from '~/store/families';
|
||||
|
|
@ -71,6 +71,7 @@ function QueueRow({
|
|||
onAnnounce,
|
||||
}: QueueRowProps) {
|
||||
const localize = useLocalize();
|
||||
const { showToast } = useToastContext();
|
||||
const rowRef = useRef<HTMLDivElement>(null);
|
||||
const gripRef = useRef<HTMLButtonElement>(null);
|
||||
const { reorderQueued, restoreQueuedOrder } = steering;
|
||||
|
|
@ -241,7 +242,15 @@ function QueueRow({
|
|||
);
|
||||
if (restored) {
|
||||
steering.removeQueued(message.id);
|
||||
return;
|
||||
}
|
||||
/* Refusing silently reads as a dead button: the row stays, nothing
|
||||
moves, and the reason (a draft in the box, another chat on screen)
|
||||
is somewhere the click was not. */
|
||||
showToast({
|
||||
message: localize('com_ui_queue_remove_blocked'),
|
||||
status: 'warning',
|
||||
});
|
||||
}}
|
||||
className={ICON_BTN}
|
||||
>
|
||||
|
|
|
|||
|
|
@ -18,6 +18,12 @@ jest.mock('~/hooks', () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
const mockShowToast = jest.fn();
|
||||
jest.mock('@librechat/client', () => ({
|
||||
...jest.requireActual('@librechat/client'),
|
||||
useToastContext: () => ({ showToast: mockShowToast }),
|
||||
}));
|
||||
|
||||
const CONVO_ID = 'convo-1';
|
||||
const mockSendQueuedNow = jest.fn();
|
||||
const mockRemoveQueued = jest.fn();
|
||||
|
|
@ -184,6 +190,11 @@ describe('Queue', () => {
|
|||
fireEvent.click(screen.getByLabelText('com_ui_remove_queued'));
|
||||
expect(onRestore).toHaveBeenCalled();
|
||||
expect(mockRemoveQueued).not.toHaveBeenCalled();
|
||||
/* Keeping the words is right; saying nothing about it is not. Without this
|
||||
the row simply does not react and the button reads as broken. */
|
||||
expect(mockShowToast).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ message: 'com_ui_queue_remove_blocked' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('hands the whole message to the composer to edit', () => {
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import { useRecoilValue, useSetRecoilState, useRecoilCallback } from 'recoil';
|
|||
import { Constants, ContentTypes, isAssistantsEndpoint } from 'librechat-data-provider';
|
||||
import type { TMessage, TConversation, TMessageContentParts } from 'librechat-data-provider';
|
||||
import type { CallbackInterface } from 'recoil';
|
||||
import type { RunEnd, PendingSteer, QueuedMessage } from '~/store/families';
|
||||
import type { PendingSteer, QueuedMessage } from '~/store/families';
|
||||
import type { ExtendedFile, FileSetter } from '~/common';
|
||||
import {
|
||||
useGetMessagesByConvoId,
|
||||
|
|
@ -13,7 +13,6 @@ import {
|
|||
useMarkFilesUsageMutation,
|
||||
} from '~/data-provider';
|
||||
import { carriedSteerContext, clearAllDrafts } from '~/utils';
|
||||
import useSteerConvert from '~/hooks/Chat/useSteerConvert';
|
||||
import { useSetFilesToDelete } from '~/hooks/Files';
|
||||
import useLocalize from '~/hooks/useLocalize';
|
||||
import store from '~/store';
|
||||
|
|
@ -165,7 +164,6 @@ export default function useSteering({
|
|||
const localize = useLocalize();
|
||||
const { showToast } = useToastContext();
|
||||
const setFilesToDelete = useSetFilesToDelete();
|
||||
const convertSteersToQueued = useSteerConvert();
|
||||
/** `mutate` is a stable callback; the mutation result objects are fresh
|
||||
* every render and would defeat the memoized return value below. */
|
||||
const { mutate: steerMessage } = useSteerMessageMutation();
|
||||
|
|
@ -205,31 +203,6 @@ export default function useSteering({
|
|||
const isSubmittingRef = useRef(isSubmitting);
|
||||
isSubmittingRef.current = isSubmitting;
|
||||
|
||||
/**
|
||||
* How each conversation's last run ended, kept because `useQueueDrain`
|
||||
* CONSUMES the one-shot signal — by the time a reclaim resolves it is already
|
||||
* gone. Every subscriber renders before the drain's effect nulls it, so the
|
||||
* outcome is captured first. Both carriers are watched: the index signal, and
|
||||
* the copy parked under the conversation when the run ended while the user
|
||||
* was looking elsewhere.
|
||||
*
|
||||
* Keyed by conversation because this hook is REUSED across chats: a single
|
||||
* slot would answer for whichever chat is on screen when the reclaim lands,
|
||||
* not the one the words belong to. An entry is dropped once that conversation
|
||||
* starts another run — an older end no longer describes what is happening.
|
||||
*/
|
||||
const runEnd = useRecoilValue(store.runEndByIndex(index));
|
||||
const parkedRunEnd = useRecoilValue(store.pendingRunEndByConvoId(queueKey));
|
||||
const runEndsRef = useRef<Map<string, RunEnd>>(new Map());
|
||||
const observedRunEnd = [runEnd, parkedRunEnd].find(
|
||||
(end) => end != null && end.conversationId === conversationId,
|
||||
);
|
||||
if (observedRunEnd != null) {
|
||||
runEndsRef.current.set(conversationId, observedRunEnd);
|
||||
} else if (isSubmitting) {
|
||||
runEndsRef.current.delete(conversationId);
|
||||
}
|
||||
|
||||
const upsertSteerChip = useRecoilCallback(
|
||||
({ set }) =>
|
||||
(convoId: string, steer: PendingSteer) => {
|
||||
|
|
@ -485,30 +458,6 @@ export default function useSteering({
|
|||
[queueKey],
|
||||
);
|
||||
|
||||
/**
|
||||
* Re-posts a spent run-end signal so the drain wakes and reconsiders the
|
||||
* queue. No-op while a signal for THIS conversation is still armed: that
|
||||
* drain has not run yet and will see the item on its own, so arming a second
|
||||
* carrier would drain twice and send two messages.
|
||||
*
|
||||
* A signal for a DIFFERENT conversation is not that proof. The index slot is
|
||||
* shared, and the drain parks a foreign signal under its own conversation and
|
||||
* then only inspects the active one's queue — this item would never be looked
|
||||
* at. Park ours alongside it.
|
||||
*/
|
||||
const rearmDrain = useRecoilCallback(
|
||||
({ snapshot, set }) =>
|
||||
(convoId: string, end: RunEnd) => {
|
||||
const indexArmed = snapshot.getLoadable(store.runEndByIndex(index)).getValue();
|
||||
const parkedArmed = snapshot.getLoadable(store.pendingRunEndByConvoId(convoId)).getValue();
|
||||
if (indexArmed?.conversationId === convoId || parkedArmed != null) {
|
||||
return;
|
||||
}
|
||||
set(store.pendingRunEndByConvoId(convoId), end);
|
||||
},
|
||||
[index],
|
||||
);
|
||||
|
||||
const armDrainAfterAbort = useRecoilCallback(
|
||||
({ set }) =>
|
||||
() => {
|
||||
|
|
@ -656,90 +605,6 @@ export default function useSteering({
|
|||
[filesLoading, enqueue, takeComposerFiles, takeComposerContext, takeComposerDraft],
|
||||
);
|
||||
|
||||
/** Retry a failed chip through the normal steer path. */
|
||||
const retrySteer = useCallback(
|
||||
(
|
||||
steerId: string,
|
||||
text: string,
|
||||
steerFiles?: TMessage['files'],
|
||||
context?: QueuedMessageContext,
|
||||
) => {
|
||||
replaceSteerChip(conversationId, steerId, null);
|
||||
submitSteer(text, steerFiles, context);
|
||||
},
|
||||
[conversationId, replaceSteerChip, submitSteer],
|
||||
);
|
||||
|
||||
const removeSteer = useCallback(
|
||||
(steerId: string) => {
|
||||
replaceSteerChip(conversationId, steerId, null);
|
||||
},
|
||||
[conversationId, replaceSteerChip],
|
||||
);
|
||||
|
||||
/**
|
||||
* Re-homes a steer the client just reclaimed from the server queue (see
|
||||
* `useSteerReclaim`) as a queued follow-up.
|
||||
*
|
||||
* Routed through the shared conversion rather than `enqueue` so it obeys the
|
||||
* same invariant as the leftover-steer path: the item keeps its ORIGINAL id
|
||||
* and `createdAt`, so a steer accepted before a later follow-up still drains
|
||||
* ahead of it — a fresh `Date.now()` would sort it last.
|
||||
*
|
||||
* The reclaim is a round-trip, so the run can end while it is in flight and
|
||||
* the drain can consume its one-shot run-end signal against an empty queue,
|
||||
* leaving nothing to auto-send this item. Rather than send it here, re-post
|
||||
* that spent signal so `useQueueDrain` runs again and decides: it owns the
|
||||
* completed-only rule, FIFO order, `NEW_CONVO` migration, and submitting via
|
||||
* `ask` (which, unlike the composer's `sendNow`, does not reset the form and
|
||||
* so cannot wipe a draft typed while the reclaim was in flight).
|
||||
*
|
||||
* Parked under the conversation rather than the index, so a run that ended
|
||||
* while the user was elsewhere still drains when they come back.
|
||||
*/
|
||||
const queueReclaimedSteer = useCallback(
|
||||
(steer: PendingSteer) => {
|
||||
convertSteersToQueued(conversationId, [
|
||||
{
|
||||
steerId: steer.steerId,
|
||||
text: steer.text,
|
||||
createdAt: steer.createdAt,
|
||||
...(steer.files && steer.files.length > 0 && { files: steer.files }),
|
||||
/** Carried on the steer itself: the conversion normally recovers this
|
||||
* from the chip, which a competing cancel can remove mid-reclaim. */
|
||||
...carriedSteerContext(steer),
|
||||
},
|
||||
]);
|
||||
/**
|
||||
* Read by conversation, so it describes THIS steer's run even once the
|
||||
* hook has moved to another chat — the user navigating away does not make
|
||||
* their words any less owed a send. No entry means that run is still going
|
||||
* (or has started another): its own end is ahead of this item and drains
|
||||
* it, so there is nothing to re-arm.
|
||||
*/
|
||||
const lastRunEnd = runEndsRef.current.get(conversationId);
|
||||
if (lastRunEnd == null || lastRunEnd.outcome !== 'completed') {
|
||||
return;
|
||||
}
|
||||
rearmDrain(conversationId, lastRunEnd);
|
||||
},
|
||||
[conversationId, convertSteersToQueued, rearmDrain],
|
||||
);
|
||||
|
||||
/** Convert a failed/unsent steer chip into a queued follow-up. */
|
||||
const convertSteerToQueue = useCallback(
|
||||
(
|
||||
steerId: string,
|
||||
text: string,
|
||||
steerFiles?: TMessage['files'],
|
||||
context?: QueuedMessageContext,
|
||||
) => {
|
||||
replaceSteerChip(conversationId, steerId, null);
|
||||
enqueue(text, { files: steerFiles, ...context });
|
||||
},
|
||||
[conversationId, replaceSteerChip, enqueue],
|
||||
);
|
||||
|
||||
/** Chip action: send a queued message into the live run instead. Keys on
|
||||
* steer availability, not the default action — a queue-preferring user
|
||||
* clicking send-now explicitly asked to inject into the live run. The
|
||||
|
|
|
|||
|
|
@ -160,6 +160,29 @@ describe('useDictation', () => {
|
|||
expect(ask).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
/* The bar disables stop and send once a take is being transcribed, and the
|
||||
hook refuses them too: a second stop would rewrite how a take that was
|
||||
already committed gets spent. */
|
||||
it('ignores a stop that arrives after the take has ended', async () => {
|
||||
const { result, rerender, currentText } = setup();
|
||||
act(() => result.current.start());
|
||||
mockIsListening = true;
|
||||
act(() => rerender());
|
||||
|
||||
act(() => mockSetTextCallback('already committed'));
|
||||
act(() => result.current.stopToComposer());
|
||||
mockIsListening = false;
|
||||
act(() => rerender());
|
||||
|
||||
mockStop.mockClear();
|
||||
act(() => result.current.stopAndSend());
|
||||
await settle(rerender);
|
||||
|
||||
expect(mockStop).not.toHaveBeenCalled();
|
||||
expect(ask).not.toHaveBeenCalled();
|
||||
expect(currentText()).toBe('already committed');
|
||||
});
|
||||
|
||||
it('keeps the draft when a cancelled external transcription lands anyway', async () => {
|
||||
mockSpeechEndpoint = 'external';
|
||||
const { result, rerender, currentText } = setup({ draft: 'my unsent draft' });
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
import type { TranslationKeys } from '~/hooks/useLocalize';
|
||||
import type { LocalizeFunction } from '~/common';
|
||||
import { isMacPlatform } from '~/utils/shortcuts';
|
||||
import useLocalize from '~/hooks/useLocalize';
|
||||
|
|
|
|||
|
|
@ -201,8 +201,17 @@ export default function useDictation({
|
|||
startRecording();
|
||||
}, [getValues, startRecording]);
|
||||
|
||||
/* Only a running take can be stopped. The bar disables these controls once a
|
||||
transcription is in flight, and this is the same rule stated where the mode
|
||||
is actually written: a second stop would otherwise rewrite how a take that
|
||||
was already committed gets spent. */
|
||||
const activeRef = useRef(active);
|
||||
activeRef.current = active;
|
||||
const stopWith = useCallback(
|
||||
(mode: StopMode) => {
|
||||
if (!activeRef.current) {
|
||||
return;
|
||||
}
|
||||
modeRef.current = mode;
|
||||
setSettling(true);
|
||||
if (mode === 'send') {
|
||||
|
|
|
|||
|
|
@ -162,17 +162,37 @@ export function useToolToggle({
|
|||
[handleChange],
|
||||
);
|
||||
|
||||
return {
|
||||
toggleState: toolValue, // Return the actual value from ephemeralAgent
|
||||
handleChange,
|
||||
isToolEnabled,
|
||||
toolValue,
|
||||
setToggleState: (value: ToolValue) => handleChange({ value }), // Adapter for direct setting
|
||||
ephemeralAgent,
|
||||
debouncedChange,
|
||||
setEphemeralAgent,
|
||||
authData: authQuery?.data,
|
||||
isPinned,
|
||||
setIsPinned,
|
||||
};
|
||||
const setToggleState = useCallback((value: ToolValue) => handleChange({ value }), [handleChange]);
|
||||
|
||||
/* Memoized because `BadgeRowContext` puts six of these in a context value that
|
||||
the composer palette derives its whole catalog from: a fresh object here on
|
||||
every render made that value change on every keystroke, whatever the
|
||||
provider did about it. */
|
||||
return useMemo(
|
||||
() => ({
|
||||
toggleState: toolValue, // Return the actual value from ephemeralAgent
|
||||
handleChange,
|
||||
isToolEnabled,
|
||||
toolValue,
|
||||
setToggleState, // Adapter for direct setting
|
||||
ephemeralAgent,
|
||||
debouncedChange,
|
||||
setEphemeralAgent,
|
||||
authData: authQuery?.data,
|
||||
isPinned,
|
||||
setIsPinned,
|
||||
}),
|
||||
[
|
||||
toolValue,
|
||||
handleChange,
|
||||
isToolEnabled,
|
||||
setToggleState,
|
||||
ephemeralAgent,
|
||||
debouncedChange,
|
||||
setEphemeralAgent,
|
||||
authQuery?.data,
|
||||
isPinned,
|
||||
setIsPinned,
|
||||
],
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1607,6 +1607,7 @@
|
|||
"com_ui_question_unanswered": "No answer was given",
|
||||
"com_ui_queue": "Queue",
|
||||
"com_ui_queue_moved": "Moved to {{0}} of {{1}}",
|
||||
"com_ui_queue_remove_blocked": "Clear the message box in this chat to take this message back.",
|
||||
"com_ui_queue_reorder": "Reorder message, {{0}} of {{1}}",
|
||||
"com_ui_queue_reorder_hint": "Use the up and down arrow keys to move this message in the queue.",
|
||||
"com_ui_queue_send": "Queue message for after the response",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue