CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-04 14:57:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

🧱 refactor: Make Queue Sendability A Property Of The Data

Browse source 
Removes the empty-edit standdown as a mechanism, rather than teaching it to a
seventh consumer.

The root cause was that `updateQueuedText` REFUSED blank text. That made the
queue deliberately disagree with the screen — the row still held the words the
user had just deleted — and every reader had to be told about the
disagreement through a shared atom: the row's own send and escalate controls,
the offscreen shortcut proxy, the drain, Merge, Clear all, and the trash. Six
sites, each found separately, each a chance to miss the seventh.

Blank is now recorded. A row holds exactly what is typed, so "there is nothing
to send" is visible in the row itself via one predicate,
`isSendableQueuedMessage`, which each reader evaluates on data it already has.
`queueEmptyEditFamily` and its claim/release lifecycle are deleted.

What keeps that safe is a single invariant: a blank row cannot outlive its
editor. Leaving the editor settles the row — trimmed if there are words, back
to the pre-edit text if there are not — by any exit: blur, Enter, Escape, a
pending send, or the remount that a collapsing group causes. So a resting
queue never holds a blank row, and `mergeQueuedMessages` refuses one anyway.

The drain refuses a blank front row by the same predicate, consuming its epoch
so the effect cannot spin, and picks the queue up on the next run end rather
than sending the row behind it out of order.
This commit is contained in:
Danny Avila 2026-08-04 08:27:21 -04:00
parent 75f89a8b68
commit c0e167b3d4
8 changed files with 162 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

90
client/src/components/Chat/Input/QueuedOutbox.tsx
View file

@ -1,6 +1,6 @@
import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useAtom } from 'jotai';
import { useToastContext } from '@librechat/client';
import { useAtom, useAtomValue, useSetAtom } from 'jotai';
import {
Zap,
Send,
@ -25,8 +25,8 @@ import {
useDefaultToggleEntry,
useInterruptToggleEntry,
} from './SteerMenu';
import { queueEmptyEditFamily, queueExpandedFamily } from '~/store/steer';
import { isMergeableQueuedMessage } from '~/utils';
import { isMergeableQueuedMessage, isSendableQueuedMessage } from '~/utils';
import { queueExpandedFamily } from '~/store/steer';
import { useLocalize } from '~/hooks';
import { cn } from '~/utils';
@ -132,27 +132,9 @@ function QueuedRowBase({
* during the pause is exactly the discoverability gap this button fixes. */
const showEscalate =
!isRecovered && (steering.pausedOnApproval || (steering.duringRunActive && steering.canSteer));
/** An emptied editor is the one state where the queue and the screen disagree
* by construction: the write-through refuses blank text, so the row still
* holds the previous words. Sending from there would send what the user just
* deleted, so the senders stand down until the edit resolves either way. */
const emptyEdit = draft != null && draft.trim().length === 0;
/** Published so the group's shortcut proxy can stand down too: it targets
* this row but lives outside it, and the shortcut is allowed while a
* textarea has focus. */
const setEmptyEditId = useSetAtom(queueEmptyEditFamily(steering.queueKey));
const claimEmptyEdit = useCallback(
(isEmpty: boolean) => {
setEmptyEditId((prev) => {
if (isEmpty) {
return message.id;
}
return prev === message.id ? null : prev;
});
},
[message.id, setEmptyEditId],
);
useEffect(() => () => claimEmptyEdit(false), [claimEmptyEdit]);
/** The queue holds exactly what is typed, so "nothing to send" is visible in
* the row itself — no shared claim to publish, release, or leak. */
const sendable = isSendableQueuedMessage(message);
/** Focus follows the explicit Edit action rather than mount, so the row can
* never steal focus from the composer on a re-render. */
@ -162,6 +144,9 @@ function QueuedRowBase({
}
}, [editing]);
const draftRef = useRef<string | null>(null);
draftRef.current = draft;
const beginEdit = useCallback(() => {
originalRef.current = message.text;
setDraft(message.text);
@ -180,28 +165,37 @@ function QueuedRowBase({
const editDraft = useCallback(
(value: string) => {
setDraft(value);
claimEmptyEdit(value.trim().length === 0);
steering.updateQueuedText(message.id, value);
},
[claimEmptyEdit, message.id, steering],
[message.id, steering],
);
/** Leaving the editor settles the row: trimmed if there are words, and back to
* the pre-edit text if there are not. That is what keeps a blank row from
* ever outliving its editor, which is in turn why every reader can trust
* `isSendableQueuedMessage` on a resting queue. */
const closeEdit = useCallback(() => {
claimEmptyEdit(false);
const typed = draftRef.current;
if (typed == null) {
return;
}
steering.updateQueuedText(
message.id,
typed.trim().length === 0 ? originalRef.current : typed.trim(),
);
setDraft(null);
}, [claimEmptyEdit]);
}, [message.id, steering]);
const abandonEdit = useCallback(() => {
steering.updateQueuedText(message.id, originalRef.current);
closeEdit();
}, [closeEdit, message.id, steering]);
setDraft(null);
}, [message.id, steering]);
const emptyEditRef = useRef(false);
emptyEditRef.current = emptyEdit;
const closeEditRef = useRef(closeEdit);
closeEditRef.current = closeEdit;
const abandonEditRef = useRef(abandonEdit);
abandonEditRef.current = abandonEdit;
/** An editor removed by a remount fires no blur, so it settles on the way out
* — otherwise a blank row could survive the group collapsing around it. */
useEffect(() => () => closeEditRef.current(), []);
/** A row whose send is pending closes its editor: the words are already
* written, and a countdown is no moment to keep typing into. An EMPTY editor
@ -209,14 +203,9 @@ function QueuedRowBase({
* closing it alone would leave the queue holding words the screen no longer
* shows, and the drain would send those. */
useEffect(() => {
if (!sendPending) {
return;
if (sendPending) {
closeEditRef.current();
}
if (emptyEditRef.current) {
abandonEditRef.current();
return;
}
closeEditRef.current();
}, [sendPending]);
/** An ordinary row is a living draft: it is rewritten in place. A recovered
@ -317,8 +306,8 @@ function QueuedRowBase({
<button
type="button"
className={PRIMARY_BTN_CLASS}
disabled={actionPending || emptyEdit}
title={emptyEdit ? localize('com_ui_queue_edit_empty') : undefined}
disabled={actionPending || !sendable}
title={sendable ? undefined : localize('com_ui_queue_edit_empty')}
onClick={() => steering.sendQueuedNow(message)}
>
{canSteerNow ? (
@ -338,7 +327,7 @@ function QueuedRowBase({
<EscalateNowButton
surface="queued"
messageText={message.text}
disabled={steering.pausedOnApproval || interruptPending || actionPending || emptyEdit}
disabled={steering.pausedOnApproval || interruptPending || actionPending || !sendable}
onClick={() => steering.sendQueuedNow(message, { preempt: true })}
/>
)}
@ -356,7 +345,7 @@ function QueuedRowBase({
* pre-edit words, so handing them back would resurrect text the
* user had visibly deleted. Emptying a row and then removing it
* reads as "delete this", and there is nothing to return. */
if (!emptyEdit) {
if (sendable) {
onRestoreToComposer(
message.text,
message.files,
@ -463,7 +452,10 @@ function QueuedOutboxBase({
const localize = useLocalize();
const { showToast } = useToastContext();
const [expanded, setExpanded] = useAtom(queueExpandedFamily(steering.queueKey));
const emptyEditId = useAtomValue(queueEmptyEditFamily(steering.queueKey));
/** Every gate below reads the rows this component already holds. That is the
* whole point of recording blank text instead of refusing it: sendability is
* a property of the data, so there is no shared claim to keep in step. */
const allSendable = useMemo(() => queued.every(isSendableQueuedMessage), [queued]);
const mergeable = useMemo(() => queued.every(isMergeableQueuedMessage), [queued]);
/** Folding reads the queue, so an unresolved empty edit would carry the words
* the user just deleted into the merged message. Same standdown the senders
@ -472,12 +464,12 @@ function QueuedOutboxBase({
if (!mergeable) {
return localize('com_ui_queue_merge_blocked');
}
return emptyEditId != null ? localize('com_ui_queue_edit_empty') : undefined;
return allSendable ? undefined : localize('com_ui_queue_edit_empty');
})();
/** Clear all folds the queue the same way Merge does, so an unresolved empty
* edit would hand words the user deleted back to the composer. Uniform with
* every other reader rather than an exception worth remembering. */
const clearBlockedReason = emptyEditId != null ? localize('com_ui_queue_edit_empty') : undefined;
const clearBlockedReason = allSendable ? undefined : localize('com_ui_queue_edit_empty');
/** The shortcut's promise is the NEWEST waiting message, which is not the
* last array slot once a promotion has reordered the queue — so pick by
* stamp. Recovery-bound rows are skipped because steering refuses them
@ -649,7 +641,7 @@ function QueuedOutboxBase({
* user deleted. Disabled rather than absent: the shortcut skips
* unavailable controls, so it falls through instead of sending
* them. */
disabled={emptyEditId === escalatable.id}
disabled={!isSendableQueuedMessage(escalatable)}
className="sr-only"
data-escalate-steer="queued"
data-testid="queued-escalate-newest"

39
client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx
View file

@ -5,7 +5,7 @@ import { RecoilRoot, useRecoilValue, useSetRecoilState } from 'recoil';
import { render, screen, fireEvent, act, waitFor } from '@testing-library/react';
import type { PendingSteer, QueuedMessage, QueueDrainHold } from '~/store/families';
import type { SteeringControls } from '~/hooks/Chat/useSteering';
import { escalatingSteerFamily, queueEmptyEditFamily, queueExpandedFamily } from '~/store/steer';
import { escalatingSteerFamily, queueExpandedFamily } from '~/store/steer';
import PendingSteerChips from '../PendingSteerChips';
import store from '~/store';
@ -76,7 +76,6 @@ const steeringStub = (overrides: Partial<SteeringControls> = {}) =>
beforeEach(() => {
act(() => {
getDefaultStore().set(queueExpandedFamily(CONVO_ID), false);
getDefaultStore().set(queueEmptyEditFamily(CONVO_ID), null);
});
});
@ -564,7 +563,14 @@ describe('PendingSteerChips — queued interrupt-now', () => {
});
const mockBumpQueued = jest.fn();
const mockUpdateQueuedText = jest.fn(() => true);
/** Mirrors the real writer: records what is typed, blank included, so the rows
* under test see the same queue the app would. */
const mockUpdateQueuedText = jest.fn((id: string, text: string) => {
updateQueueForTest?.((current) =>
current.map((item) => (item.id === id ? { ...item, text } : item)),
);
return true;
});
const mockMergeQueued = jest.fn(() => true);
const mockCancelQueueDrain = jest.fn();
const mockEnqueue = jest.fn();
@ -1185,6 +1191,33 @@ describe('PendingSteerChips — queued row editing', () => {
expect(screen.getByTestId('queued-escalate-newest')).not.toBeDisabled();
});
/** The invariant that lets every reader trust the data: a blank row cannot
* outlive its editor, so a resting queue never holds one. */
it('restores the original when an emptied editor is closed rather than abandoned', () => {
renderChips([twoQueued[0]], { steering: outboxSteering() });
fireEvent.click(screen.getByText('first thought'));
const editor = screen.getByTestId('queued-message-edit');
fireEvent.change(editor, { target: { value: '' } });
// Blank while the editor is open, which is what the senders read.
expect(JSON.parse(screen.getByTestId('queue-state').textContent ?? '[]')[0].text).toBe('');
fireEvent.blur(editor);
expect(mockUpdateQueuedText).toHaveBeenLastCalledWith('q1', 'first thought');
expect(screen.queryByTestId('queued-message-edit')).toBeNull();
});
it('trims the words it settles', () => {
renderChips([twoQueued[0]], { steering: outboxSteering() });
fireEvent.click(screen.getByText('first thought'));
const editor = screen.getByTestId('queued-message-edit');
fireEvent.change(editor, { target: { value: ' spaced out ' } });
fireEvent.keyDown(editor, { key: 'Enter' });
expect(mockUpdateQueuedText).toHaveBeenLastCalledWith('q1', 'spaced out');
});
it('puts the original words back on Escape', () => {
renderChips([twoQueued[0]], { steering: outboxSteering() });

33
client/src/hooks/Chat/__tests__/useQueueDrain.spec.tsx
View file

@ -97,6 +97,39 @@ describe('useQueueDrain', () => {
expect(ask).toHaveBeenCalledWith({ text: 'first follow-up' }, emptyOverrides);
});
/** A row being edited holds exactly what is typed, blank included. Blank is
* not a message, and skipping to the row behind it would send out of order —
* so this epoch drains nothing and the next run end picks the queue up. */
it('drains nothing when the front row is mid-edit and blank', async () => {
const { ask, setters } = setup(({ set }) => {
set(store.queuedMessagesByConvoId(CONVO_ID), [
{ id: 'q1', text: ' ', createdAt: 1 },
queuedMessage('q2', 'behind it'),
]);
});
act(() => {
setters.setRunEnd!(runEnd());
});
await new Promise((resolve) => setTimeout(resolve, 25));
expect(ask).not.toHaveBeenCalled();
// The epoch was consumed, so the effect is not spinning; a later run end
// drains normally once the row has words again.
act(() => {
setters.setQueue!([
queuedMessage('q1', 'now it has words'),
queuedMessage('q2', 'behind it'),
]);
});
act(() => {
setters.setRunEnd!(runEnd({ generationCreatedAt: 77 }));
});
await waitFor(() => expect(ask).toHaveBeenCalledTimes(1));
expect(ask).toHaveBeenCalledWith({ text: 'now it has words' }, emptyOverrides);
});
it('parks a mismatched signal instead of draining into the wrong conversation', async () => {
const { ask, setters } = setup(({ set }) => {
set(store.queuedMessagesByConvoId(CONVO_ID), [queuedMessage('q1', 'stay put')]);

9
client/src/hooks/Chat/useQueueDrain.ts
View file

@ -3,7 +3,7 @@ import { Constants } from 'librechat-data-provider';
import { useRecoilValue, useRecoilCallback } from 'recoil';
import type { DrainAfterAbort, QueuedMessage, QueuedMessageOrigin, RunEnd } from '~/store/families';
import type { TAskFunction } from '~/common';
import { compareQueuedMessages, isSameRunEpoch } from '~/utils';
import { compareQueuedMessages, isSameRunEpoch, isSendableQueuedMessage } from '~/utils';
import { useMarkFilesUsageMutation } from '~/data-provider';
import store from '~/store';
@ -311,7 +311,12 @@ export default function useQueueDrain(
});
return null;
}
const next = shouldDrain ? (merged[0] ?? null) : null;
/** A row mid-edit can be blank, and blank is not a message. Skipping to
* the row behind it would send out of order, so this epoch simply
* drains nothing — consumed above, so the effect cannot spin — and the
* next run end picks the queue up again. */
const front = merged[0] ?? null;
const next = shouldDrain && front != null && isSendableQueuedMessage(front) ? front : null;
const remainder = next ? merged.slice(1) : merged;
if (shouldMigrate && newConvoQueue.length > 0) {

18
client/src/hooks/Chat/useSteering.ts
View file

@ -16,6 +16,7 @@ import {
insertQueuedOrigin,
isMergeableQueuedMessage,
isSameRunEpoch,
isSendableQueuedMessage,
mergeQueuedMessages,
} from '~/utils';
import {
@ -687,18 +688,23 @@ export default function useSteering({
const updateQueuedText = useRecoilCallback(
({ snapshot, set }) =>
(id: string, text: string): boolean => {
const trimmed = text.trim();
const queue = snapshot.getLoadable(store.queuedMessagesByConvoId(queueKey)).getValue();
const target = queue.find((item) => item.id === id);
if (trimmed.length === 0 || target == null || !isMergeableQueuedMessage(target)) {
if (target == null || !isMergeableQueuedMessage(target)) {
return false;
}
if (target.text === trimmed) {
/** Blank is recorded, not refused. Refusing it was the root of a whole
* family of bugs: the queue kept words the screen no longer showed, and
* every reader — the drain, Send now, the escalate shortcut, Merge,
* Clear all, the trash — had to be taught about that disagreement.
* Now the row simply is not sendable, which each reader can see for
* itself via `isSendableQueuedMessage`. */
if (target.text === text) {
return true;
}
set(
store.queuedMessagesByConvoId(queueKey),
queue.map((item) => (item.id === id ? { ...item, text: trimmed } : item)),
queue.map((item) => (item.id === id ? { ...item, text } : item)),
);
return true;
},
@ -1468,6 +1474,10 @@ export default function useSteering({
if (isSubmitting && (!duringRunActive || !canSteer || item.recoverySteerId != null)) {
return;
}
/** Nothing to send: the row is mid-edit and currently blank. */
if (!isSendableQueuedMessage(item)) {
return;
}
/** UI callers always find the item; a stale/direct caller has no original
* neighbours, so restoration falls back to the queue's priority split. */
const origin = takeQueued(item.id) ?? { item, beforeIds: [], afterIds: [] };

10
client/src/store/steer.ts
View file

@ -28,13 +28,3 @@ export const escalatingSteerFamily = atomFamily((_conversationId: string) => ato
* not survive a reload, so persisting the disclosure would outlive its subject.
*/
export const queueExpandedFamily = atomFamily((_queueKey: string) => atom<boolean>(false));
/**
* Id of the queued row whose inline editor is currently empty, if any. The
* write-through refuses blank text, so that row still holds words the user has
* visibly deleted and nothing may send it. The row owns its editor state but
* the group owns the shortcut proxy, so the fact has to live where both can
* see it. At most one editor is open at a time — opening another blurs the
* first — so a single id is enough.
*/
export const queueEmptyEditFamily = atomFamily((_queueKey: string) => atom<string | null>(null));

15
client/src/utils/__tests__/steer.spec.ts
View file

@ -3,6 +3,7 @@ import type { TMessage, TSteerAppliedEvent } from 'librechat-data-provider';
import type { QueuedMessage } from '~/store/families';
import {
isSameRunEpoch,
isSendableQueuedMessage,
getSteerPart,
applySteerPart,
resolveRunEndTarget,
@ -385,7 +386,21 @@ describe('isMergeableQueuedMessage', () => {
});
});
describe('isSendableQueuedMessage', () => {
it('is the whole rule: a row with words is sendable, one without is not', () => {
expect(isSendableQueuedMessage(queued({ id: 'a', text: 'words' }))).toBe(true);
expect(isSendableQueuedMessage(queued({ id: 'b', text: '' }))).toBe(false);
expect(isSendableQueuedMessage(queued({ id: 'c', text: ' \n ' }))).toBe(false);
});
});
describe('mergeQueuedMessages', () => {
it('refuses a batch containing a row being emptied, so a fold cannot bake in a blank', () => {
expect(
mergeQueuedMessages([queued({ id: 'a', text: 'kept' }), queued({ id: 'b', text: ' ' })]),
).toBeNull();
});
it('joins texts in drain order as paragraphs', () => {
const merged = mergeQueuedMessages([
queued({ id: 'a', text: 'first thought', createdAt: 1 }),

16
client/src/utils/steer.ts
View file

@ -198,6 +198,17 @@ export function compareQueuedMessages(a: QueuedMessage, b: QueuedMessage): numbe
return a.createdAt - b.createdAt;
}
/**
* Whether a row can be sent as it stands. A row being edited holds exactly what
* the user has typed, blank included, so "there is nothing to send" is a fact
* about the row rather than a state every sender has to be told about
* separately. Blank rows exist only while their editor is open — closing it
* restores the pre-edit words — so this never describes a resting queue.
*/
export function isSendableQueuedMessage(item: QueuedMessage): boolean {
return item.text.trim().length > 0;
}
/** Queued texts are separate thoughts, so a join reads as paragraphs. */
export const QUEUED_TEXT_SEPARATOR = '\n\n';
@ -246,6 +257,11 @@ export function mergeQueuedMessages(items: QueuedMessage[]): QueuedMessage | nul
if (items.length < 2 || items.some((item) => !isMergeableQueuedMessage(item))) {
return null;
}
/** Folding a row mid-edit would bake in whatever is on screen at that instant,
* blank included. Self-protecting here so no caller has to remember. */
if (items.some((item) => !isSendableQueuedMessage(item))) {
return null;
}
const [first] = items;
const files = dedupeFiles(items);
const quotes = dedupeStrings(items.map((item) => item.quotes));