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:
Marco Beretta 2026-07-29 03:47:12 +02:00
parent e4a1a40615
commit 4d4a2b85ae
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
9 changed files with 95 additions and 152 deletions

View file

@ -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>

View file

@ -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}
>

View file

@ -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', () => {

View file

@ -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) => {
@ -486,30 +459,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 }) =>
() => {
@ -657,90 +606,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

View file

@ -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' });

View file

@ -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';

View file

@ -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') {

View file

@ -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,
],
);
}

View file

@ -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",