diff --git a/client/src/Providers/MediaContext.tsx b/client/src/Providers/MediaContext.tsx new file mode 100644 index 0000000000..be271b591c --- /dev/null +++ b/client/src/Providers/MediaContext.tsx @@ -0,0 +1,15 @@ +import { createContext, useContext } from 'react'; +import type { TAttachment } from 'librechat-data-provider'; + +type MediaContext = { + /** + * The turn's attachments, addressable by the filename a markdown image + * names them with. A model that writes `![DTI](5_dti.png)` is referring to + * a file the run produced, not to a path the browser can fetch, so the + * markdown renderer resolves the reference through this map. + */ + attachmentsByName?: ReadonlyMap; +}; + +export const MediaContext = createContext({}); +export const useMediaContext = () => useContext(MediaContext); diff --git a/client/src/Providers/index.ts b/client/src/Providers/index.ts index 2752e09178..76aad5b456 100644 --- a/client/src/Providers/index.ts +++ b/client/src/Providers/index.ts @@ -26,4 +26,5 @@ export * from './UploadModalContext'; export * from './ArtifactsContext'; export * from './PromptGroupsContext'; export * from './MessagesViewContext'; +export * from './MediaContext'; export { default as BadgeRowProvider } from './BadgeRowContext'; diff --git a/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx b/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx index 87ccc807d2..5f205e066e 100644 --- a/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx @@ -2,7 +2,7 @@ import { useId, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Button } from '@librechat/client'; import { ChevronDown } from 'lucide-react'; import { ContentTypes } from 'librechat-data-provider'; -import type { TMessageContentParts } from 'librechat-data-provider'; +import type { TAttachment, TMessageContentParts } from 'librechat-data-provider'; import type { CSSProperties, ReactNode } from 'react'; import { useExpandCollapse, @@ -12,7 +12,7 @@ import { } from '~/hooks'; import useSmoothStreaming from '~/hooks/Messages/useSmoothStreaming'; import { getActivityLabelText } from '~/utils/activityLabels'; -import { EmptyText } from './Parts'; +import { AttachmentGroup, EmptyText } from './Parts'; import Container from './Container'; import { cn } from '~/utils'; @@ -53,6 +53,7 @@ export default function ActivityPhaseGroup({ labelPart, children, hasContent, + attachments, showCursor = false, animateEntrance = false, hasPendingApproval = false, @@ -60,6 +61,13 @@ export default function ActivityPhaseGroup({ labelPart: ActivityPhasePart; children: ReactNode; hasContent: boolean; + /** Files the phase produced, lifted out of the fold by the parent renderer. + * A summary card is collapsed the moment it settles, so anything rendered + * inside it is, for most readers, not rendered at all — and a chart the run + * spent its turn producing is exactly what the reader came for. They ride + * their own row under the header instead, where the fold cannot take + * them. */ + attachments?: TAttachment[]; showCursor?: boolean; animateEntrance?: boolean; hasPendingApproval?: boolean; @@ -159,8 +167,17 @@ export default function ActivityPhaseGroup({ ) : null; + const media = + attachments != null && attachments.length > 0 ? ( + + ) : null; if (!label) { - return <>{children}; + return ( + <> + {children} + {media} + + ); } const group = !hasContent ? (
{group} + {media} {cursor} ); diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index af665dbf99..acf419dd9e 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -12,6 +12,8 @@ import type { ActivityPhaseSegment } from '~/utils/activityLabels'; import { mapAttachments, getPartKeyIndex, + attachmentRenderKey, + buildAttachmentsByName, filterAttachmentsForPart, groupSequentialToolCalls, } from '~/utils'; @@ -22,8 +24,8 @@ import { } from '~/utils/activityLabels'; import WorkspaceChanges, { partitionWorkspaceChanges } from './Parts/WorkspaceChanges'; import { ParallelContentRenderer, type PartWithIndex } from './ParallelContent'; +import { MediaContext, MessageContext, SearchContext } from '~/Providers'; import MemoryArtifacts, { hasMemoryArtifacts } from './MemoryArtifacts'; -import { MessageContext, SearchContext } from '~/Providers'; import PendingSkillCall from './Parts/PendingSkillCall'; import ActivityPhaseGroup from './ActivityPhaseGroup'; import { hasPendingApprovalInPart } from '~/utils'; @@ -215,6 +217,10 @@ type ContentPartsProps = { | undefined; /** Internal recursion guard for nested phase segments. */ nestedActivityPhase?: boolean; + /** Internal signal that the parent phase card lifted this segment's + * attachments into its own media row — the parts below must not render + * them a second time inside the fold. */ + hideAttachments?: boolean; /** Internal signal that this segment renders inside a completed phase card. */ withinActivityPhase?: boolean; /** Internal signal that a phase card already carries this message's @@ -264,6 +270,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ nestedActivityPhase = false, withinActivityPhase = false, cursorOwnedElsewhere = false, + hideAttachments = false, workspaceAttachmentsPartitioned = false, contentIndexOffset = 0, contentIndices, @@ -300,6 +307,16 @@ const ContentPartsBody = memo(function ContentPartsBody({ } return owners; }, [toolCallStepOwnersById, content]); + const attachmentsForPart = useCallback( + (part: TMessageContentParts): TAttachment[] | undefined => + filterAttachmentsForPart( + attachmentMap[getToolCallId(part)], + getPartAgentId(part), + getPartStepId(part), + getSiblingStepIds(part, resolvedToolCallStepOwners), + ), + [attachmentMap, resolvedToolCallStepOwners], + ); const effectiveIsSubmitting = isLatestMessage ? isSubmitting : false; const localToolGroupExpansionRef = useRef(new Map()); const expansionState = toolGroupExpansionState ?? localToolGroupExpansionRef.current; @@ -330,6 +347,45 @@ const ContentPartsBody = memo(function ContentPartsBody({ () => (nestedActivityPhase ? undefined : groupActivityPhases(content)), [nestedActivityPhase, content], ); + /** Every file a phase's parts produced, in transcript order, deduplicated + * across parts that share a tool call. */ + const collectPhaseAttachments = useCallback( + (parts: ReadonlyArray): TAttachment[] | undefined => { + const collected: TAttachment[] = []; + /** Position of each identity already collected, so a repeat REPLACES it + * in place. A tool that writes the same file twice emits two records + * and the later one carries the fresher lifecycle fields — status, + * preview text, generated contents — which is why the attachment + * renderer gives duplicates distinct keys and lets the latest win. + * Hoisting makes this row the only surface for those files, so keeping + * the first-seen record would pin a stale one on screen. */ + const positionOf = new Map(); + for (const part of parts) { + if (part == null) { + continue; + } + for (const attachment of attachmentsForPart(part) ?? []) { + /** No identity means nothing to compare against — two rows that + * cannot be told apart are two rows, so both are kept rather than + * silently collapsed onto whichever arrived first. */ + const identity = attachmentRenderKey(attachment); + if (identity == null) { + collected.push(attachment); + continue; + } + const at = positionOf.get(identity); + if (at != null) { + collected[at] = attachment; + continue; + } + positionOf.set(identity, collected.length); + collected.push(attachment); + } + } + return collected.length > 0 ? collected : undefined; + }, + [attachmentsForPart], + ); const completedPhaseKeys = useMemo(() => { const indices = new Set(); const labels = new Map(); @@ -478,27 +534,23 @@ const ContentPartsBody = memo(function ContentPartsBody({ isCreatedByUser={isCreatedByUser} nextType={content?.[localIdx + 1]?.type} isSubmitting={effectiveIsSubmitting} - partAttachments={filterAttachmentsForPart( - attachmentMap[getToolCallId(part)], - getPartAgentId(part), - getPartStepId(part), - getSiblingStepIds(part, resolvedToolCallStepOwners), - )} + partAttachments={attachmentsForPart(part)} + hideAttachments={hideAttachments} /> ); }, [ - attachmentMap, + attachmentsForPart, content, contentIndexOffset, localIndexByAbsolute, conversationId, effectiveIsSubmitting, + hideAttachments, isCreatedByUser, isLast, isLatestMessage, messageId, - resolvedToolCallStepOwners, ], ); @@ -518,19 +570,14 @@ const ContentPartsBody = memo(function ContentPartsBody({ isCreatedByUser={isCreatedByUser} nextType={content?.[localIdx + 1]?.type} isSubmitting={effectiveIsSubmitting} - partAttachments={filterAttachmentsForPart( - attachmentMap[getToolCallId(part)], - getPartAgentId(part), - getPartStepId(part), - getSiblingStepIds(part, resolvedToolCallStepOwners), - )} + partAttachments={attachmentsForPart(part)} hideAttachments onToolExpand={onToolExpand} /> ); }, [ - attachmentMap, + attachmentsForPart, content, contentIndexOffset, localIndexByAbsolute, @@ -540,7 +587,6 @@ const ContentPartsBody = memo(function ContentPartsBody({ isLast, isLatestMessage, messageId, - resolvedToolCallStepOwners, ], ); @@ -607,23 +653,19 @@ const ContentPartsBody = memo(function ContentPartsBody({ * so preserve the first group's historic stable key and distinguish * later occurrences by sequence rather than a shifting content index. */ const groupId = occurrence === 1 ? baseGroupId : `${baseGroupId}:occurrence:${occurrence}`; - const groupAttachments = group.parts.flatMap( - ({ part }) => - filterAttachmentsForPart( - attachmentMap[getToolCallId(part)], - getPartAgentId(part), - getPartStepId(part), - getSiblingStepIds(part, resolvedToolCallStepOwners), - ) ?? [], - ); + /** Hoisted a level higher when a phase card owns the media row, so the + * same file is not offered by both the block and the card. */ + const groupAttachments = hideAttachments + ? undefined + : group.parts.flatMap(({ part }) => attachmentsForPart(part) ?? []); return { ...group, groupId, groupAttachments }; }); }, [ sequentialParts, - attachmentMap, + attachmentsForPart, fallbackScope, + hideAttachments, resolvedToolGroupOccurrences, - resolvedToolCallStepOwners, ]); /** The re-attribution node for a part resuming after a steer block, shared @@ -739,6 +781,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ key: string, withinPhase = false, ownsCursor = false, + hoisted = false, ) => { return ( part != null && hasPendingApprovalInPart(part), ); - /** `ToolCallGroup` hoists `groupAttachments` OUTSIDE its own panel - * precisely so a generated image or file survives collapsing the - * group. Folding that group into a card puts the hoist back - * inside a disclosure and the output vanishes — and these never - * appear in `segment.content`, so the visible-part allowlist - * cannot catch them. */ - const hasSpanAttachments = segment.content.some( - (part) => - part != null && - (filterAttachmentsForPart( - attachmentMap[getToolCallId(part)], - getPartAgentId(part), - getPartStepId(part), - getSiblingStepIds(part, resolvedToolCallStepOwners), - )?.length ?? 0) > 0, - ); + /** The span's files, lifted out of the fold into their own row + * under the header. + * + * This is why a span carrying attachments no longer has to skip + * folding. `ToolCallGroup` hoists `groupAttachments` outside its + * own panel so generated output survives collapsing the group, + * and folding that group into a card used to put the hoist back + * inside a disclosure — so a synthesized span with any output + * rendered unfolded rather than swallow it. Hoisting one level + * further makes the card safe for exactly that content, and + * agent runs that produce files are the ones with the longest + * lists to fold. */ + const phaseAttachments = collectPhaseAttachments(segment.content); /** A fold summarizes work that is DONE. An unresolved approval is * the run asking the reader for something and blocking until it * gets it — never put that behind a disclosure they have to find. @@ -845,8 +886,8 @@ const ContentPartsBody = memo(function ContentPartsBody({ * resolves), but a subagent nested in an already-labeled group * can raise one long after its parent's label filled. The span * renders exactly as it would without the feature until it - * clears, then folds. The same escape covers hoisted tool output. */ - if (synthesized && (hasPendingApproval || hasSpanAttachments)) { + * clears, then folds. */ + if (synthesized && hasPendingApproval) { return renderSegment( segment.content, absoluteIndexAt(segment.startIndex), @@ -859,6 +900,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ key={`activity-phase-${cardKey}`} labelPart={segment.labelPart} hasContent={segment.hasContent} + attachments={phaseAttachments} hasPendingApproval={hasPendingApproval} animateEntrance={ /** Never for a synthesized card. The entrance mounts a card @@ -891,6 +933,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ `phase-content-${cardKey}`, true, ownsCursor, + true, )} ); @@ -1015,7 +1058,27 @@ const ContentPartsBody = memo(function ContentPartsBody({ }); const ContentParts = memo(function ContentParts(props: ContentPartsProps) { - return ; + const { attachments } = props; + /** Published once for the whole message so every markdown block below — + * including the ones nested inside phase cards — resolves a bare + * `![DTI](5_dti.png)` against the files this turn actually produced. + * + * `buildAttachmentsByName` indexes only what an `` can display, so a + * `![report](report.csv)` keeps its download chip instead of resolving to a + * picture element that shows nothing. + * + * Memoized in two steps on purpose. `useAttachments` hands back a fresh + * `[]` whenever its inputs move, and a turn with no files is the common + * case — indexing through the shared empty map keeps the context value + * itself referentially stable there, so a churning array cannot push a new + * value at every markdown block in the message. */ + const attachmentsByName = useMemo(() => buildAttachmentsByName(attachments), [attachments]); + const media = useMemo(() => ({ attachmentsByName }), [attachmentsByName]); + return ( + + + + ); }); export default ContentParts; diff --git a/client/src/components/Chat/Messages/Content/Image.tsx b/client/src/components/Chat/Messages/Content/Image.tsx index 4289a5561a..7e44b2e828 100644 --- a/client/src/components/Chat/Messages/Content/Image.tsx +++ b/client/src/components/Chat/Messages/Content/Image.tsx @@ -1,8 +1,8 @@ import React, { useState, useRef, useMemo, useEffect } from 'react'; import { Skeleton } from '@librechat/client'; import { apiBaseUrl } from 'librechat-data-provider'; +import { cn, toAbsoluteFilePath } from '~/utils'; import DialogImage from './DialogImage'; -import { cn } from '~/utils'; /** Max display height for chat images (Tailwind JIT class) */ export const IMAGE_MAX_H = 'max-h-[45vh]' as const; @@ -48,21 +48,10 @@ const Image = ({ const [isOpen, setIsOpen] = useState(false); const triggerRef = useRef(null); - const absoluteImageUrl = useMemo(() => { - if (!imagePath) return imagePath; - - if (imagePath.startsWith('http') || imagePath.startsWith('data:')) { - return imagePath; - } - - // Root-relative server paths (`/images/...` static, `/api/share/...` share - // routes) are resolved against the API base so they load under a subpath. - if (imagePath.startsWith('/images/') || imagePath.startsWith('/api/')) { - return `${apiBaseUrl()}${imagePath}`; - } - - return imagePath; - }, [imagePath]); + /** Root-relative server paths (`/images/...` static, `/api/...` downloads and + * share routes) are resolved against the API base so they load under a + * subpath deployment. */ + const absoluteImageUrl = useMemo(() => toAbsoluteFilePath(imagePath, apiBaseUrl()), [imagePath]); const downloadImage = async () => { try { diff --git a/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx b/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx index 8a2af906e7..a891014955 100644 --- a/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx +++ b/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx @@ -2,12 +2,17 @@ import React, { memo, useMemo, useRef, useEffect } from 'react'; import { useRecoilValue } from 'recoil'; import { useToastContext } from '@librechat/client'; import { PermissionTypes, Permissions, apiBaseUrl } from 'librechat-data-provider'; +import { + handleDoubleClick, + triggerDownload, + resolveInlineMedia, + toAbsoluteFilePath, +} from '~/utils'; import Mermaid, { MermaidErrorBoundary } from '~/components/Messages/Content/Mermaid'; +import { useCodeBlockContext, useMediaContext } from '~/Providers'; import CodeBlock from '~/components/Messages/Content/CodeBlock'; -import { handleDoubleClick, triggerDownload } from '~/utils'; import useHasAccess from '~/hooks/Roles/useHasAccess'; import { useFileDownload } from '~/data-provider'; -import { useCodeBlockContext } from '~/Providers'; import { useLocalize } from '~/hooks'; import store from '~/store'; @@ -219,19 +224,17 @@ export const img: React.ElementType = memo(function MarkdownImage({ }: TImageProps) { // Get the base URL from the API endpoints const baseURL = apiBaseUrl(); + /** A model writing `![DTI](5_dti.png)` is naming a file its run produced, + * not a path the browser can fetch. Resolving the reference against the + * turn's attachments is what turns those into the chart instead of a + * broken-image glyph; an unmatched source keeps its original behavior. */ + const { attachmentsByName } = useMediaContext(); - // If src starts with /images/, prepend the base URL const fixedSrc = useMemo(() => { if (!src) return src; - - // If it's already an absolute URL or doesn't start with /images/, return as is - if (src.startsWith('http') || src.startsWith('data:') || !src.startsWith('/images/')) { - return src; - } - - // Prepend base URL to the image path - return `${baseURL}${src}`; - }, [src, baseURL]); + const resolved = resolveInlineMedia(src, attachmentsByName)?.filepath ?? src; + return toAbsoluteFilePath(resolved, baseURL); + }, [src, baseURL, attachmentsByName]); return {alt}; }); diff --git a/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx b/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx index 08d91a43b5..ee90039c7f 100644 --- a/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx +++ b/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx @@ -4,7 +4,7 @@ import supersub from 'remark-supersub'; import ReactMarkdown from 'react-markdown'; import rehypeHighlight from 'rehype-highlight'; import type { PluggableList } from 'unified'; -import { code, codeNoExecution, a, p, table } from './MarkdownComponents'; +import { code, codeNoExecution, a, p, img, table } from './MarkdownComponents'; import { langSubset, remarkApproxTilde } from '~/utils'; import { CodeBlockProvider } from '~/Providers'; @@ -73,6 +73,10 @@ class MarkdownErrorBoundary extends React.Component< code: codeExecution ? code : codeNoExecution, a, p, + /** The recovery path is exactly where a generated reference + * must still resolve; the default `img` would send + * `![chart](chart.png)` to a page-relative URL. */ + img, table, } as { [nodeType: string]: React.ElementType; 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 b7ee48d449..cbae29dd16 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 @@ -1,7 +1,7 @@ import React from 'react'; import { RecoilRoot } from 'recoil'; import { ContentTypes } from 'librechat-data-provider'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, within } from '@testing-library/react'; import type { TAttachment, TMessageContentParts } from 'librechat-data-provider'; import ContentParts from '../ContentParts'; @@ -71,7 +71,11 @@ jest.mock('@librechat/client', () => ({ jest.mock('../Parts', () => ({ AttachmentGroup: ({ attachments }: { attachments?: TAttachment[] }) => ( -
+
a.filepath).join(',')} + /> ), ExecuteCode: () =>
, ImageGen: () =>
, @@ -713,18 +717,26 @@ describe('ContentParts — synthesized activity folds', () => { ); }); - it('refuses to fold a span whose tool output is hoisted outside its group', () => { - /** `ToolCallGroup` renders `groupAttachments` outside its own panel so a - * generated image survives collapsing the group. A fold would put that - * hoist back inside a disclosure. */ + it('folds a span whose tool output is hoisted, carrying the output outside', () => { + /** This span used to refuse to fold: `ToolCallGroup` renders + * `groupAttachments` outside its own panel so a generated image survives + * collapsing the group, and folding put that hoist back inside a + * disclosure. The card now lifts those files one level further, into its + * own row outside the fold, so the span folds like any other — which + * matters because runs that produce files are the ones with the longest + * lists to fold. */ renderContentParts({ ...baseProps, content: labeledRun(), attachments: [imageAttachment('t2', 'chart.png')], }); - expect(screen.queryByTestId('activity-phase-panel')).toBeNull(); - expect(screen.getByRole('button', { name: FIRST })).toBeInTheDocument(); + expect(screen.getByTestId('activity-phase-panel')).toBeInTheDocument(); + const row = screen.getByTestId('attachment-group'); + expect(row.getAttribute('data-count')).toBe('1'); + expect( + within(screen.getByTestId('activity-phase-panel')).queryByTestId('attachment-group'), + ).toBeNull(); }); it('keeps the reader’s card open when the message takes its server id', () => { @@ -824,3 +836,81 @@ describe('ContentParts — synthesized activity folds', () => { expect(screen.queryByTestId('activity-phase-panel')).toBeNull(); }); }); + +describe('ContentParts integration: phase media row', () => { + const baseProps = { + messageId: 'msg1', + isCreatedByUser: false, + isLast: true, + isSubmitting: false, + isLatestMessage: true, + }; + + /** Two labeled calls the server has claimed as one phase, plus whatever the + * answer says afterwards. */ + const phaseContent = (answer?: string): TMessageContentParts[] => [ + makeMcpToolCall('t1'), + makeMcpToolCall('t2'), + makePhasePart(0, 2, 'Generated five mortgage charts'), + ...(answer == null ? [] : [makeTextPart(answer)]), + ]; + const phaseAttachments = [imageAttachment('t1', 'a.png'), imageAttachment('t2', 'b.png')]; + + it("lifts a phase's files into a media row the fold cannot hide", () => { + renderContentParts({ ...baseProps, content: phaseContent(), attachments: phaseAttachments }); + + const rows = screen.getAllByTestId('attachment-group'); + expect(rows).toHaveLength(1); + expect(rows[0].getAttribute('data-count')).toBe('2'); + // The row is a sibling of the collapsed card, not a child of its panel. + expect( + within(screen.getByTestId('activity-phase-panel')).queryByTestId('attachment-group'), + ).toBeNull(); + }); + + it('still shows one row once the reader expands the phase', () => { + renderContentParts({ ...baseProps, content: phaseContent(), attachments: phaseAttachments }); + + fireEvent.click(screen.getByRole('button', { name: 'Generated five mortgage charts' })); + + const rows = screen.getAllByTestId('attachment-group'); + expect(rows).toHaveLength(1); + expect(rows[0].getAttribute('data-count')).toBe('2'); + }); + + it('lifts the file of a lone tool call too, not just a grouped batch', () => { + // One call never forms a ToolCallGroup, so this exercises Part -> ToolCall's + // own `hideAttachments` rather than the group-level hoist. + const content = [makeMcpToolCall('t1'), makePhasePart(0, 1, 'Rendered the chart')]; + + renderContentParts({ ...baseProps, content, attachments: [imageAttachment('t1', 'a.png')] }); + + const rows = screen.getAllByTestId('attachment-group'); + expect(rows).toHaveLength(1); + expect(rows[0].getAttribute('data-count')).toBe('1'); + expect( + within(screen.getByTestId('activity-phase-panel')).queryByTestId('attachment-group'), + ).toBeNull(); + }); + + it('shows the latest record when a tool writes the same file twice', () => { + // Both records name one stored file; the later carries the fresher + // lifecycle fields. Hoisting makes this row the only surface for it, so a + // first-seen dedup would pin the stale record on screen. + const stale = { ...imageAttachment('t1', 'a.png'), file_id: 'f1' } as TAttachment; + const fresh = { ...stale, filepath: '/files/a.png?v=2' } as TAttachment; + const content = [makeMcpToolCall('t1'), makePhasePart(0, 1, 'Redrew the chart')]; + + renderContentParts({ ...baseProps, content, attachments: [stale, fresh] }); + + const row = screen.getByTestId('attachment-group'); + expect(row.getAttribute('data-count')).toBe('1'); + expect(row.getAttribute('data-paths')).toBe('/files/a.png?v=2'); + }); + + it('renders no row for a phase that produced nothing', () => { + renderContentParts({ ...baseProps, content: phaseContent() }); + + expect(screen.queryByTestId('attachment-group')).toBeNull(); + }); +}); 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 4930e7d8c6..08d1ec900c 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx @@ -12,6 +12,9 @@ jest.mock('~/utils', () => ({ groupSequentialToolCalls: jest.fn(), hasPendingApprovalInPart: jest.requireActual('~/utils/groupToolCalls').hasPendingApprovalInPart, getPartKeyIndex: jest.requireActual('~/utils/messages').getPartKeyIndex, + /** Real implementations: the media helpers are pure and drive the phase + * card's attachment row, so stubbing them would make that path inert here. */ + ...jest.requireActual('~/utils/media'), })); jest.mock('~/Providers', () => { @@ -33,6 +36,10 @@ jest.mock('~/Providers', () => { * the component's own catch swallowed it, so the sources path was dead here * while the suite still passed. */ useSearchContext: () => ({ searchResults: undefined }), + MediaContext: { + Provider: ({ children }: { children: React.ReactNode }) => <>{children}, + }, + useMediaContext: () => ({ attachmentsByName: undefined }), }; }); @@ -40,6 +47,9 @@ jest.mock('../Parts', () => ({ EmptyText: ({ underHeaderIcon }: { underHeaderIcon?: boolean }) => (
), + AttachmentGroup: ({ attachments }: { attachments?: unknown[] }) => ( +
+ ), AgentUpdate: ({ currentAgentId }: { currentAgentId: string }) => (
), diff --git a/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx index e7e0b99f1e..cef7093651 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx @@ -8,6 +8,9 @@ jest.mock('~/utils', () => ({ .flat(Infinity) .filter((c): c is string => typeof c === 'string' && c.length > 0) .join(' '), + /** Real implementation: URL resolution is what the cases below assert. */ + toAbsoluteFilePath: + jest.requireActual('~/utils/media').toAbsoluteFilePath, })); jest.mock('librechat-data-provider', () => ({ diff --git a/client/src/components/Chat/Messages/Content/__tests__/MarkdownImage.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/MarkdownImage.test.tsx new file mode 100644 index 0000000000..38ad6551ee --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/MarkdownImage.test.tsx @@ -0,0 +1,96 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import type { TAttachment } from 'librechat-data-provider'; +import { img as MarkdownImage } from '../MarkdownComponents'; +import { MediaContext } from '~/Providers/MediaContext'; +import { buildAttachmentsByName } from '~/utils/media'; + +jest.mock('librechat-data-provider', () => ({ + ...jest.requireActual('librechat-data-provider'), + apiBaseUrl: () => 'https://chat.example.com', +})); + +const chart: TAttachment = { + filename: '5_dti.png', + filepath: '/api/files/code/download/sess/abc/5_dti.png', + conversationId: 'c1', +} as unknown as TAttachment; + +const renderImage = (src: string, attachments?: TAttachment[]) => { + const { container } = render( + + + , + ); + return container.querySelector('img'); +}; + +describe('markdown img', () => { + const served = `https://chat.example.com${chart.filepath}`; + + it('resolves a filename the run produced to the real file', () => { + expect(renderImage('5_dti.png', [chart])).toHaveAttribute('src', served); + }); + + it('resolves the bare sandbox path the model tends to write', () => { + expect(renderImage('/mnt/data/5_dti.png', [chart])).toHaveAttribute('src', served); + }); + + it('leaves a sandbox: scheme unresolved', () => { + // react-markdown's default urlTransform allows only http/https/irc/mailto/ + // xmpp, so this source is blanked before `img` is called at all. Resolving + // it here would only be reachable from a test that skips the pipeline. + expect(renderImage('sandbox:/mnt/data/5_dti.png', [chart])).toHaveAttribute( + 'src', + 'sandbox:/mnt/data/5_dti.png', + ); + }); + + it('keeps an explicitly addressed server path over a same-named attachment', () => { + // The author addressed one file. Resolving by basename would display the + // unrelated attachment instead. + const other = { ...chart, filepath: '/api/files/other/5_dti.png' } as TAttachment; + expect(renderImage('/api/files/explicit/5_dti.png', [other])).toHaveAttribute( + 'src', + 'https://chat.example.com/api/files/explicit/5_dti.png', + ); + }); + + it('leaves an unmatched reference exactly as it was', () => { + expect(renderImage('missing.png', [chart])).toHaveAttribute('src', 'missing.png'); + }); + + it('leaves an absolute URL alone', () => { + const src = 'https://example.com/5_dti.png'; + expect(renderImage(src, [chart])).toHaveAttribute('src', src); + }); + + it('still prefixes the API base onto a bare /images/ path', () => { + expect(renderImage('/images/user/pic.png')).toHaveAttribute( + 'src', + 'https://chat.example.com/images/user/pic.png', + ); + }); + + it('prefixes the API base onto a resolved /api/ attachment', () => { + // The default filepath shape for code-execution artifacts. Without the + // prefix this 404s on any subpath deployment. + expect(renderImage('5_dti.png', [chart])).toHaveAttribute( + 'src', + `https://chat.example.com${chart.filepath}`, + ); + }); + + it('prefixes the API base onto a resolved /images/ attachment', () => { + const upload = { ...chart, filepath: '/images/user/5_dti.png' } as TAttachment; + expect(renderImage('5_dti.png', [upload])).toHaveAttribute( + 'src', + 'https://chat.example.com/images/user/5_dti.png', + ); + }); + + it('renders unchanged with no media context at all', () => { + const { container } = render(); + expect(container.querySelector('img')).toHaveAttribute('src', '5_dti.png'); + }); +}); diff --git a/client/src/hooks/Messages/useAttachments.ts b/client/src/hooks/Messages/useAttachments.ts index 5cfe7e1baf..0b5fe881d9 100644 --- a/client/src/hooks/Messages/useAttachments.ts +++ b/client/src/hooks/Messages/useAttachments.ts @@ -2,6 +2,7 @@ import { useMemo } from 'react'; import { useRecoilValue } from 'recoil'; import type { TAttachment, TFile } from 'librechat-data-provider'; import { useSearchResultsByTurn } from './useSearchResultsByTurn'; +import { attachmentIdentity } from '~/utils/media'; import store from '~/store'; function fileKeyOf(attachment: TAttachment): string | undefined { @@ -18,29 +19,12 @@ function agentIdOf(attachment: TAttachment): string | undefined { } /** - * Stable identity for merging DB and live attachments: `file_id ?? filepath` - * scoped by `toolCallId` and `agentId` (sibling code calls can share a - * claimed file_id for the same filename, and handoff agents can repeat - * provider tool ids — each combination anchors its own card), else - * `type:toolCallId` for unkeyed tool artifacts like file_search citations. - * Undefined = no stable identity. + * Stable identity for merging DB and live attachments. Shared with the + * message renderer, which dedupes the same rows on the same key — two + * definitions of what makes an attachment "the same one" is exactly the drift + * that puts a file on screen twice in one surface and not at all in another. */ -function attachmentKey(attachment: TAttachment): string | undefined { - const { type } = attachment as { type?: string }; - const toolCallId = toolCallIdOf(attachment); - const fileKey = fileKeyOf(attachment); - if (fileKey) { - if (toolCallId == null) { - return fileKey; - } - const agentId = agentIdOf(attachment); - return agentId ? `${fileKey}::${toolCallId}::${agentId}` : `${fileKey}::${toolCallId}`; - } - if (type != null && toolCallId != null) { - return `${type}:${toolCallId}`; - } - return undefined; -} +const attachmentKey = attachmentIdentity; /** * Wildcard-tolerant match for the lifecycle overlay: a missing `toolCallId` diff --git a/client/src/utils/__tests__/media.spec.ts b/client/src/utils/__tests__/media.spec.ts new file mode 100644 index 0000000000..279e840a9f --- /dev/null +++ b/client/src/utils/__tests__/media.spec.ts @@ -0,0 +1,268 @@ +import type { TAttachment } from 'librechat-data-provider'; +import { + attachmentIdentity, + attachmentRenderKey, + buildAttachmentsByName, + resolveInlineMedia, + toAbsoluteFilePath, +} from '~/utils/media'; + +const attachment = (overrides: Partial = {}): TAttachment => + ({ + filename: '5_dti.png', + filepath: '/api/files/code/download/sess/abc/5_dti.png', + conversationId: 'c1', + ...overrides, + }) as unknown as TAttachment; + +describe('buildAttachmentsByName', () => { + it('indexes by the leaf filename, case-insensitively', () => { + const file = attachment({ filename: 'charts/5_DTI.png' }); + const byName = buildAttachmentsByName([file]); + expect(byName.get('5_dti.png')).toBe(file); + }); + + it('keeps resolving when one stored file surfaces under two calls', () => { + // Inherited across steps, or a regeneration that updated the record in + // place. One file, so nothing is ambiguous — and the later record wins, + // since that is the one carrying the fresher lifecycle fields. + const first = attachment({ file_id: 'f1', toolCallId: 't1' } as Partial); + const again = attachment({ file_id: 'f1', toolCallId: 't2' } as Partial); + expect(buildAttachmentsByName([first, again]).get('5_dti.png')).toBe(again); + }); + + it.each([ + ['two different agents', 'a1', 'a2'], + ['two sibling calls by one agent', 'a1', 'a1'], + ['legacy rows naming no agent', undefined, undefined], + ])('refuses a basename two stored files claim — %s', (_label, left, right) => { + // A regeneration and two siblings that both wrote `output.png` are + // indistinguishable from the metadata, so picking either is a coin flip + // that renders one file under the other's caption. + const a = attachment({ filepath: '/files/left.png', toolCallId: 't1', agentId: left }); + const b = attachment({ filepath: '/files/right.png', toolCallId: 't2', agentId: right }); + expect(buildAttachmentsByName([a, b]).has('5_dti.png')).toBe(false); + }); + + it('keeps a name ambiguous once a third entry arrives', () => { + const left = attachment({ filepath: '/files/left.png' }); + const right = attachment({ filepath: '/files/right.png' }); + const later = attachment({ filepath: '/files/later.png' }); + expect(buildAttachmentsByName([left, right, later]).has('5_dti.png')).toBe(false); + }); + + it('does not let one collision poison an unrelated name', () => { + const left = attachment({ filepath: '/files/left.png', agentId: 'a1' }); + const right = attachment({ filepath: '/files/right.png', agentId: 'a2' }); + const other = attachment({ filename: 'other.png', filepath: '/files/o.png', agentId: 'a3' }); + const byName = buildAttachmentsByName([left, right, other]); + expect(byName.has('5_dti.png')).toBe(false); + expect(byName.get('other.png')).toBe(other); + }); + + it('skips a file no can display', () => { + // An `` aimed at a CSV renders nothing; the file keeps its download + // chip instead. + const csv = attachment({ filename: 'report.csv', filepath: '/api/files/report.csv' }); + expect(buildAttachmentsByName([csv]).size).toBe(0); + }); + + it('indexes an image fallback that carries no dimensions', () => { + // An oversized output falls back to a download URL with an image name and + // no width/height. `` needs those to reserve layout space; a + // markdown `` does not, and the URL serves the picture fine. + const fallback = attachment({ width: undefined, height: undefined } as Partial); + expect(buildAttachmentsByName([fallback]).get('5_dti.png')).toBe(fallback); + }); + + it('resolves the latest record of one rewritten file', () => { + // A tool rewriting one path reuses its file_id; the later record carries + // the newer URL, so keeping the first would resolve to a stale one. + const first = attachment({ file_id: 'f1', filepath: '/api/f/a.png' } as Partial); + const rewritten = attachment({ + file_id: 'f1', + filepath: '/api/f/a.png?v=2', + } as Partial); + expect(buildAttachmentsByName([first, rewritten]).get('5_dti.png')).toBe(rewritten); + }); + + it('skips attachments with nothing to point at', () => { + expect(buildAttachmentsByName([attachment({ filepath: undefined })]).size).toBe(0); + expect(buildAttachmentsByName([attachment({ filename: undefined })]).size).toBe(0); + }); + + it('keeps resolving after a rewritten file, without reading it as ambiguous', () => { + const first = attachment({ file_id: 'f1', filepath: '/api/f/a.png' } as Partial); + const rewritten = attachment({ + file_id: 'f1', + filepath: '/api/f/a.png?v=2', + } as Partial); + const third = attachment({ + file_id: 'f1', + filepath: '/api/f/a.png?v=3', + } as Partial); + expect(buildAttachmentsByName([first, rewritten, third]).get('5_dti.png')).toBe(third); + }); + + it('returns one shared instance when nothing is indexable', () => { + expect(buildAttachmentsByName(undefined)).toBe(buildAttachmentsByName([])); + }); +}); + +describe('resolveInlineMedia', () => { + const file = attachment(); + const byName = buildAttachmentsByName([file]); + + it.each([ + ['a bare filename', '5_dti.png'], + ['a sandbox path', '/mnt/data/5_dti.png'], + ['a percent-encoded name', '5%5Fdti.png'], + ['a cache-busting query', '5_dti.png?v=2'], + ])('resolves %s', (_label, src) => { + expect(resolveInlineMedia(src, byName)).toBe(file); + }); + + it.each([ + ['an absolute URL', 'https://example.com/5_dti.png'], + ['a data URI', 'data:image/png;base64,AAAA'], + ['a protocol-relative URL', '//cdn.example.com/5_dti.png'], + ['a sandbox scheme', 'sandbox:/mnt/data/5_dti.png'], + ['an explicit /api/ path', '/api/files/other/session/5_dti.png'], + ['an explicit /images/ path', '/images/user/5_dti.png'], + ])('leaves %s alone', (_label, src) => { + // The served paths matter even though a `5_dti.png` attachment exists: the + // author addressed one specific file, and reducing it to a basename would + // display a DIFFERENT attachment that happens to share the leaf. + expect(resolveInlineMedia(src, byName)).toBeUndefined(); + }); + + it('resolves nothing for a file the turn never produced', () => { + expect(resolveInlineMedia('missing.png', byName)).toBeUndefined(); + }); + + it('resolves nothing without a map', () => { + expect(resolveInlineMedia('5_dti.png', undefined)).toBeUndefined(); + expect(resolveInlineMedia(undefined, byName)).toBeUndefined(); + }); + + it('survives a malformed escape rather than throwing', () => { + expect(() => resolveInlineMedia('%E0%A4%A.png', byName)).not.toThrow(); + }); +}); + +describe('attachmentIdentity', () => { + it('prefers file_id over filepath', () => { + expect(attachmentIdentity(attachment({ file_id: 'f1' } as Partial))).toBe('f1'); + expect(attachmentIdentity(attachment())).toBe('/api/files/code/download/sess/abc/5_dti.png'); + }); + + it('keeps sibling calls that share a claimed file_id apart', () => { + // Documented in useAttachments: sibling code calls can share a claimed + // file_id for the same filename, and each anchors its own card. + const left = attachment({ file_id: 'f1', toolCallId: 't1' } as Partial); + const right = attachment({ file_id: 'f1', toolCallId: 't2' } as Partial); + expect(attachmentIdentity(left)).not.toBe(attachmentIdentity(right)); + }); + + it('keeps handoff agents repeating a provider tool id apart', () => { + const left = attachment({ + file_id: 'f1', + toolCallId: 't1', + agentId: 'a1', + } as Partial); + const right = attachment({ + file_id: 'f1', + toolCallId: 't1', + agentId: 'a2', + } as Partial); + expect(attachmentIdentity(left)).not.toBe(attachmentIdentity(right)); + }); + + it('falls back to type:toolCallId for unkeyed tool artifacts', () => { + const citation = { + type: 'file_search', + toolCallId: 't1', + conversationId: 'c1', + } as unknown as TAttachment; + expect(attachmentIdentity(citation)).toBe('file_search:t1'); + }); + + it('gives no identity to a row that names no stored file', () => { + // Two rows that cannot be told apart are two rows, not one. + expect( + attachmentIdentity(attachment({ file_id: undefined, filepath: undefined })), + ).toBeUndefined(); + }); +}); + +describe('toAbsoluteFilePath', () => { + const base = 'https://chat.example.com/librechat'; + + it.each([ + ['a code-execution download', '/api/files/code/download/sess/abc/5_dti.png'], + ['a share route', '/api/share/abc/img.png'], + ['an uploaded image', '/images/user/pic.png'], + ])('prefixes the API base onto %s', (_label, path) => { + // Without this every generated chart 404s against the origin root on a + // subpath deployment — and /api/ is what code-execution artifacts use. + expect(toAbsoluteFilePath(path, base)).toBe(`${base}${path}`); + }); + + it.each([ + ['an absolute URL', 'https://cdn.example.com/a.png'], + ['a data URI', 'data:image/png;base64,AAAA'], + ['a bare relative name', '5_dti.png'], + ['an unserved root path', '/static/a.png'], + ['an empty path', ''], + ])('leaves %s alone', (_label, path) => { + expect(toAbsoluteFilePath(path, base)).toBe(path); + }); + + it('serves exactly the paths resolution declines to look up', () => { + // Both read SERVED_PATH_PATTERN. If they ever disagree, an explicitly + // addressed file either loses its base URL or gets swapped for another + // attachment sharing its basename. + for (const path of ['/images/user/a.png', '/api/files/x/a.png']) { + expect(toAbsoluteFilePath(path, base)).toBe(`${base}${path}`); + expect( + resolveInlineMedia(path, buildAttachmentsByName([attachment({ filename: 'a.png' })])), + ).toBeUndefined(); + } + }); +}); + +describe('attachmentRenderKey', () => { + it('splits two run steps that reuse one provider tool-call id', () => { + // `filterAttachmentsForPart` routes these to different parts by step, so + // they are two artifacts. Collapsing them drops one from the media row — + // the only place it appears once the fold hides the parts' own copies. + const first = attachment({ + file_id: 'f1', + toolCallId: 't1', + stepId: 's1', + } as Partial); + const second = attachment({ + file_id: 'f1', + toolCallId: 't1', + stepId: 's2', + } as Partial); + expect(attachmentRenderKey(first)).not.toBe(attachmentRenderKey(second)); + }); + + it('keeps one record of one artifact together across re-renders', () => { + const a = attachment({ file_id: 'f1', toolCallId: 't1', stepId: 's1' } as Partial); + const b = attachment({ file_id: 'f1', toolCallId: 't1', stepId: 's1' } as Partial); + expect(attachmentRenderKey(a)).toBe(attachmentRenderKey(b)); + }); + + it('leaves a step-less legacy row on the plain identity', () => { + const legacy = attachment({ file_id: 'f1', toolCallId: 't1' } as Partial); + expect(attachmentRenderKey(legacy)).toBe(attachmentIdentity(legacy)); + }); + + it('has no key when the identity has none', () => { + expect( + attachmentRenderKey(attachment({ file_id: undefined, filepath: undefined })), + ).toBeUndefined(); + }); +}); diff --git a/client/src/utils/index.ts b/client/src/utils/index.ts index 5a42efa804..9a6706259b 100644 --- a/client/src/utils/index.ts +++ b/client/src/utils/index.ts @@ -9,6 +9,7 @@ export * from './icons'; export * from './email'; export * from './share'; export * from './files'; +export * from './media'; export * from './greeting'; export * from './latex'; export * from './tilde'; diff --git a/client/src/utils/media.ts b/client/src/utils/media.ts new file mode 100644 index 0000000000..93deef8921 --- /dev/null +++ b/client/src/utils/media.ts @@ -0,0 +1,216 @@ +import { imageExtRegex } from 'librechat-data-provider'; +import type { TAttachment, TFile } from 'librechat-data-provider'; + +/** Sources the browser already resolves on its own. */ +const ABSOLUTE_SOURCE_PATTERN = /^(?:[a-z][a-z\d+\-.]*:|\/\/)/i; + +/** + * Root-relative paths this app serves itself. One constant because two + * questions depend on the same answer: which paths need the API base + * (`toAbsoluteFilePath`), and which markdown sources are already an explicit + * address rather than a name to look up (`mediaKeyFromSource`). Letting those + * drift apart is how a source the author addressed exactly gets reduced to its + * basename and resolved to a different attachment that shares the leaf. + */ +const SERVED_PATH_PATTERN = /^\/(?:images|api)\//i; + +/** Shared instance so the overwhelmingly common "no attachments" turn keeps a + * referentially stable map, and every consumer memoized on it stays put. */ +const EMPTY_ATTACHMENTS_BY_NAME: ReadonlyMap = new Map(); + +/** + * Lookup key for a filename: the final path segment, lowercased. The backend + * stores filenames as forward-slash paths regardless of host OS, so the last + * segment is all a markdown reference and an attachment need to agree on. + */ +function mediaNameKey(name: string): string { + const trimmed = name.trim(); + if (!trimmed) { + return ''; + } + const slash = trimmed.lastIndexOf('/'); + return (slash < 0 ? trimmed : trimmed.slice(slash + 1)).toLowerCase(); +} + +function fileKeyOf(attachment: TAttachment): string | undefined { + const { file_id, filepath } = attachment as Partial; + return file_id ?? filepath; +} + +function ownerOf(attachment: TAttachment): { + toolCallId?: string; + agentId?: string; +} { + const owned = attachment as { toolCallId?: string; agentId?: string }; + return { toolCallId: owned.toolCallId, agentId: owned.agentId }; +} + +/** + * Stable identity for an attachment: `file_id ?? filepath` scoped by + * `toolCallId` and `agentId`, else `type:toolCallId` for unkeyed tool + * artifacts like file_search citations. `undefined` means the row has no + * stable identity and must never be compared to another — two entries that + * cannot be told apart are two entries, not one. + * + * The ownership dimensions are not decoration. Sibling code calls can share a + * claimed `file_id` for the same filename and handoff agents can repeat + * provider tool ids, so `file_id` alone collapses attachments the merge in + * `useAttachments` deliberately keeps as separate cards — which is why that + * merge is the caller this lives next to, rather than a second definition. + */ +export function attachmentIdentity(attachment: TAttachment): string | undefined { + const { type } = attachment as { type?: string }; + const { toolCallId, agentId } = ownerOf(attachment); + const fileKey = fileKeyOf(attachment); + if (fileKey) { + if (toolCallId == null) { + return fileKey; + } + return agentId ? `${fileKey}::${toolCallId}::${agentId}` : `${fileKey}::${toolCallId}`; + } + if (type != null && toolCallId != null) { + return `${type}:${toolCallId}`; + } + return undefined; +} + +/** + * Identity for deciding whether two records render as the SAME artifact, which + * is `attachmentIdentity` plus the host run step. + * + * The extra dimension is the difference between merging and rendering. The + * merge in `useAttachments` reconciles DB rows against live SSE records, whose + * `stepId` may not have arrived yet, so it matches wildcard-tolerantly and + * must not split on a field one side can lack. A rendered row has no such + * excuse: `filterAttachmentsForPart` deliberately routes records to their + * owning part BY run step, so two executions that reuse a provider tool-call + * id and claim the same `file_id` are two artifacts. Collapsing them would + * drop one from the media row, which — once the fold hides the parts' own + * copies — is the only place it appears. + * + * A record with no `stepId` keeps the plain identity, so a legacy row never + * splits away from itself. + */ +export function attachmentRenderKey(attachment: TAttachment): string | undefined { + const identity = attachmentIdentity(attachment); + if (identity == null) { + return undefined; + } + const { stepId } = attachment as { stepId?: string }; + return stepId == null ? identity : `${identity}::step:${stepId}`; +} + +/** + * Indexes a turn's attachments by the filename a markdown image would name + * them with. + * + * A basename naming more than one STORED FILE is ambiguous and is dropped. + * Nothing in `![...](output.png)` says which one the author meant, and picking + * either renders one file under the other's caption — the only outcome worse + * than not rendering. Dropping the name costs little: the reference renders as + * it does today, and the media row, which never lost either file, is where + * both stay visible. + * + * Only files an `` can display are indexed — an image extension and a + * fetchable path. Deliberately NOT `isImageAttachment`, which additionally + * demands width and height: those exist so `` can reserve layout space, + * and a markdown `` needs neither. Requiring them would drop the + * download-fallback an oversized output produces, which carries an image name + * and a working URL and displays perfectly well. + * + * Identity is the stored file (`file_id ?? filepath`), not the call that + * emitted it. One file surfacing under two calls — inherited across steps, or + * a regeneration that updated the record in place — is one file and still + * resolves. Two different files are ambiguous no matter who wrote them: an + * earlier version read a same-author collision as supersession and let the + * later file win, but a regeneration and two sibling calls that both wrote + * `output.png` are indistinguishable from the metadata, so that guess could + * only ever have been right by luck. + */ +export function buildAttachmentsByName( + attachments: ReadonlyArray | undefined, +): ReadonlyMap { + const byName = new Map(); + const ambiguous = new Set(); + for (const attachment of attachments ?? []) { + if (!attachment?.filepath || !imageExtRegex.test(attachment.filename ?? '')) { + continue; + } + const key = mediaNameKey(attachment.filename ?? ''); + if (!key || ambiguous.has(key)) { + continue; + } + const claimed = byName.get(key); + if (claimed != null && fileKeyOf(claimed) !== fileKeyOf(attachment)) { + ambiguous.add(key); + byName.delete(key); + continue; + } + /** New, or a fresher record of the SAME file — a tool rewriting one path + * reuses its `file_id` and the later record carries the newer URL, so + * keeping the first would resolve to a stale one. Matches what + * `collectPhaseAttachments` and the attachment renderer both do. */ + byName.set(key, attachment); + } + return byName.size === 0 ? EMPTY_ATTACHMENTS_BY_NAME : byName; +} + +/** + * Absolute URL for a stored file path. + * + * Root-relative paths the app serves itself — `/images/…` uploads and `/api/…` + * downloads, which is what every code-execution artifact resolves to — must + * carry the API base or the browser requests them against the origin root and + * they 404 under a subpath deployment. Shared with `Image` so a rendered + * attachment and an inline reference to that same attachment cannot disagree + * about which paths need the prefix. + */ +export function toAbsoluteFilePath(path: string, baseUrl: string): string { + if (!path || path.startsWith('http') || path.startsWith('data:')) { + return path; + } + return SERVED_PATH_PATTERN.test(path) ? `${baseUrl}${path}` : path; +} + +/** + * Lookup key for a markdown image destination, or `''` when the source is + * already an address rather than a name — anything carrying a scheme, or a + * root-relative path this app serves. + * + * A scheme is never looked up, `sandbox:` included. react-markdown's default + * `urlTransform` allows only http/https/irc/mailto/xmpp, so a `sandbox:` + * source is blanked before the image component ever sees it; treating it as a + * name here would claim a file the renderer cannot display. Supporting that + * form needs a `urlTransform` on the markdown pipeline first. The last case matters: an author writing + * `/api/files/…/chart.png` addressed one specific file, and reducing that to + * `chart.png` could resolve it to a different attachment sharing the leaf and + * display the wrong image. + */ +function mediaKeyFromSource(src: string): string { + if (ABSOLUTE_SOURCE_PATTERN.test(src) || SERVED_PATH_PATTERN.test(src)) { + return ''; + } + let decoded = src; + try { + decoded = decodeURIComponent(src); + } catch { + /* A malformed escape is not a reference we can resolve; match the raw form. */ + } + return mediaNameKey(decoded.split(/[?#]/, 1)[0]); +} + +/** + * The attachment a markdown image source refers to, or `undefined` when the + * source is already resolvable or names nothing this turn produced — in which + * case the caller keeps its existing behavior. + */ +export function resolveInlineMedia( + src: string | undefined, + byName: ReadonlyMap | undefined, +): TAttachment | undefined { + if (!src || byName == null || byName.size === 0) { + return undefined; + } + const key = mediaKeyFromSource(src); + return key ? byName.get(key) : undefined; +}