mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🪗 fix: Let Settled Labels Collapse Void Tools and Keep the Tail Cursor
Round-eleven review (all P2, client-side). Two fixed; the other two findings restate documented Known limitations (edited+reconnect run-step index space; parallel-lane collapsible headers), answered on-thread. - Void-tool auto-collapse (ToolCallGroup.tsx): `allCompleted` keyed solely on output truthiness, so a tool that legitimately returns an empty string kept its labeled group expanded forever. A settled, filled label is itself a completion proof — the PostToolBatch claim only happens after every output in the batch returned — so it now satisfies `allCompleted`; pending labels keep the group live. - Trailing-reservation cursor (ContentParts.tsx): a blank label reservation at the content tail renders nothing but still counted as the last part, stripping the streaming cursor and last-item affordances from the last VISIBLE part until the next delta. `lastContentIdx` now walks back past empty label slots. Tests: labeled void-tool group auto-collapses, pending-label group stays expanded (ToolCallGroup.test).
This commit is contained in:
parent
612327f781
commit
bb5c4c5e3b
3 changed files with 77 additions and 5 deletions
|
|
@ -8,6 +8,7 @@ import type {
|
|||
} from 'librechat-data-provider';
|
||||
import type { ToolCallGroupExpansionState } from './ToolCallGroup';
|
||||
import { mapAttachments, filterAttachmentsForPart, groupSequentialToolCalls } from '~/utils';
|
||||
import { getActivityLabelPart, getActivityLabelText } from '~/utils/activityLabels';
|
||||
import { ParallelContentRenderer, type PartWithIndex } from './ParallelContent';
|
||||
import { MessageContext, SearchContext } from '~/Providers';
|
||||
import PendingSkillCall from './Parts/PendingSkillCall';
|
||||
|
|
@ -393,7 +394,22 @@ const ContentParts = memo(function ContentParts({
|
|||
|
||||
const safeContent = content ?? [];
|
||||
const showEmptyCursor = safeContent.length === 0 && effectiveIsSubmitting;
|
||||
const lastContentIdx = safeContent.length - 1;
|
||||
/** A trailing BLANK label reservation renders nothing, so counting it as
|
||||
* the last part would strip the streaming cursor and last-item
|
||||
* affordances from the last VISIBLE part for the whole interval until
|
||||
* the next delta. Walk back past invisible label slots. */
|
||||
let lastContentIdx = safeContent.length - 1;
|
||||
while (lastContentIdx > 0) {
|
||||
const tail = safeContent[lastContentIdx];
|
||||
if (
|
||||
tail?.type === ContentTypes.ACTIVITY_LABEL &&
|
||||
getActivityLabelText(getActivityLabelPart(tail)).length === 0
|
||||
) {
|
||||
lastContentIdx -= 1;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Parallel content: use dedicated renderer with columns (TMessageContentParts includes ContentMetadata)
|
||||
const hasParallelContent = safeContent.some((part) => part?.groupId != null);
|
||||
|
|
|
|||
|
|
@ -151,13 +151,20 @@ export default function ToolCallGroup({
|
|||
() => parts.some(({ part }) => hasPendingApprovalInPart(part)),
|
||||
[parts],
|
||||
);
|
||||
const allCompleted = useMemo(
|
||||
() => toolMetadata.every((m) => m.hasOutput === true),
|
||||
[toolMetadata],
|
||||
);
|
||||
const activityLabel = getActivityLabelPart(labelPart?.part);
|
||||
const activityLabelText = getActivityLabelText(activityLabel);
|
||||
const activityFailed = activityLabel?.status === 'failed' || activityLabel?.status === 'partial';
|
||||
/** A settled, filled label is itself a completion proof: the PostToolBatch
|
||||
* claim only happens after every output in the batch returned. Without
|
||||
* it, a tool that legitimately returns an empty string reads as
|
||||
* `hasOutput: false` forever and its labeled group never auto-collapses. */
|
||||
const labelSettled =
|
||||
activityLabelText.length > 0 &&
|
||||
(labelPart?.part as { pending?: boolean } | undefined)?.pending !== true;
|
||||
const allCompleted = useMemo(
|
||||
() => labelSettled || toolMetadata.every((m) => m.hasOutput === true),
|
||||
[toolMetadata, labelSettled],
|
||||
);
|
||||
const toolNames = useMemo(() => toolMetadata.map((m) => m.name), [toolMetadata]);
|
||||
const iconToolNames = useMemo(() => toolMetadata.map((m) => m.iconName), [toolMetadata]);
|
||||
|
||||
|
|
|
|||
|
|
@ -203,6 +203,55 @@ describe('ToolCallGroup image hoisting', () => {
|
|||
expect(mockScheduleMessageContentLayoutReconcile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/** A settled label proves the batch finished — a void tool's legitimate
|
||||
* empty output must not keep its labeled group expanded forever. */
|
||||
it('auto-collapses a labeled group whose only tool returned an empty output', () => {
|
||||
const voidToolParts = [{ part: makePart('t1', '', 'update_settings'), idx: 0 }];
|
||||
const labelPart = {
|
||||
part: {
|
||||
type: ContentTypes.ACTIVITY_LABEL,
|
||||
[ContentTypes.ACTIVITY_LABEL]: 'Updated the notification settings',
|
||||
pending: false,
|
||||
} as unknown as TMessageContentParts,
|
||||
idx: 1,
|
||||
};
|
||||
|
||||
renderGroup({
|
||||
...baseProps,
|
||||
parts: voidToolParts,
|
||||
lastContentIdx: 1,
|
||||
labelPart,
|
||||
});
|
||||
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Updated the notification settings' }),
|
||||
).toBeInTheDocument();
|
||||
/** Collapsed: bodies not mounted. */
|
||||
expect(screen.queryByTestId('inner-0')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('keeps a pending-label group expanded while its tool has no output', () => {
|
||||
const voidToolParts = [{ part: makePart('t1', '', 'update_settings'), idx: 0 }];
|
||||
const labelPart = {
|
||||
part: {
|
||||
type: ContentTypes.ACTIVITY_LABEL,
|
||||
[ContentTypes.ACTIVITY_LABEL]: '',
|
||||
pending: true,
|
||||
} as unknown as TMessageContentParts,
|
||||
idx: 1,
|
||||
};
|
||||
|
||||
renderGroup({
|
||||
...baseProps,
|
||||
parts: voidToolParts,
|
||||
lastContentIdx: 1,
|
||||
labelPart,
|
||||
isSubmitting: true,
|
||||
});
|
||||
|
||||
expect(screen.getByTestId('inner-0')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not render tool bodies for an initially collapsed large completed group', () => {
|
||||
const largeParts = Array.from({ length: 59 }, (_, idx) => ({
|
||||
part: makePart(`t${idx}`),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue