diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index acf419dd9e..361b029fd4 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -26,6 +26,7 @@ import WorkspaceChanges, { partitionWorkspaceChanges } from './Parts/WorkspaceCh import { ParallelContentRenderer, type PartWithIndex } from './ParallelContent'; import { MediaContext, MessageContext, SearchContext } from '~/Providers'; import MemoryArtifacts, { hasMemoryArtifacts } from './MemoryArtifacts'; +import { hasParallelLanes, parallelLaneGroups } from '~/utils/lanes'; import PendingSkillCall from './Parts/PendingSkillCall'; import ActivityPhaseGroup from './ActivityPhaseGroup'; import { hasPendingApprovalInPart } from '~/utils'; @@ -234,6 +235,9 @@ type ContentPartsProps = { contentIndexOffset?: number; /** Absolute transcript index for each compacted sparse segment entry. */ contentIndices?: ReadonlyArray; + /** Message-wide lane cardinality retained across nested phase segments, so a + * slice holding one agent of a real two-agent group keeps its columns. */ + laneGroups?: ReadonlySet; /** Message-wide steer attribution retained across nested phase segments. */ resumeAuthors?: ReadonlyMap; /** Message-wide tool-group expansion overrides retained across phase slices. */ @@ -274,6 +278,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ workspaceAttachmentsPartitioned = false, contentIndexOffset = 0, contentIndices, + laneGroups, resumeAuthors, toolGroupExpansionState, toolGroupOccurrenceByIndex, @@ -343,9 +348,16 @@ const ContentPartsBody = memo(function ContentPartsBody({ /** Hoisted above the early returns to feed the entrance-detection hook * below, so it is memoized rather than re-walked on every unrelated * re-render of a message that has no phases at all. */ + /** Resolved once over the whole message and handed to every slice below — + * and to `groupActivityPhases`, so a streamed delta scans content once. */ + const messageLaneGroups = useMemo( + () => laneGroups ?? parallelLaneGroups(content), + [laneGroups, content], + ); + const phaseSegments = useMemo( - () => (nestedActivityPhase ? undefined : groupActivityPhases(content)), - [nestedActivityPhase, content], + () => (nestedActivityPhase ? undefined : groupActivityPhases(content, messageLaneGroups)), + [nestedActivityPhase, content, messageLaneGroups], ); /** Every file a phase's parts produced, in transcript order, deduplicated * across parts that share a tool call. */ @@ -804,6 +816,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ workspaceAttachmentsPartitioned contentIndexOffset={segmentStartIndex} contentIndices={segmentIndices} + laneGroups={messageLaneGroups} resumeAuthors={postSteerAuthors} toolGroupExpansionState={expansionState} toolGroupOccurrenceByIndex={resolvedToolGroupOccurrences} @@ -811,7 +824,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ /> ); }; - const hasParallelContent = content?.some((part) => part?.groupId != null) === true; + const hasParallelContent = hasParallelLanes(content, messageLaneGroups); return ( @@ -960,8 +973,9 @@ const ContentPartsBody = memo(function ContentPartsBody({ const relativeLastContentIdx = lastCursorContentIdx(safeContent); const lastContentIdx = relativeLastContentIdx < 0 ? -1 : absoluteIndexAt(relativeLastContentIdx); - // Parallel content: use dedicated renderer with columns (TMessageContentParts includes ContentMetadata) - const hasParallelContent = safeContent.some((part) => part?.groupId != null); + /** Columns only when at least two agents share a group — a lone group + * renders here, where tool grouping and activity labels apply. */ + const hasParallelContent = hasParallelLanes(safeContent, messageLaneGroups); if (hasParallelContent) { const parallelContent = ( <> @@ -979,6 +993,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ showDecorations={!nestedActivityPhase} contentIndexOffset={contentIndexOffset} contentIndices={contentIndices} + laneGroups={messageLaneGroups} /> {!nestedActivityPhase && } diff --git a/client/src/components/Chat/Messages/Content/ParallelContent.tsx b/client/src/components/Chat/Messages/Content/ParallelContent.tsx index 27b961a3df..5a156db3d8 100644 --- a/client/src/components/Chat/Messages/Content/ParallelContent.tsx +++ b/client/src/components/Chat/Messages/Content/ParallelContent.tsx @@ -1,4 +1,4 @@ -import { memo, useMemo } from 'react'; +import { memo, useMemo, Fragment } from 'react'; import { ContentTypes } from 'librechat-data-provider'; import type { TMessageContentParts, SearchResultData, TAttachment } from 'librechat-data-provider'; import { @@ -6,6 +6,7 @@ import { getActivityLabelText, lastCursorContentIdx, } from '~/utils/activityLabels'; +import { MIN_PARALLEL_LANES, UNATTRIBUTED_LANE, isLaneMarkerPart } from '~/utils/lanes'; import MemoryArtifacts from './MemoryArtifacts'; import Sources from '~/components/Web/Sources'; import { cn, getPartKeyIndex } from '~/utils'; @@ -37,6 +38,7 @@ export function groupParallelContent( content: Array | undefined, contentIndexOffset = 0, contentIndices?: ReadonlyArray, + laneGroups?: ReadonlySet, ): { parallelSections: ParallelSection[]; sequentialParts: PartWithIndex[] } { if (!content) { return { parallelSections: [], sequentialParts: [] }; @@ -89,7 +91,7 @@ export function groupParallelContent( for (const { part, idx } of parts) { // Read agentId directly from content part (TMessageContentParts includes ContentMetadata) - const agentId = part.agentId ?? 'unknown'; + const agentId = part.agentId ?? UNATTRIBUTED_LANE; if (!columnMap.has(agentId)) { columnMap.set(agentId, []); @@ -127,6 +129,31 @@ export function groupParallelContent( parts: columnMap.get(agentId)!, })); + /** Which columns are an agent's own output, mirroring `laneAgentsByGroup`: + * the sentinel column belongs to no agent, and a column holding only a + * handoff marker is nothing to compare against. A placeholder column has + * no parts at all and still claims its lane — that is how a dual run + * shows both agents from the first render. */ + const claimedColumns = columns.filter( + ({ agentId, parts }) => + agentId !== UNATTRIBUTED_LANE && + (parts.length === 0 || parts.some(({ part }) => !isLaneMarkerPart(part))), + ).length; + + /** One column is not a comparison. Its parts rejoin the sequential flow, + * where tool grouping, activity-label headers and phase folds apply and + * the message's own author header is the only attribution shown. + * + * `laneGroups` is the message-level verdict: a phase slice holding one + * agent of a real two-agent group keeps its columns, because the group + * is a comparison even where this slice cannot show it. */ + if (claimedColumns < MIN_PARALLEL_LANES && laneGroups?.has(groupId) !== true) { + for (const column of columns) { + noGroup.push(...column.parts); + } + continue; + } + sections.push({ groupId, columns }); } @@ -139,6 +166,10 @@ export function groupParallelContent( return aMin - bMin; }); + /** Demoted lane parts are appended after the parts that preceded them, so + * restore transcript order before the before/after split reads indexes. */ + noGroup.sort((a, b) => a.idx - b.idx); + return { parallelSections: sections, sequentialParts: noGroup }; } @@ -241,6 +272,8 @@ type ParallelContentRendererProps = { contentIndexOffset?: number; /** Absolute transcript index for each compacted sparse segment entry. */ contentIndices?: ReadonlyArray; + /** Message-level lane cardinality, so a phase slice does not recount. */ + laneGroups?: ReadonlySet; }; /** @@ -260,10 +293,11 @@ export const ParallelContentRenderer = memo(function ParallelContentRenderer({ showDecorations = true, contentIndexOffset = 0, contentIndices, + laneGroups, }: ParallelContentRendererProps) { const { parallelSections, sequentialParts } = useMemo( - () => groupParallelContent(content, contentIndexOffset, contentIndices), - [content, contentIndexOffset, contentIndices], + () => groupParallelContent(content, contentIndexOffset, contentIndices, laneGroups), + [content, contentIndexOffset, contentIndices, laneGroups], ); /** Same walk-back as `ContentParts`: a trailing BLANK label reservation is @@ -275,24 +309,37 @@ export const ParallelContentRenderer = memo(function ParallelContentRenderer({ ? -1 : (contentIndices?.[relativeLastContentIdx] ?? relativeLastContentIdx + contentIndexOffset); - // Split sequential parts into before/after parallel sections - const { before, after } = useMemo(() => { - if (parallelSections.length === 0) { - return { before: sequentialParts, after: [] }; + /** Sequential parts are laid out AROUND each section rather than split once + * around all of them: two groups with ordinary content between them would + * otherwise render both groups first and the intervening part last. + * + * A part landing between a section's own first and last index has no column + * and no slot inside one, so it falls to the next block — after the lanes it + * interleaves with. Bounding it by the last lane index instead dropped it + * from the message entirely, and a placeholder-only section made + * `Math.min(...[])` Infinity, which rendered every part twice. */ + const { blocks, trailing } = useMemo(() => { + const laid: Array<{ leading: PartWithIndex[]; section: ParallelSection }> = []; + let cursor = 0; + for (const section of parallelSections) { + const indices = section.columns.flatMap((column) => column.parts.map((part) => part.idx)); + const sectionStart = indices.length > 0 ? Math.min(...indices) : Infinity; + const leading: PartWithIndex[] = []; + while (cursor < sequentialParts.length && sequentialParts[cursor].idx < sectionStart) { + leading.push(sequentialParts[cursor]); + cursor += 1; + } + laid.push({ leading, section }); } - - const allParallelIndices = parallelSections.flatMap((s) => - s.columns.flatMap((c) => c.parts.map((p) => p.idx)), - ); - const minParallelIdx = Math.min(...allParallelIndices); - const maxParallelIdx = Math.max(...allParallelIndices); - - return { - before: sequentialParts.filter(({ idx }) => idx < minParallelIdx), - after: sequentialParts.filter(({ idx }) => idx > maxParallelIdx), - }; + return { blocks: laid, trailing: sequentialParts.slice(cursor) }; }, [parallelSections, sequentialParts]); + const renderSequential = ({ part, idx }: PartWithIndex) => { + const attribution = renderResumeAttribution?.(idx, getPartKeyIndex(part, idx)); + const rendered = renderPart(part, idx, idx === lastContentIdx); + return attribution != null ? [attribution, rendered] : [rendered]; + }; + return ( {showDecorations && } @@ -300,34 +347,25 @@ export const ParallelContentRenderer = memo(function ParallelContentRenderer({ )} - {/* Sequential content BEFORE parallel sections */} - {before.flatMap(({ part, idx }) => { - const attribution = renderResumeAttribution?.(idx, getPartKeyIndex(part, idx)); - const rendered = renderPart(part, idx, false); - return attribution != null ? [attribution, rendered] : [rendered]; - })} - - {/* Parallel sections - each group renders as columns */} - {parallelSections.map(({ groupId, columns }) => ( - + {/* Each section preceded by the sequential content that runs up to it */} + {blocks.map(({ leading, section: { groupId, columns } }) => ( + + {leading.flatMap(renderSequential)} + + ))} - {/* Sequential content AFTER parallel sections */} - {after.flatMap(({ part, idx }) => { - const attribution = renderResumeAttribution?.(idx, getPartKeyIndex(part, idx)); - const rendered = renderPart(part, idx, idx === lastContentIdx); - return attribution != null ? [attribution, rendered] : [rendered]; - })} + {/* Sequential content after the last section */} + {trailing.flatMap(renderSequential)} ); }); diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx index cbae29dd16..5f4ea691f9 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx @@ -121,6 +121,13 @@ jest.mock('../Container', () => ({ default: ({ children }: { children?: React.ReactNode }) =>
{children}
, })); +jest.mock('../SiblingHeader', () => ({ + __esModule: true, + default: ({ agentId }: { agentId?: string }) => ( +
+ ), +})); + jest.mock('~/utils', () => { const actual = jest.requireActual('~/utils'); return { @@ -914,3 +921,106 @@ describe('ContentParts integration: phase media row', () => { expect(screen.queryByTestId('attachment-group')).toBeNull(); }); }); + +describe('ContentParts integration: lane groups backed by one agent', () => { + const baseProps = { + messageId: 'msg-lane', + isCreatedByUser: false, + isLast: true, + isSubmitting: false, + isLatestMessage: true, + }; + + const inLane = (part: TMessageContentParts, agentId: string, groupId = 1): TMessageContentParts => + ({ ...(part as object), agentId, groupId }) as unknown as TMessageContentParts; + + it('renders no lane header when a phase marker splits a single-agent group', () => { + /** The reported shape: a multi-agent graph stamped a group id on the + * primary agent's own output, and a phase marker ending mid-group split + * it — so one lane rendered as TWO bordered cards, each repeating the + * message's own author, and the answer read as a separate message. */ + const answer = 'Monitoring the pull requests for the next hour.'; + const content = [ + makeMcpToolCall('t1'), + makeMcpToolCall('t2'), + inLane(makeMcpToolCall('t3'), 'agent_a'), + inLane(makeTextPart(answer), 'agent_a'), + makePhasePart(0, 3, 'Planned the monitoring run'), + ]; + + renderContentParts({ ...baseProps, content }); + /** The card mounts collapsed and its body is lazy, so open it — the split + * drew one header inside the card and one after it. */ + fireEvent.click(screen.getByRole('button', { name: 'Planned the monitoring run' })); + + expect(screen.queryAllByTestId('sibling-header')).toHaveLength(0); + expect(screen.getAllByText(answer)).toHaveLength(1); + }); + + it('groups the tool calls of a single-agent group, which lanes never do', () => { + const content = [ + inLane(makeMcpToolCall('t1'), 'agent_a'), + inLane(makeMcpToolCall('t2'), 'agent_a'), + ]; + + renderContentParts({ ...baseProps, content }); + + expect(screen.getByRole('button', { name: 'Used 2 tools' })).toBeInTheDocument(); + }); + + it('still renders columns once a second agent shares the group', () => { + const content = [ + inLane(makeTextPart('primary answer'), 'agent_a'), + inLane(makeTextPart('added answer'), 'agent_b____1'), + ]; + + renderContentParts({ ...baseProps, content }); + + const headers = screen.getAllByTestId('sibling-header'); + expect(headers.map((header) => header.getAttribute('data-agent-id'))).toEqual([ + 'agent_a', + 'agent_b____1', + ]); + }); + + it('keeps lane attribution when a phase marker leaves one agent per slice', () => { + /** Lane cardinality belongs to the MESSAGE. A marker that puts each agent + * of a real two-agent group in a different slice would otherwise demote + * both, and the added agent's answer would read as the primary's. */ + const content = [ + inLane(makeTextPart('primary answer'), 'agent_a'), + inLane(makeTextPart('added answer'), 'agent_b____1'), + makePhasePart(0, 1, 'Ran both agents'), + ]; + + renderContentParts({ ...baseProps, content }); + fireEvent.click(screen.getByRole('button', { name: 'Ran both agents' })); + + const agentIds = screen + .getAllByTestId('sibling-header') + .map((header) => header.getAttribute('data-agent-id')); + expect(agentIds).toContain('agent_a'); + expect(agentIds).toContain('agent_b____1'); + }); + + it('keeps content that runs between two lane groups in transcript order', () => { + const content = [ + inLane(makeTextPart('first wave primary'), 'agent_a', 1), + inLane(makeTextPart('first wave added'), 'agent_b____1', 1), + makeTextPart('handover note between the waves'), + inLane(makeTextPart('second wave primary'), 'agent_a', 2), + inLane(makeTextPart('second wave added'), 'agent_b____1', 2), + ]; + + renderContentParts({ ...baseProps, content }); + + const transcript = document.body.textContent ?? ''; + expect(transcript).toContain('handover note between the waves'); + expect(transcript.indexOf('handover note between the waves')).toBeGreaterThan( + transcript.indexOf('first wave primary'), + ); + expect(transcript.indexOf('handover note between the waves')).toBeLessThan( + transcript.indexOf('second wave primary'), + ); + }); +}); diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx index 08d1ec900c..7e3e96c855 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx @@ -265,11 +265,19 @@ describe('ContentParts — interim skill cards', () => { }); it('renders pending skill cards above parallel content', () => { + /** Two agents, so the group renders as columns at all. */ const parallelContent: TMessageContentParts[] = [ { type: ContentTypes.TEXT, - text: 'parallel', - groupId: 'group-1', + text: 'primary', + agentId: 'agent_a', + groupId: 1, + } as unknown as TMessageContentParts, + { + type: ContentTypes.TEXT, + text: 'added', + agentId: 'agent_b____1', + groupId: 1, } as unknown as TMessageContentParts, ]; render(); @@ -421,15 +429,17 @@ describe('ContentParts — post-steer author re-attribution', () => { }); it('provides resume attribution to the parallel renderer for its sequential stretches', () => { - const parallelText = { - type: ContentTypes.TEXT, - text: 'column', - groupId: 1, - } as unknown as TMessageContentParts; + const laneText = (agentId: string) => + ({ + type: ContentTypes.TEXT, + text: `column ${agentId}`, + agentId, + groupId: 1, + }) as unknown as TMessageContentParts; render( , ); @@ -563,18 +573,21 @@ describe('ContentParts — activity phase state', () => { activity_count: 2, pending: false, } as unknown as TMessageContentParts; - const parallel = { - type: ContentTypes.TEXT, - text: 'lane result', - groupId: 1, - } as unknown as TMessageContentParts; + const lane = (agentId: string) => + ({ + type: ContentTypes.TEXT, + text: `lane result ${agentId}`, + agentId, + groupId: 1, + }) as unknown as TMessageContentParts; render( , diff --git a/client/src/components/Chat/Messages/Content/__tests__/ParallelContent.test.ts b/client/src/components/Chat/Messages/Content/__tests__/ParallelContent.test.ts index ec1b821bfb..95488955ed 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ParallelContent.test.ts +++ b/client/src/components/Chat/Messages/Content/__tests__/ParallelContent.test.ts @@ -3,43 +3,163 @@ import type { TMessageContentParts } from 'librechat-data-provider'; import { groupParallelContent, lastParallelColumnCursorIdx } from '../ParallelContent'; describe('groupParallelContent', () => { - test('reports absolute indices for a dense phase segment', () => { - const sequential = { + const sequentialPart = { + type: ContentTypes.TEXT, + text: 'before lanes', + } as unknown as TMessageContentParts; + + /** Two agents, so the group renders as columns at all — see the lane + * demotion suite below. */ + const lanePart = (agentId: string) => + ({ type: ContentTypes.TEXT, - text: 'before lanes', - } as unknown as TMessageContentParts; - const parallel = { - type: ContentTypes.TEXT, - text: 'lane result', + text: `${agentId} result`, groupId: 1, - agentId: 'agent-1', - } as unknown as TMessageContentParts; + agentId, + }) as unknown as TMessageContentParts; - const grouped = groupParallelContent([sequential, parallel], 4); + test('reports absolute indices for a dense phase segment', () => { + const first = lanePart('agent-1'); + const second = lanePart('agent-2'); - expect(grouped.sequentialParts).toEqual([{ part: sequential, idx: 4 }]); - expect(grouped.parallelSections[0]?.columns[0]?.parts).toEqual([{ part: parallel, idx: 5 }]); + const grouped = groupParallelContent([sequentialPart, first, second], 4); + + expect(grouped.sequentialParts).toEqual([{ part: sequentialPart, idx: 4 }]); + expect(grouped.parallelSections[0]?.columns[0]?.parts).toEqual([{ part: first, idx: 5 }]); + expect(grouped.parallelSections[0]?.columns[1]?.parts).toEqual([{ part: second, idx: 6 }]); }); test('preserves absolute indices for a compacted sparse phase segment', () => { - const sequential = { - type: ContentTypes.TEXT, - text: 'before lanes', - } as unknown as TMessageContentParts; - const parallel = { - type: ContentTypes.TEXT, - text: 'lane result', - groupId: 1, - agentId: 'agent-1', - } as unknown as TMessageContentParts; + const first = lanePart('agent-1'); + const second = lanePart('agent-2'); - const grouped = groupParallelContent([sequential, parallel], 0, [2, 10_000]); + const grouped = groupParallelContent([sequentialPart, first, second], 0, [2, 10_000, 10_001]); - expect(grouped.sequentialParts).toEqual([{ part: sequential, idx: 2 }]); - expect(grouped.parallelSections[0]?.columns[0]?.parts).toEqual([ - { part: parallel, idx: 10_000 }, + expect(grouped.sequentialParts).toEqual([{ part: sequentialPart, idx: 2 }]); + expect(grouped.parallelSections[0]?.columns[0]?.parts).toEqual([{ part: first, idx: 10_000 }]); + expect(grouped.parallelSections[0]?.columns[1]?.parts).toEqual([{ part: second, idx: 10_001 }]); + }); +}); + +describe('groupParallelContent — lane demotion', () => { + const lanePart = (agentId: string, text: string, groupId = 1): TMessageContentParts => + ({ type: ContentTypes.TEXT, text, agentId, groupId }) as unknown as TMessageContentParts; + + const plainPart = (text: string): TMessageContentParts => + ({ type: ContentTypes.TEXT, text }) as unknown as TMessageContentParts; + + it('renders a group backed by one agent sequentially', () => { + /** A multi-agent graph stamps a group id on every starting node, so an + * agent that merely has subagents available carries one on its own + * output. One column is not a comparison — a lane there would draw a + * second author header and opt the parts out of tool grouping. */ + const thinking = lanePart('agent_a', 'thinking'); + const answer = lanePart('agent_a', 'answer'); + + const grouped = groupParallelContent([thinking, answer]); + + expect(grouped.parallelSections).toEqual([]); + expect(grouped.sequentialParts).toEqual([ + { part: thinking, idx: 0 }, + { part: answer, idx: 1 }, ]); }); + + it('keeps columns once a second agent shares the group', () => { + const primary = lanePart('agent_a', 'primary answer'); + const added = lanePart('agent_b____1', 'added answer'); + + const grouped = groupParallelContent([primary, added]); + + expect(grouped.parallelSections).toHaveLength(1); + expect(grouped.parallelSections[0]?.columns.map((column) => column.agentId)).toEqual([ + 'agent_a', + 'agent_b____1', + ]); + expect(grouped.sequentialParts).toEqual([]); + }); + + it('restores transcript order after demoting a lone group', () => { + const preface = plainPart('preface'); + const lane = lanePart('agent_a', 'lane output'); + const closing = plainPart('closing'); + + const grouped = groupParallelContent([preface, lane, closing]); + + expect(grouped.sequentialParts.map(({ idx }) => idx)).toEqual([0, 1, 2]); + }); + + it('demotes a lone group beside a real one without dropping its parts', () => { + const primary = lanePart('agent_a', 'primary answer'); + const added = lanePart('agent_b____1', 'added answer'); + const lone = lanePart('agent_c', 'handoff output', 2); + + const grouped = groupParallelContent([primary, lone, added]); + + expect(grouped.parallelSections).toHaveLength(1); + expect(grouped.parallelSections[0]?.groupId).toBe(1); + expect(grouped.sequentialParts).toEqual([{ part: lone, idx: 1 }]); + }); + + it('demotes a group whose only second column is unattributed', () => { + const attributed = lanePart('agent_a', 'primary answer'); + const unattributed = { + type: ContentTypes.TEXT, + text: 'server sent no agent id', + groupId: 1, + } as unknown as TMessageContentParts; + + const grouped = groupParallelContent([attributed, unattributed]); + + expect(grouped.parallelSections).toEqual([]); + expect(grouped.sequentialParts.map(({ idx }) => idx)).toEqual([0, 1]); + }); + + it('demotes a group whose second column is only a handoff marker', () => { + const output = lanePart('agent_a', 'primary answer'); + const handoff = { + type: ContentTypes.AGENT_UPDATE, + [ContentTypes.AGENT_UPDATE]: { agentId: 'agent_b' }, + agentId: 'agent_b', + groupId: 1, + } as unknown as TMessageContentParts; + + const grouped = groupParallelContent([output, handoff]); + + expect(grouped.parallelSections).toEqual([]); + expect(grouped.sequentialParts.map(({ idx }) => idx)).toEqual([0, 1]); + }); + + it('keeps columns when a real dual run also carries a handoff marker', () => { + const primary = lanePart('agent_a', 'primary answer'); + const added = lanePart('agent_b____1', 'added answer'); + const handoff = { + type: ContentTypes.AGENT_UPDATE, + [ContentTypes.AGENT_UPDATE]: { agentId: 'agent_c' }, + agentId: 'agent_c', + groupId: 1, + } as unknown as TMessageContentParts; + + const grouped = groupParallelContent([primary, added, handoff]); + + expect(grouped.parallelSections).toHaveLength(1); + expect(grouped.sequentialParts).toEqual([]); + }); + + it('keeps a slice column when the message says the group has two agents', () => { + /** A phase marker can hand this slice one agent of a real two-agent group. + * Counting the slice alone would demote it and strip the attribution the + * sibling slice still shows, so the message-level verdict wins. */ + const primaryOnly = lanePart('agent_a', 'primary answer'); + + const grouped = groupParallelContent([primaryOnly], 0, undefined, new Set([1])); + + expect(grouped.parallelSections).toHaveLength(1); + expect(grouped.parallelSections[0]?.columns.map((column) => column.agentId)).toEqual([ + 'agent_a', + ]); + expect(grouped.sequentialParts).toEqual([]); + }); }); describe('lastParallelColumnCursorIdx', () => { diff --git a/client/src/components/Chat/Messages/MultiMessage.tsx b/client/src/components/Chat/Messages/MultiMessage.tsx index 5af59b7fa9..5a2d8aa67f 100644 --- a/client/src/components/Chat/Messages/MultiMessage.tsx +++ b/client/src/components/Chat/Messages/MultiMessage.tsx @@ -7,6 +7,7 @@ import type { TMessageProps } from '~/common'; import EventSubagentActivityGroup from '~/components/Chat/Subagents/EventSubagentActivityGroup'; import MessageContent from '~/components/Messages/MessageContent'; import { useRowMountWindow } from '~/hooks/Messages'; +import { hasParallelLanes } from '~/utils/lanes'; import MessageParts from './MessageParts'; import Message from './Message'; import store from '~/store'; @@ -199,8 +200,7 @@ function MultiMessage({ } const isEditingActivityAnchor = typeof currentEditId === 'string' && activityParentMessageIds.includes(currentEditId); - const hasParallelContent = - !message.isCreatedByUser && message.content?.some((part) => part?.groupId != null) === true; + const hasParallelContent = !message.isCreatedByUser && hasParallelLanes(message.content); /** * The child recursion is a sibling of the row (not rendered inside it), so a diff --git a/client/src/components/Chat/Messages/__tests__/MultiMessage.spec.tsx b/client/src/components/Chat/Messages/__tests__/MultiMessage.spec.tsx index 7933482fd8..d5bf5ce960 100644 --- a/client/src/components/Chat/Messages/__tests__/MultiMessage.spec.tsx +++ b/client/src/components/Chat/Messages/__tests__/MultiMessage.spec.tsx @@ -170,7 +170,11 @@ describe('MultiMessage sibling selection', () => { it('matches the wider layout of a parallel assistant response', () => { const assistant = { ...msg('assistant'), - content: [{ type: 'text', text: 'answer', groupId: 'parallel-group' }], + /** Two agents, so the response really does lay out columns. */ + content: [ + { type: 'text', text: 'primary', agentId: 'agent_a', groupId: 1 }, + { type: 'text', text: 'added', agentId: 'agent_b____1', groupId: 1 }, + ], } as unknown as TMessage; render( @@ -190,6 +194,35 @@ describe('MultiMessage sibling selection', () => { ); }); + it('keeps the standard width when a group is backed by one agent', () => { + /** A multi-agent graph stamps a group id on every starting node, so an + * ordinary agent that merely has subagents available carries one on its + * OWN output. Widening for columns that never arrive is the regression. */ + const assistant = { + ...msg('assistant'), + content: [ + { type: 'text', text: 'thinking', agentId: 'agent_a', groupId: 1 }, + { type: 'text', text: 'answer', agentId: 'agent_a', groupId: 1 }, + ], + } as unknown as TMessage; + + render( + + + , + ); + + expect(screen.getByTestId('event-subagent-activity')).toHaveAttribute( + 'data-has-parallel-content', + 'false', + ); + }); + it('renders assistant content containing an undefined streaming placeholder', () => { const assistant = { ...msg('assistant'), diff --git a/client/src/hooks/Messages/useContentMetadata.ts b/client/src/hooks/Messages/useContentMetadata.ts index 117ac97874..6387514e6a 100644 --- a/client/src/hooks/Messages/useContentMetadata.ts +++ b/client/src/hooks/Messages/useContentMetadata.ts @@ -1,14 +1,16 @@ import { useMemo } from 'react'; import type { TMessage } from 'librechat-data-provider'; +import { hasParallelLanes } from '~/utils/lanes'; export type ContentMetadataResult = { - /** Whether the message has parallel content (content with groupId) */ + /** Whether the message renders agent columns side by side */ hasParallelContent: boolean; }; /** - * Hook to check if a message has parallel content. - * Returns whether any content part has a groupId. + * Hook to check if a message has parallel content — content the renderer lays + * out as columns. Widens the message row, so it asks the same question the + * renderer does: a group id backed by a single agent is not parallel content. * * @param message - The message to check * @returns ContentMetadataResult with hasParallelContent boolean @@ -22,9 +24,6 @@ export default function useContentMetadata( return { hasParallelContent: false }; } - // Check if any content part has a groupId (TMessageContentParts now includes ContentMetadata) - const hasParallelContent = content.some((part) => part?.groupId != null); - - return { hasParallelContent }; + return { hasParallelContent: hasParallelLanes(content) }; }, [message?.content]); } diff --git a/client/src/utils/__tests__/activityLabels.spec.ts b/client/src/utils/__tests__/activityLabels.spec.ts index fd4be7b3fd..9c526ee26c 100644 --- a/client/src/utils/__tests__/activityLabels.spec.ts +++ b/client/src/utils/__tests__/activityLabels.spec.ts @@ -653,22 +653,48 @@ describe('groupActivityPhases — synthesized folds', () => { expect(foldOf(failed)?.labelPart.status).toBe('failed'); }); - it('stands down when the message carries parallel content', () => { - const parallel = { - ...(toolPart('t2') as object), - groupId: 'g1', - } as unknown as TMessageContentParts; + const inLane = (part: TMessageContentParts, agentId: string): TMessageContentParts => + ({ ...(part as object), agentId, groupId: 1 }) as unknown as TMessageContentParts; + it('stands down when the message carries parallel content', () => { expect( groupActivityPhases([ - toolPart('t1'), - childLabel('Read the config'), - parallel, - childLabel('Checked the callers'), + inLane(toolPart('t1'), 'agent_a'), + inLane(childLabel('Read the config'), 'agent_a'), + inLane(toolPart('t2'), 'agent_b'), + inLane(childLabel('Checked the callers'), 'agent_b'), ]), ).toBeUndefined(); }); + it('folds a labeled run whose group id is backed by one agent', () => { + /** A multi-agent graph stamps a group id on every starting node, so an + * agent that merely has subagents available carries one on its own + * output. It renders sequentially, so it folds like any other run. */ + const segments = groupActivityPhases([ + inLane(toolPart('t1'), 'agent_a'), + inLane(childLabel('Read the config'), 'agent_a'), + inLane(toolPart('t2'), 'agent_a'), + inLane(childLabel('Checked the callers'), 'agent_a'), + ]); + + expect(foldOf(segments)).toMatchObject({ synthesized: true, contentIndices: [0, 1, 2, 3] }); + }); + + it("takes the caller's lane scan instead of repeating it", () => { + /** `ContentParts` already scans the message for lanes; passing the answer + * keeps a streamed delta to one pass, and must decide identically. */ + const run = [ + inLane(toolPart('t1'), 'agent_a'), + inLane(childLabel('Read the config'), 'agent_a'), + inLane(toolPart('t2'), 'agent_a'), + inLane(childLabel('Checked the callers'), 'agent_a'), + ]; + + expect(foldOf(groupActivityPhases(run, new Set()))).toMatchObject({ synthesized: true }); + expect(groupActivityPhases(run, new Set([1]))).toBeUndefined(); + }); + it('never folds a span a server marker already claims', () => { const phase = labelPart({ activity_label: 'Reviewed the release paths', pending: false }); Object.assign(phase, { diff --git a/client/src/utils/__tests__/lanes.spec.ts b/client/src/utils/__tests__/lanes.spec.ts new file mode 100644 index 0000000000..4297d1084a --- /dev/null +++ b/client/src/utils/__tests__/lanes.spec.ts @@ -0,0 +1,112 @@ +import { ContentTypes } from 'librechat-data-provider'; +import type { TMessageContentParts } from 'librechat-data-provider'; +import { hasParallelLanes, laneAgentsByGroup, parallelLaneGroups } from '../lanes'; + +const lanePart = (agentId?: string, groupId?: number, text = 'output'): TMessageContentParts => + ({ + type: ContentTypes.TEXT, + text, + ...(agentId == null ? {} : { agentId }), + ...(groupId == null ? {} : { groupId }), + }) as unknown as TMessageContentParts; + +const placeholder = (agentId: string, groupId: number): TMessageContentParts => + ({ type: '', agentId, groupId }) as unknown as TMessageContentParts; + +describe('hasParallelLanes', () => { + it('is false without any group id', () => { + expect(hasParallelLanes([lanePart('agent_a'), lanePart('agent_a')])).toBe(false); + }); + + it('is false for a group backed by one agent', () => { + /** A multi-agent graph assigns a group id to every starting node, so an + * agent that merely has subagents available carries one on its OWN + * output — with no second column it is not parallel content. */ + expect(hasParallelLanes([lanePart('agent_a', 1), lanePart('agent_a', 1)])).toBe(false); + }); + + it('is true once a second agent shares the group', () => { + expect(hasParallelLanes([lanePart('agent_a', 1), lanePart('agent_b', 1)])).toBe(true); + }); + + it('counts placeholder columns, which a dual run seeds before any content', () => { + expect(hasParallelLanes([placeholder('agent_a', 1), placeholder('agent_b____1', 1)])).toBe( + true, + ); + }); + + it('shares one column across parts that carry no agent id', () => { + expect(hasParallelLanes([lanePart(undefined, 1), lanePart(undefined, 1)])).toBe(false); + }); + + it('ignores holes in a sparse in-run content array', () => { + const sparse: Array = []; + sparse[0] = lanePart('agent_a', 1); + sparse[4] = lanePart('agent_b', 1); + expect(hasParallelLanes(sparse)).toBe(true); + expect(laneAgentsByGroup(sparse).get(1)?.size).toBe(2); + }); + + it('separates lanes by group id', () => { + const lanes = laneAgentsByGroup([ + lanePart('agent_a', 1), + lanePart('agent_b', 1), + lanePart('agent_c', 2), + ]); + expect(lanes.get(1)?.size).toBe(2); + expect(lanes.get(2)?.size).toBe(1); + }); + + it('collects only the group ids that render as columns', () => { + const groups = parallelLaneGroups([ + lanePart('agent_a', 1), + lanePart('agent_b', 1), + lanePart('agent_c', 2), + ]); + expect([...groups]).toEqual([1]); + }); + + it('does not count a part with no agent id as a second lane', () => { + /** `agentId` and `groupId` are independently optional on a run step, so a + * group can hold a part the server never attributed. It shares the + * unattributed column, but it is not another agent to compare against. */ + const content = [lanePart('agent_a', 1), lanePart(undefined, 1)]; + expect(hasParallelLanes(content)).toBe(false); + expect([...parallelLaneGroups(content)]).toEqual([]); + expect(laneAgentsByGroup(content).get(1)?.size).toBe(2); + }); + + it('does not let a sequential handoff marker claim a lane', () => { + /** `useStepHandler` stamps an agent update with the CURRENT group id even + * when the destination's own run steps carry none, so the marker names a + * second agent inside a single-lane group. Counting it split one run into + * columns at the handoff. */ + const handoff = { + type: ContentTypes.AGENT_UPDATE, + [ContentTypes.AGENT_UPDATE]: { agentId: 'agent_b' }, + agentId: 'agent_b', + groupId: 1, + } as unknown as TMessageContentParts; + const content = [lanePart('agent_a', 1), handoff]; + + expect(hasParallelLanes(content)).toBe(false); + expect(laneAgentsByGroup(content).get(1)?.size).toBe(1); + }); + + it('scans a content array once, and a new array afresh', () => { + /** One message is scanned by `MultiMessage`, `useContentMetadata` and + * `ContentParts` in a single render pass. */ + const content = [lanePart('agent_a', 1), lanePart('agent_b', 1)]; + + expect(laneAgentsByGroup(content)).toBe(laneAgentsByGroup(content)); + expect(laneAgentsByGroup([...content])).not.toBe(laneAgentsByGroup(content)); + }); + + it('answers for a slice from the message-level groups it is given', () => { + /** The slice holds one agent of group 1; the message knows better. */ + const slice = [lanePart('agent_a', 1)]; + expect(hasParallelLanes(slice)).toBe(false); + expect(hasParallelLanes(slice, new Set([1]))).toBe(true); + expect(hasParallelLanes(slice, new Set([2]))).toBe(false); + }); +}); diff --git a/client/src/utils/activityLabels.ts b/client/src/utils/activityLabels.ts index 49d49d8d74..25b215dd93 100644 --- a/client/src/utils/activityLabels.ts +++ b/client/src/utils/activityLabels.ts @@ -1,5 +1,6 @@ import { Constants, ContentTypes } from 'librechat-data-provider'; import type { TMessage, TActivityLabelEvent, TMessageContentParts } from 'librechat-data-provider'; +import { hasParallelLanes } from '~/utils/lanes'; type ActivityLabelPart = Extract & { activity_label_type?: 'phase'; @@ -412,6 +413,7 @@ function synthesizeActivityFolds( */ export function groupActivityPhases( content: Array | undefined, + laneGroups?: ReadonlySet, ): ActivityPhaseSegment[] | undefined { if (!content) { return undefined; @@ -420,10 +422,14 @@ export function groupActivityPhases( /** Parallel columns lay their own activity out and are rendered by * `ParallelContentRenderer`, which a synthesized card would pull onto the * phase path. Server markers may still claim parallel spans; only the - * client-built folds stand down. */ - const foldable = !definedIndices.some( - (index) => (content[index] as { groupId?: string } | undefined)?.groupId != null, - ); + * client-built folds stand down. A group id alone is not enough: one that + * resolves to a single agent renders sequentially, so it folds like any + * other run. + * + * `laneGroups` is the caller's own lane scan over this same content, and + * content is rewritten on every streamed delta — taking the answer rather + * than repeating the scan keeps a render to one pass. */ + const foldable = laneGroups != null ? laneGroups.size === 0 : !hasParallelLanes(content); const completed = definedIndices .map((index) => ({ part: getActivityLabelPart(content[index]), index })) .filter( diff --git a/client/src/utils/lanes.ts b/client/src/utils/lanes.ts new file mode 100644 index 0000000000..78a83603a2 --- /dev/null +++ b/client/src/utils/lanes.ts @@ -0,0 +1,127 @@ +import { ContentTypes } from 'librechat-data-provider'; +import type { TMessageContentParts } from 'librechat-data-provider'; + +/** Column key for a lane part that carries no agent id of its own. */ +export const UNATTRIBUTED_LANE = 'unknown'; + +/** + * Lanes an agent actually claims. `agentId` and `groupId` are independently + * optional on a run step, so a group can hold a part with no agent of its + * own; it shares the unattributed column, but it is NOT a second agent — + * counting the sentinel would let one agent plus one metadata-less part + * masquerade as a comparison. + */ +export function attributedLaneCount(agents: ReadonlySet): number { + return agents.has(UNATTRIBUTED_LANE) ? agents.size - 1 : agents.size; +} + +/** + * An agent update is a transition marker, not lane content: it names the agent + * the run hands off TO. `useStepHandler` stamps it with the current group id + * even when the destination's own steps carry none, so counting it would read + * a sequential handoff as a second lane and split one run into columns. + */ +export function isLaneMarkerPart(part: TMessageContentParts | undefined): boolean { + return part?.type === ContentTypes.AGENT_UPDATE; +} + +/** + * Columns a lane group needs before it renders as columns. + * + * `groupId` marks content the server ran as one wave, but a wave is only ever + * shown as a side-by-side COMPARISON — the added-conversation feature seeds a + * placeholder per agent so both columns exist from the first render. A group + * that resolves to one agent has nothing to compare against, and rendering it + * as a lane costs more than a redundant border: lanes draw their own author + * header and branch control (restating the message's own sender) and they + * render raw parts, which opts those parts out of tool grouping, activity + * label headers and phase folds, while the message row widens for columns + * that never arrive. + * + * A lone group is not exotic. A multi-agent graph assigns a group id to every + * starting node, so an ordinary agent that merely has subagents available + * carries one on its OWN output. + */ +export const MIN_PARALLEL_LANES = 2; + +/** + * Distinct lane agents per group id. Mirrors the column derivation in + * `groupParallelContent`: a placeholder (empty `type`) establishes its agent's + * column without contributing content, and a part with no agent id of its own + * shares the single unattributed column. + */ +const laneAgentsCache = new WeakMap>>(); + +export function laneAgentsByGroup( + content: ReadonlyArray | undefined, +): ReadonlyMap> { + if (content == null) { + return new Map(); + } + /** One message is scanned by `MultiMessage`, `useContentMetadata` and + * `ContentParts` in a single render pass. Keying on the array itself + * collapses those to one traversal and is exactly as fresh as the render: + * every streamed delta rebuilds `content` (`[...(message.content || [])]` + * in `useStepHandler`), and an update that kept the identity would not + * re-render either. Entries die with the array they key. */ + const cached = laneAgentsCache.get(content); + if (cached != null) { + return cached; + } + const lanes = new Map>(); + content.forEach((part) => { + if (part?.groupId == null || isLaneMarkerPart(part)) { + return; + } + const agents = lanes.get(part.groupId) ?? new Set(); + agents.add(part.agentId ?? UNATTRIBUTED_LANE); + lanes.set(part.groupId, agents); + }); + laneAgentsCache.set(content, lanes); + return lanes; +} + +/** + * The group ids that render as columns, resolved over WHOLE message content. + * + * Lane cardinality is a property of the message, not of the slice in front of + * you: a phase marker can partition a real two-agent group so that one slice + * holds a single agent's parts. Counting that slice on its own would demote it + * and drop the per-agent attribution its sibling slice still shows, so every + * slice asks this set instead of recounting. + */ +export function parallelLaneGroups( + content: ReadonlyArray | undefined, +): Set { + const groups = new Set(); + for (const [groupId, agents] of laneAgentsByGroup(content)) { + if (attributedLaneCount(agents) >= MIN_PARALLEL_LANES) { + groups.add(groupId); + } + } + return groups; +} + +/** + * True when some group is backed by enough distinct agents to render as + * columns. The predicate every consumer of "does this message have parallel + * content" asks, so a lone group renders exactly like content with no group + * id at all. + * + * `laneGroups` carries the message-level answer into a phase slice; without it + * the content passed in is counted on its own. + */ +export function hasParallelLanes( + content: ReadonlyArray | undefined, + laneGroups?: ReadonlySet, +): boolean { + if (laneGroups != null) { + return content?.some((part) => part?.groupId != null && laneGroups.has(part.groupId)) === true; + } + for (const agents of laneAgentsByGroup(content).values()) { + if (attributedLaneCount(agents) >= MIN_PARALLEL_LANES) { + return true; + } + } + return false; +}