-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(ai): support URL to base64 conversion in tool toModelOutput
#8257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,10 @@ import { | |
LanguageModelV2Message, | ||
LanguageModelV2Prompt, | ||
LanguageModelV2TextPart, | ||
LanguageModelV2ToolResultOutput, | ||
} from '@ai-sdk/provider'; | ||
import { | ||
convertToBase64, | ||
DataContent, | ||
FilePart, | ||
ImagePart, | ||
|
@@ -156,7 +158,10 @@ export function convertToLanguageModelMessage({ | |
type: 'tool-result' as const, | ||
toolCallId: part.toolCallId, | ||
toolName: part.toolName, | ||
output: part.output, | ||
output: convertOutputToLanguageModelOutput( | ||
part.output, | ||
downloadedAssets, | ||
), | ||
providerOptions, | ||
}; | ||
} | ||
|
@@ -173,7 +178,10 @@ export function convertToLanguageModelMessage({ | |
type: 'tool-result' as const, | ||
toolCallId: part.toolCallId, | ||
toolName: part.toolName, | ||
output: part.output, | ||
output: convertOutputToLanguageModelOutput( | ||
part.output, | ||
downloadedAssets, | ||
), | ||
providerOptions: part.providerOptions, | ||
})), | ||
providerOptions: message.providerOptions, | ||
|
@@ -237,8 +245,49 @@ async function downloadAssets( | |
}), | ||
})); | ||
|
||
const toolUrls = messages | ||
.filter(message => message.role === 'tool') | ||
.map(message => message.content) | ||
.flat() | ||
.filter(item => item.type === 'tool-result') | ||
.flatMap(item => { | ||
if (item.output.type === 'content') { | ||
const results = item.output.value; | ||
return results | ||
.map(result => { | ||
if (result.type === 'media' && result.data && result.mediaType) { | ||
const url = | ||
typeof result.data === 'string' ? new URL(result.data) : null; | ||
if (url instanceof URL) { | ||
return { ...result, data: url }; | ||
} | ||
} | ||
return null; | ||
}) | ||
.filter(url => url !== null); | ||
} | ||
return null; | ||
}) | ||
.filter( | ||
item => | ||
item && | ||
!isUrlSupported({ | ||
url: item.data.toString(), | ||
mediaType: item.mediaType, | ||
supportedUrls, | ||
}), | ||
) | ||
.map(item => item!.data); | ||
|
||
const allUrls = [...urls, ...toolUrls]; | ||
|
||
// download in parallel: | ||
const downloadedFiles = await download(plannedDownloads); | ||
const downloadedFiles = await Promise.all( | ||
allUrls.map(async url => ({ | ||
url, | ||
data: await downloadImplementation({ url }), | ||
})), | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The array index mapping is incorrect - View Details📝 Patch Detailsdiff --git a/packages/ai/src/prompt/convert-to-language-model-prompt.test.ts b/packages/ai/src/prompt/convert-to-language-model-prompt.test.ts
index d34044049..00c87dcf3 100644
--- a/packages/ai/src/prompt/convert-to-language-model-prompt.test.ts
+++ b/packages/ai/src/prompt/convert-to-language-model-prompt.test.ts
@@ -1455,15 +1455,17 @@ describe('convertToLanguageModelMessage', () => {
],
},
supportedUrls: {},
- downloadImplementation: async ({ url }) => {
- expect(url).toEqual(new URL('https://example.com/screenshot.png'));
- return {
- data: new Uint8Array([137, 80, 78, 71]), // PNG magic bytes
- mediaType: 'image/png',
+ download: async (downloads) => {
+ return downloads.map(({ url }) => {
+ expect(url).toEqual(new URL('https://example.com/screenshot.png'));
+ return {
+ data: new Uint8Array([137, 80, 78, 71]), // PNG magic bytes
+ mediaType: 'image/png',
};
+ });
+
},
});
-
expect(result).toEqual([
{
role: 'tool',
diff --git a/packages/ai/src/prompt/convert-to-language-model-prompt.ts b/packages/ai/src/prompt/convert-to-language-model-prompt.ts
index 13ddda99a..bb5650a72 100644
--- a/packages/ai/src/prompt/convert-to-language-model-prompt.ts
+++ b/packages/ai/src/prompt/convert-to-language-model-prompt.ts
@@ -279,30 +279,27 @@ async function downloadAssets(
)
.map(item => item!.data);
- const allUrls = [...urls, ...toolUrls];
+ const urls = plannedDownloads.map(pd => pd.url);
+ const allDownloads = [
+ ...plannedDownloads.map(pd => ({ url: pd.url, isUrlSupportedByModel: pd.isUrlSupportedByModel })),
+ ...toolUrls.map(url => ({ url, isUrlSupportedByModel: false }))
+ ];
// download in parallel:
- const downloadedFiles = await Promise.all(
- allUrls.map(async url => ({
- url,
- data: await downloadImplementation({ url }),
- })),
- );
+ const downloadedFiles = await download(allDownloads);
return Object.fromEntries(
downloadedFiles
- .filter(
- (
- downloadedFile,
- ): downloadedFile is {
- mediaType: string | undefined;
- data: Uint8Array;
- } => downloadedFile?.data != null,
- )
- .map(({ data, mediaType }, index) => [
- plannedDownloads[index].url.toString(),
- { data, mediaType },
- ]),
+ .map((downloadedFile, index) => {
+ if (downloadedFile?.data != null) {
+ return [
+ allDownloads[index].url.toString(),
+ { data: downloadedFile.data, mediaType: downloadedFile.mediaType },
+ ];
+ }
+ return null;
+ })
+ .filter(entry => entry !== null) as [string, { data: Uint8Array; mediaType: string | undefined }][],
);
}
AnalysisArray Index Mapping Bug in URL Download FunctionTechnical Issue DescriptionThe Primary Issues Identified
Code AnalysisThe
Impact
Root CauseThe code appears to be in an incomplete or partially refactored state where:
This represents a fundamental logical error in array correspondence that would prevent the feature from working correctly when both user message URLs and tool message URLs are present. |
||
|
||
return Object.fromEntries( | ||
downloadedFiles | ||
|
@@ -347,3 +396,38 @@ function convertPartToLanguageModelPart( | |
} | ||
} | ||
} | ||
|
||
function convertOutputToLanguageModelOutput( | ||
output: LanguageModelV2ToolResultOutput, | ||
downloadedAssets: Record< | ||
string, | ||
{ mediaType: string | undefined; data: Uint8Array } | ||
>, | ||
): LanguageModelV2ToolResultOutput { | ||
if (output.type === 'content') { | ||
return { | ||
type: 'content' as const, | ||
value: output.value.map(result => { | ||
if (result.type === 'media' && result.data && result.mediaType) { | ||
const { data: convertedData, mediaType: convertedMediaType } = | ||
convertToLanguageModelV2DataContent(result.data); | ||
if (convertedData instanceof URL) { | ||
const downloadedFile = downloadedAssets[convertedData.toString()]; | ||
if (downloadedFile) { | ||
return { | ||
type: 'media' as const, | ||
data: convertToBase64(downloadedFile.data), | ||
mediaType: | ||
downloadedFile.mediaType ?? | ||
convertedMediaType ?? | ||
result.mediaType, | ||
}; | ||
} | ||
} | ||
} | ||
return result; | ||
Comment on lines
+411
to
+428
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to avoid nesting, can you please implement an early return pattern? |
||
}), | ||
}; | ||
} | ||
return output; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable
downloadImplementation
is undefined - it should bedownload
to reference the function parameter.View Details
📝 Patch Details
Analysis
Undefined Variables in Download Assets Function
Issue Summary
The
downloadAssets
function inpackages/ai/src/prompt/convert-to-language-model-prompt.ts
contains multiple undefined variables that cause TypeScript compilation errors and would result in runtime failures.Specific Problems
1. Undefined Variable:
downloadImplementation
(Line 288)The code calls
downloadImplementation({ url })
but this variable does not exist in scope. The function parameter is nameddownload
.2. Undefined Variable:
urls
(Line 282)The code references
[...urls, ...toolUrls]
buturls
is never defined. This should be extracting URLs from theplannedDownloads
array.3. Function Signature Mismatch
The
DownloadFunction
type expects an array of download requests and returns an array of results, but the current code calls it with individual URL objects. This architectural mismatch shows the code expects batch downloads but implements individual calls.Compilation Impact
These errors prevent the TypeScript build from completing:
Runtime Impact
If the TypeScript errors were somehow bypassed, this would cause:
ReferenceError: downloadImplementation is not defined
when URLs need downloadingReferenceError: urls is not defined
when processing URL collectionsRoot Cause
This appears to be incomplete refactoring where:
download
parameter was renamed but call sites weren't updated