-
Notifications
You must be signed in to change notification settings - Fork 118
feat(images): enable multi-turn conversations about image context parts #2604
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
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 |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ import type { | |
| import type { FinishReason, StepResult, ToolSet } from 'ai'; | ||
| import type { MidGenerationCompressor } from '../compression/MidGenerationCompressor'; | ||
| import type { ContextResolver } from '../context'; | ||
| import type { HydrateBlobToDataUrl } from '../services/blob-storage'; | ||
| import type { StreamHelper } from '../stream/stream-helpers'; | ||
| import type { ImageDetail } from '../types/chat'; | ||
| import type { SandboxConfig } from '../types/executionContext'; | ||
|
|
@@ -235,4 +236,6 @@ export interface AgentRunContext { | |
| currentCompressor: MidGenerationCompressor | null; | ||
| functionToolRelationshipIdByName: Map<string, string>; | ||
| taskDenialRedirects: Array<{ toolName: string; toolCallId: string; reason: string }>; | ||
| /** When set, conversation history image parts with blob URIs are hydrated to data URLs (e.g. when the model supports images). */ | ||
|
Contributor
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. Also where is the check for models supporting images?
Contributor
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. Looking at the code, there isn't one — |
||
| hydrateBlobToDataUrl?: HydrateBlobToDataUrl; | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,12 @@ | ||||||||||||||||||||||||||
| import type { FilePart } from '@inkeep/agents-core'; | ||||||||||||||||||||||||||
| import type { FilePart, MessageSelect } from '@inkeep/agents-core'; | ||||||||||||||||||||||||||
| import type { ModelMessage } from 'ai'; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| createDefaultConversationHistoryConfig, | ||||||||||||||||||||||||||
| formatMessagesAsConversationHistory, | ||||||||||||||||||||||||||
| getConversationHistoryWithCompression, | ||||||||||||||||||||||||||
| } from '../../data/conversations'; | ||||||||||||||||||||||||||
| import type { HydrateBlobToDataUrl } from '../../services/blob-storage'; | ||||||||||||||||||||||||||
| import { isBlobUri } from '../../services/blob-storage'; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| type ContextBreakdown, | ||||||||||||||||||||||||||
| calculateBreakdownTotal, | ||||||||||||||||||||||||||
|
|
@@ -11,15 +15,69 @@ import { | |||||||||||||||||||||||||
| import type { AgentRunContext, AiSdkContentPart } from '../agent-types'; | ||||||||||||||||||||||||||
| import { getPrimaryModel, getSummarizerModel } from './model-config'; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export async function buildConversationHistory( | ||||||||||||||||||||||||||
| ctx: AgentRunContext, | ||||||||||||||||||||||||||
| contextId: string, | ||||||||||||||||||||||||||
| taskId: string, | ||||||||||||||||||||||||||
| userMessage: string, | ||||||||||||||||||||||||||
| streamRequestId: string | undefined, | ||||||||||||||||||||||||||
| initialContextBreakdown: ContextBreakdown | ||||||||||||||||||||||||||
| ): Promise<{ conversationHistory: string; contextBreakdown: ContextBreakdown }> { | ||||||||||||||||||||||||||
| let conversationHistory = ''; | ||||||||||||||||||||||||||
| export async function hydrateConversationHistoryBlobParts( | ||||||||||||||||||||||||||
| messages: MessageSelect[], | ||||||||||||||||||||||||||
| hydrate: HydrateBlobToDataUrl | ||||||||||||||||||||||||||
| ): Promise<{ hydrated: MessageSelect[]; nonHydrated: MessageSelect[] }> { | ||||||||||||||||||||||||||
| const hydratedResults = await Promise.all( | ||||||||||||||||||||||||||
| messages.map(async (msg): Promise<{ message: MessageSelect; hydrated: boolean }> => { | ||||||||||||||||||||||||||
| const content = msg.content; | ||||||||||||||||||||||||||
| if (!content?.parts?.length) { | ||||||||||||||||||||||||||
| return { message: msg, hydrated: false }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| let hasHydratedParts = false; | ||||||||||||||||||||||||||
| const parts = await Promise.all( | ||||||||||||||||||||||||||
|
Contributor
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. 🟠 MAJOR: Unbounded parallel blob downloads may exhaust connections Issue: The nested Why: The upload path uses
Fix: Apply the same worker pool pattern used in import pLimit from 'p-limit';
const limit = pLimit(3);
const parts = await Promise.all(
content.parts.map((part) => limit(async () => {
// existing hydration logic
}))
);Refs:
Contributor
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. I think bounding by a concurrency measure is valid
Contributor
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. Agreed. Using |
||||||||||||||||||||||||||
| content.parts.map(async (part: NonNullable<MessageSelect['content']['parts']>[number]) => { | ||||||||||||||||||||||||||
| if (part.kind !== 'file' || typeof part.data !== 'string' || !isBlobUri(part.data)) { | ||||||||||||||||||||||||||
| return part; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| const mimeType = | ||||||||||||||||||||||||||
| typeof part.metadata?.mimeType === 'string' ? part.metadata.mimeType : undefined; | ||||||||||||||||||||||||||
| const dataUrl = await hydrate(part.data, mimeType); | ||||||||||||||||||||||||||
| if (dataUrl) { | ||||||||||||||||||||||||||
| hasHydratedParts = true; | ||||||||||||||||||||||||||
| return { ...part, data: dataUrl }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return part; | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const processedMsg = { ...msg, content: { ...content, parts } }; | ||||||||||||||||||||||||||
| return { message: processedMsg, hydrated: hasHydratedParts }; | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const hydrated = hydratedResults | ||||||||||||||||||||||||||
| .filter((result) => result.hydrated) | ||||||||||||||||||||||||||
| .map((result) => result.message); | ||||||||||||||||||||||||||
| const nonHydrated = hydratedResults | ||||||||||||||||||||||||||
| .filter((result) => !result.hydrated) | ||||||||||||||||||||||||||
| .map((result) => result.message); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return { hydrated, nonHydrated }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export async function buildConversationHistory({ | ||||||||||||||||||||||||||
| ctx, | ||||||||||||||||||||||||||
| contextId, | ||||||||||||||||||||||||||
| taskId, | ||||||||||||||||||||||||||
| userMessage, | ||||||||||||||||||||||||||
| streamRequestId, | ||||||||||||||||||||||||||
| initialContextBreakdown, | ||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||
| ctx: AgentRunContext; | ||||||||||||||||||||||||||
| contextId: string; | ||||||||||||||||||||||||||
| taskId: string; | ||||||||||||||||||||||||||
| userMessage: string; | ||||||||||||||||||||||||||
| streamRequestId: string | undefined; | ||||||||||||||||||||||||||
| initialContextBreakdown: ContextBreakdown; | ||||||||||||||||||||||||||
| }): Promise<{ | ||||||||||||||||||||||||||
| conversationHistoryWithFileData: MessageSelect[]; | ||||||||||||||||||||||||||
| conversationHistoryString: string; | ||||||||||||||||||||||||||
| contextBreakdown: ContextBreakdown; | ||||||||||||||||||||||||||
| }> { | ||||||||||||||||||||||||||
| let conversationHistory: MessageSelect[] = []; | ||||||||||||||||||||||||||
| const historyConfig = | ||||||||||||||||||||||||||
| ctx.config.conversationHistoryConfig ?? createDefaultConversationHistoryConfig(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -63,7 +121,32 @@ export async function buildConversationHistory( | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const conversationHistoryTokens = estimateTokens(conversationHistory); | ||||||||||||||||||||||||||
| let conversationHistoryWithFileData: MessageSelect[] = []; | ||||||||||||||||||||||||||
| if (ctx.hydrateBlobToDataUrl && conversationHistory.length > 0) { | ||||||||||||||||||||||||||
| const { hydrated, nonHydrated } = await hydrateConversationHistoryBlobParts( | ||||||||||||||||||||||||||
| conversationHistory, | ||||||||||||||||||||||||||
| ctx.hydrateBlobToDataUrl | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| conversationHistoryWithFileData = hydrated; | ||||||||||||||||||||||||||
| conversationHistory = nonHydrated; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const conversationHistoryString = formatMessagesAsConversationHistory(conversationHistory); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const hydratedHistoryWithFileData = conversationHistoryWithFileData | ||||||||||||||||||||||||||
| .flatMap((msg) => msg.content?.parts ?? []) | ||||||||||||||||||||||||||
| .map((part) => { | ||||||||||||||||||||||||||
| if (part.kind === 'text' && typeof part.text === 'string') { | ||||||||||||||||||||||||||
| return part.text; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (part.kind === 'file' && typeof part.data === 'string') { | ||||||||||||||||||||||||||
| return part.data; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return ''; | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
| .join('\n'); | ||||||||||||||||||||||||||
| const conversationHistoryTokens = | ||||||||||||||||||||||||||
| estimateTokens(conversationHistoryString) + estimateTokens(hydratedHistoryWithFileData); | ||||||||||||||||||||||||||
| const updatedContextBreakdown: ContextBreakdown = { | ||||||||||||||||||||||||||
| components: { | ||||||||||||||||||||||||||
| ...initialContextBreakdown.components, | ||||||||||||||||||||||||||
|
|
@@ -74,22 +157,56 @@ export async function buildConversationHistory( | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| calculateBreakdownTotal(updatedContextBreakdown); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return { conversationHistory, contextBreakdown: updatedContextBreakdown }; | ||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| conversationHistoryWithFileData, | ||||||||||||||||||||||||||
| conversationHistoryString, | ||||||||||||||||||||||||||
| contextBreakdown: updatedContextBreakdown, | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function buildInitialMessages( | ||||||||||||||||||||||||||
| systemPrompt: string, | ||||||||||||||||||||||||||
| conversationHistory: string, | ||||||||||||||||||||||||||
| userMessage: string, | ||||||||||||||||||||||||||
| imageParts?: FilePart[] | ||||||||||||||||||||||||||
| ): any[] { | ||||||||||||||||||||||||||
| const messages: any[] = []; | ||||||||||||||||||||||||||
| export function buildInitialMessages({ | ||||||||||||||||||||||||||
| systemPrompt, | ||||||||||||||||||||||||||
| conversationHistory, | ||||||||||||||||||||||||||
| userMessage, | ||||||||||||||||||||||||||
| imageParts, | ||||||||||||||||||||||||||
| conversationHistoryWithFileData, | ||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||
| systemPrompt: string; | ||||||||||||||||||||||||||
| conversationHistory: string; | ||||||||||||||||||||||||||
| userMessage: string; | ||||||||||||||||||||||||||
| imageParts?: FilePart[]; | ||||||||||||||||||||||||||
| conversationHistoryWithFileData?: MessageSelect[]; | ||||||||||||||||||||||||||
| }): ModelMessage[] { | ||||||||||||||||||||||||||
| const messages: ModelMessage[] = []; | ||||||||||||||||||||||||||
| messages.push({ role: 'system', content: systemPrompt }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (conversationHistory.trim() !== '') { | ||||||||||||||||||||||||||
| messages.push({ role: 'user', content: conversationHistory }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (conversationHistoryWithFileData?.length) { | ||||||||||||||||||||||||||
| conversationHistoryWithFileData.forEach((msg) => { | ||||||||||||||||||||||||||
| const content = (msg.content?.parts ?? []).reduce<AiSdkContentPart[]>((acc, part) => { | ||||||||||||||||||||||||||
| if (part.kind === 'text' && typeof part.text === 'string') { | ||||||||||||||||||||||||||
| acc.push({ type: 'text', text: part.text }); | ||||||||||||||||||||||||||
| return acc; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (part.kind === 'file' && typeof part.data === 'string') { | ||||||||||||||||||||||||||
| // Temporarily hard code to image. Must update when we add other file types. | ||||||||||||||||||||||||||
| acc.push({ type: 'image', image: part.data }); | ||||||||||||||||||||||||||
| return acc; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+195
to
+199
Contributor
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. 🟡 Minor: Hardcoded assumption that all file parts are images Issue: The comment acknowledges this ("Temporarily hard code to image. Must update when we add other file types."), but the type system allows any Why: If a non-image file part is stored in conversation history, it will be sent to the model as an image, which will likely cause API errors or undefined behavior. Fix: Add a type guard for image MIME types:
Suggested change
Refs:
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return acc; | ||||||||||||||||||||||||||
| }, []); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (content?.length && content.length > 0) { | ||||||||||||||||||||||||||
| messages.push({ role: 'user', content }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
Comment on lines
+187
to
+207
Contributor
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. Text context is dropped. Hydrated messages only extract
Contributor
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. @mike-inkeep Is this true?
Contributor
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. This was true in the original code but has been fixed in the "Review feedback" commit. Looking at the current |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const userContent = buildUserMessageContent(userMessage, imageParts); | ||||||||||||||||||||||||||
| messages.push({ | ||||||||||||||||||||||||||
| role: 'user', | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||

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.
we probably dont need the comment here
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.
Agreed — the comment restates what the type name already conveys. Safe to remove.