mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-24 16:46:33 +01:00
🪄 refactor: Simplify MCP Tool Content Formatting to Unified String Output (#12352)
* refactor: Simplify content formatting in MCP service and parser - Consolidated content handling in `formatToolContent` to return a plain-text string instead of an array for all providers, enhancing clarity and consistency. - Removed unnecessary checks for content array providers, streamlining the logic for handling text and image artifacts. - Updated related tests to reflect changes in expected output format, ensuring comprehensive coverage for the new implementation. * fix: Return empty string for image-only tool responses instead of '(No response)' When artifacts exist (images/UI resources) but no text content is present, return an empty string rather than the misleading '(No response)' fallback. Adds missing test assertions for image-only content and standardizes length checks to explicit `> 0` comparisons.
This commit is contained in:
Danny Avila
parent
b5c097e5c7
commit
7829fa9eca
4 changed files with 95 additions and 154 deletions
|
|
@ -15,13 +15,7 @@ const {
|
|||
GenerationJobManager,
|
||||
resolveJsonSchemaRefs,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Time,
|
||||
CacheKeys,
|
||||
Constants,
|
||||
ContentTypes,
|
||||
isAssistantsEndpoint,
|
||||
} = require('librechat-data-provider');
|
||||
const { Time, CacheKeys, Constants, isAssistantsEndpoint } = require('librechat-data-provider');
|
||||
const {
|
||||
getOAuthReconnectionManager,
|
||||
getMCPServersRegistry,
|
||||
|
|
@ -605,9 +599,6 @@ function createToolInstance({
|
|||
if (isAssistantsEndpoint(provider) && Array.isArray(result)) {
|
||||
return result[0];
|
||||
}
|
||||
if (isGoogle && Array.isArray(result[0]) && result[0][0]?.type === ContentTypes.TEXT) {
|
||||
return [result[0][0].text, result[1]];
|
||||
}
|
||||
return result;
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
|
|
|
|||
|
|
@ -31,12 +31,22 @@ describe('formatToolContent', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('recognized providers - content array providers', () => {
|
||||
const contentArrayProviders: t.Provider[] = ['google', 'anthropic', 'openai', 'azureopenai'];
|
||||
describe('recognized providers', () => {
|
||||
const allProviders: t.Provider[] = [
|
||||
'google',
|
||||
'anthropic',
|
||||
'openai',
|
||||
'azureopenai',
|
||||
'openrouter',
|
||||
'xai',
|
||||
'deepseek',
|
||||
'ollama',
|
||||
'bedrock',
|
||||
];
|
||||
|
||||
contentArrayProviders.forEach((provider) => {
|
||||
allProviders.forEach((provider) => {
|
||||
describe(`${provider} provider`, () => {
|
||||
it('should format text content as content array', () => {
|
||||
it('should format text content as string', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{ type: 'text', text: 'First text' },
|
||||
|
|
@ -45,11 +55,11 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, provider);
|
||||
expect(content).toEqual([{ type: 'text', text: 'First text\n\nSecond text' }]);
|
||||
expect(content).toBe('First text\n\nSecond text');
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should separate text blocks when images are present', () => {
|
||||
it('should extract images to artifacts and keep text as string', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{ type: 'text', text: 'Before image' },
|
||||
|
|
@ -59,10 +69,7 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, provider);
|
||||
expect(content).toEqual([
|
||||
{ type: 'text', text: 'Before image' },
|
||||
{ type: 'text', text: 'After image' },
|
||||
]);
|
||||
expect(content).toBe('Before image\n\nAfter image');
|
||||
expect(artifacts).toEqual({
|
||||
content: [
|
||||
{
|
||||
|
|
@ -76,62 +83,21 @@ describe('formatToolContent', () => {
|
|||
it('should handle empty content', () => {
|
||||
const result: t.MCPToolCallResponse = { content: [] };
|
||||
const [content, artifacts] = formatToolContent(result, provider);
|
||||
expect(content).toEqual([{ type: 'text', text: '(No response)' }]);
|
||||
expect(content).toBe('(No response)');
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('recognized providers - string providers', () => {
|
||||
const stringProviders: t.Provider[] = ['openrouter', 'xai', 'deepseek', 'ollama', 'bedrock'];
|
||||
|
||||
stringProviders.forEach((provider) => {
|
||||
describe(`${provider} provider`, () => {
|
||||
it('should format content as string', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{ type: 'text', text: 'First text' },
|
||||
{ type: 'text', text: 'Second text' },
|
||||
],
|
||||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, provider);
|
||||
expect(content).toBe('First text\n\nSecond text');
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should handle images with string output', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{ type: 'text', text: 'Some text' },
|
||||
{ type: 'image', data: 'base64data', mimeType: 'image/png' },
|
||||
],
|
||||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, provider);
|
||||
expect(content).toBe('Some text');
|
||||
expect(artifacts).toEqual({
|
||||
content: [
|
||||
{
|
||||
type: 'image_url',
|
||||
image_url: { url: 'data:image/png;base64,base64data' },
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('image handling', () => {
|
||||
it('should handle images with http URLs', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [{ type: 'image', data: 'https://example.com/image.png', mimeType: 'image/png' }],
|
||||
};
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const [_content, artifacts] = formatToolContent(result, 'openai');
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(content).toBe('');
|
||||
expect(artifacts).toEqual({
|
||||
content: [
|
||||
{
|
||||
|
|
@ -147,8 +113,8 @@ describe('formatToolContent', () => {
|
|||
content: [{ type: 'image', data: 'iVBORw0KGgoAAAA...', mimeType: 'image/png' }],
|
||||
};
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const [_content, artifacts] = formatToolContent(result, 'openai');
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(content).toBe('');
|
||||
expect(artifacts).toEqual({
|
||||
content: [
|
||||
{
|
||||
|
|
@ -158,6 +124,29 @@ describe('formatToolContent', () => {
|
|||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('should return empty string for image-only content when artifacts exist', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [{ type: 'image', data: 'base64data', mimeType: 'image/png' }],
|
||||
};
|
||||
const [content, artifacts] = formatToolContent(result, 'anthropic');
|
||||
expect(content).toBe('');
|
||||
expect(artifacts).toBeDefined();
|
||||
expect(artifacts?.content).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should handle multiple images without text', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{ type: 'image', data: 'https://example.com/a.png', mimeType: 'image/png' },
|
||||
{ type: 'image', data: 'https://example.com/b.jpg', mimeType: 'image/jpeg' },
|
||||
],
|
||||
};
|
||||
const [content, artifacts] = formatToolContent(result, 'google');
|
||||
expect(content).toBe('');
|
||||
expect(artifacts).toBeDefined();
|
||||
expect(artifacts?.content).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resource handling', () => {
|
||||
|
|
@ -176,13 +165,11 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(Array.isArray(content)).toBe(true);
|
||||
const textContent = Array.isArray(content) ? content[0] : { text: '' };
|
||||
expect(textContent).toMatchObject({ type: 'text' });
|
||||
expect(textContent.text).toContain('UI Resource ID:');
|
||||
expect(textContent.text).toContain('UI Resource Marker: \\ui{');
|
||||
expect(textContent.text).toContain('Resource URI: ui://carousel');
|
||||
expect(textContent.text).toContain('Resource MIME Type: application/json');
|
||||
expect(typeof content).toBe('string');
|
||||
expect(content).toContain('UI Resource ID:');
|
||||
expect(content).toContain('UI Resource Marker: \\ui{');
|
||||
expect(content).toContain('Resource URI: ui://carousel');
|
||||
expect(content).toContain('Resource MIME Type: application/json');
|
||||
|
||||
const uiResourceArtifact = artifacts?.ui_resources?.data?.[0];
|
||||
expect(uiResourceArtifact).toBeTruthy();
|
||||
|
|
@ -209,15 +196,11 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(content).toEqual([
|
||||
{
|
||||
type: 'text',
|
||||
text:
|
||||
'Resource Text: Document content\n' +
|
||||
'Resource URI: file://document.pdf\n' +
|
||||
'Resource MIME Type: application/pdf',
|
||||
},
|
||||
]);
|
||||
expect(content).toBe(
|
||||
'Resource Text: Document content\n' +
|
||||
'Resource URI: file://document.pdf\n' +
|
||||
'Resource MIME Type: application/pdf',
|
||||
);
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
|
||||
|
|
@ -235,12 +218,7 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(content).toEqual([
|
||||
{
|
||||
type: 'text',
|
||||
text: 'Resource URI: https://example.com/resource',
|
||||
},
|
||||
]);
|
||||
expect(content).toBe('Resource URI: https://example.com/resource');
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
|
||||
|
|
@ -267,14 +245,12 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(Array.isArray(content)).toBe(true);
|
||||
const textEntry = Array.isArray(content) ? content[0] : { text: '' };
|
||||
expect(textEntry).toMatchObject({ type: 'text' });
|
||||
expect(textEntry.text).toContain('Some text');
|
||||
expect(textEntry.text).toContain('UI Resource Marker: \\ui{');
|
||||
expect(textEntry.text).toContain('Resource URI: ui://button');
|
||||
expect(textEntry.text).toContain('Resource MIME Type: application/json');
|
||||
expect(textEntry.text).toContain('Resource URI: file://data.csv');
|
||||
expect(typeof content).toBe('string');
|
||||
expect(content).toContain('Some text');
|
||||
expect(content).toContain('UI Resource Marker: \\ui{');
|
||||
expect(content).toContain('Resource URI: ui://button');
|
||||
expect(content).toContain('Resource MIME Type: application/json');
|
||||
expect(content).toContain('Resource URI: file://data.csv');
|
||||
|
||||
const uiResource = artifacts?.ui_resources?.data?.[0];
|
||||
expect(uiResource).toMatchObject({
|
||||
|
|
@ -302,14 +278,11 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(Array.isArray(content)).toBe(true);
|
||||
if (Array.isArray(content)) {
|
||||
expect(content[0]).toMatchObject({ type: 'text', text: 'Content with multimedia' });
|
||||
expect(content[1].type).toBe('text');
|
||||
expect(content[1].text).toContain('UI Resource Marker: \\ui{');
|
||||
expect(content[1].text).toContain('Resource URI: ui://graph');
|
||||
expect(content[1].text).toContain('Resource MIME Type: application/json');
|
||||
}
|
||||
expect(typeof content).toBe('string');
|
||||
expect(content).toContain('Content with multimedia');
|
||||
expect(content).toContain('UI Resource Marker: \\ui{');
|
||||
expect(content).toContain('Resource URI: ui://graph');
|
||||
expect(content).toContain('Resource MIME Type: application/json');
|
||||
expect(artifacts).toEqual({
|
||||
content: [
|
||||
{
|
||||
|
|
@ -341,12 +314,9 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(content).toEqual([
|
||||
{
|
||||
type: 'text',
|
||||
text: 'Normal text\n\n' + JSON.stringify({ type: 'unknown', data: 'some data' }, null, 2),
|
||||
},
|
||||
]);
|
||||
expect(content).toBe(
|
||||
'Normal text\n\n' + JSON.stringify({ type: 'unknown', data: 'some data' }, null, 2),
|
||||
);
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
@ -379,20 +349,16 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'anthropic');
|
||||
expect(Array.isArray(content)).toBe(true);
|
||||
if (Array.isArray(content)) {
|
||||
expect(content[0]).toEqual({ type: 'text', text: 'Introduction' });
|
||||
expect(content[1].type).toBe('text');
|
||||
expect(content[1].text).toContain('Middle section');
|
||||
expect(content[1].text).toContain('UI Resource ID:');
|
||||
expect(content[1].text).toContain('UI Resource Marker: \\ui{');
|
||||
expect(content[1].text).toContain('Resource URI: ui://chart');
|
||||
expect(content[1].text).toContain('Resource MIME Type: application/json');
|
||||
expect(content[1].text).toContain('Resource URI: https://api.example.com/data');
|
||||
expect(content[2].type).toBe('text');
|
||||
expect(content[2].text).toContain('Conclusion');
|
||||
expect(content[2].text).toContain('UI Resource Markers Available:');
|
||||
}
|
||||
expect(typeof content).toBe('string');
|
||||
expect(content).toContain('Introduction');
|
||||
expect(content).toContain('Middle section');
|
||||
expect(content).toContain('UI Resource ID:');
|
||||
expect(content).toContain('UI Resource Marker: \\ui{');
|
||||
expect(content).toContain('Resource URI: ui://chart');
|
||||
expect(content).toContain('Resource MIME Type: application/json');
|
||||
expect(content).toContain('Resource URI: https://api.example.com/data');
|
||||
expect(content).toContain('Conclusion');
|
||||
expect(content).toContain('UI Resource Markers Available:');
|
||||
expect(artifacts).toMatchObject({
|
||||
content: [
|
||||
{
|
||||
|
|
@ -424,7 +390,7 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'openai');
|
||||
expect(content).toEqual([{ type: 'text', text: 'Error occurred' }]);
|
||||
expect(content).toBe('Error occurred');
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
|
||||
|
|
@ -435,7 +401,7 @@ describe('formatToolContent', () => {
|
|||
};
|
||||
|
||||
const [content, artifacts] = formatToolContent(result, 'google');
|
||||
expect(content).toEqual([{ type: 'text', text: 'Response with metadata' }]);
|
||||
expect(content).toBe('Response with metadata');
|
||||
expect(artifacts).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -18,7 +18,6 @@ const RECOGNIZED_PROVIDERS = new Set([
|
|||
'ollama',
|
||||
'bedrock',
|
||||
]);
|
||||
const CONTENT_ARRAY_PROVIDERS = new Set(['google', 'anthropic', 'azureopenai', 'openai']);
|
||||
|
||||
const imageFormatters: Record<string, undefined | t.ImageFormatter> = {
|
||||
// google: (item) => ({
|
||||
|
|
@ -81,13 +80,13 @@ function parseAsString(result: t.MCPToolCallResponse): string {
|
|||
}
|
||||
|
||||
/**
|
||||
* Converts MCPToolCallResponse content into recognized content block types
|
||||
* First element: string or formatted content (excluding image_url)
|
||||
* Second element: Recognized types - "image", "image_url", "text", "json"
|
||||
* Converts MCPToolCallResponse content into a plain-text string plus optional artifacts
|
||||
* (images, UI resources). All providers receive string content; images are separated into
|
||||
* artifacts and merged back by the agents package via formatArtifactPayload / formatAnthropicArtifactContent.
|
||||
*
|
||||
* @param result - The MCPToolCallResponse object
|
||||
* @param provider - The provider name (google, anthropic, openai)
|
||||
* @returns Tuple of content and image_urls
|
||||
* @param provider - Used only to distinguish recognized vs. unrecognized providers.
|
||||
* All recognized providers currently produce identical string output;
|
||||
* provider-specific artifact merging is delegated to the agents package.
|
||||
*/
|
||||
export function formatToolContent(
|
||||
result: t.MCPToolCallResponse,
|
||||
|
|
@ -99,13 +98,12 @@ export function formatToolContent(
|
|||
|
||||
const content = result?.content ?? [];
|
||||
if (!content.length) {
|
||||
return [[{ type: 'text', text: '(No response)' }], undefined];
|
||||
return ['(No response)', undefined];
|
||||
}
|
||||
|
||||
const formattedContent: t.FormattedContent[] = [];
|
||||
const imageUrls: t.FormattedContent[] = [];
|
||||
let currentTextBlock = '';
|
||||
const uiResources: UIResource[] = [];
|
||||
let currentTextBlock = '';
|
||||
|
||||
type ContentHandler = undefined | ((item: t.ToolContentPart) => void);
|
||||
|
||||
|
|
@ -122,17 +120,11 @@ export function formatToolContent(
|
|||
if (!isImageContent(item)) {
|
||||
return;
|
||||
}
|
||||
if (CONTENT_ARRAY_PROVIDERS.has(provider) && currentTextBlock) {
|
||||
formattedContent.push({ type: 'text', text: currentTextBlock });
|
||||
currentTextBlock = '';
|
||||
}
|
||||
const formatter = imageFormatters.default as t.ImageFormatter;
|
||||
const formattedImage = formatter(item);
|
||||
|
||||
if (formattedImage.type === 'image_url') {
|
||||
imageUrls.push(formattedImage);
|
||||
} else {
|
||||
formattedContent.push(formattedImage);
|
||||
}
|
||||
},
|
||||
|
||||
|
|
@ -195,25 +187,17 @@ UI Resource Markers Available:
|
|||
currentTextBlock += uiInstructions;
|
||||
}
|
||||
|
||||
if (CONTENT_ARRAY_PROVIDERS.has(provider) && currentTextBlock) {
|
||||
formattedContent.push({ type: 'text', text: currentTextBlock });
|
||||
}
|
||||
|
||||
let artifacts: t.Artifacts = undefined;
|
||||
if (imageUrls.length) {
|
||||
if (imageUrls.length > 0) {
|
||||
artifacts = { content: imageUrls };
|
||||
}
|
||||
|
||||
if (uiResources.length) {
|
||||
if (uiResources.length > 0) {
|
||||
artifacts = {
|
||||
...artifacts,
|
||||
[Tools.ui_resources]: { data: uiResources },
|
||||
};
|
||||
}
|
||||
|
||||
if (CONTENT_ARRAY_PROVIDERS.has(provider)) {
|
||||
return [formattedContent, artifacts];
|
||||
}
|
||||
|
||||
return [currentTextBlock, artifacts];
|
||||
return [currentTextBlock || (artifacts !== undefined ? '' : '(No response)'), artifacts];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -138,7 +138,7 @@ export type Artifacts =
|
|||
}
|
||||
| undefined;
|
||||
|
||||
export type FormattedContentResult = [string | FormattedContent[], undefined | Artifacts];
|
||||
export type FormattedContentResult = [string, Artifacts | undefined];
|
||||
|
||||
export type ImageFormatter = (item: ImageContent) => FormattedContent;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue