Repository navigation
fix: pass native image/file parts through AgentRuntimeMessage pipeline - #1469
Conversation
b64bf20 to
af857bd
Compare
kwakayama
left a comment
There was a problem hiding this comment.
Code Review — 92/100
Fix is correct and complete. The three-layer change (type → convert → collect) is logically sound and all paths are covered by tests.
What's well done
toNativeFileParthelper eliminates the duplicateddata:URL guard and theas "image" | "file"cast — single source of truth for the URL validation logicconvertStructuredPartnow falls back tonullcleanly for data: URLs; annotation collection path is explicit withcontinuecollectAgentRuntimeProviderContentPartssymmetric with the provider→runtime direction — both sides filterdata:URLs consistentlyconvertAgentRuntimePartToChildForkMessageParthas explicitimage/fileguard +neverthrow for future variants- Single clean commit, bisectable history
- Tests cover: native passthrough, annotation fallback, file-only not dropped,
data:URL dropped on round-trip, resolver returningundefined
Remaining debt (pre-existing, not introduced here)
AgentRuntimeMessageLikePart uses { type: string; toolCallId: ... } instead of `tool-${string}`
This prevents TypeScript from discriminating type === "image" cleanly, which is why toNativeFilePart takes type as an explicit parameter rather than reading part.type after narrowing. The helper is the right workaround. The fix for the root cause would be changing the tool-call variants in the union to type: `tool-${string}`, but that's a separate PR.
Verdict: approve once CI is green.
kwakayama
left a comment
There was a problem hiding this comment.
Code Review — 92/100
Fix is correct and complete. The three-layer change (type → convert → collect) is logically sound and all paths are covered by tests.
What's well done
- `toNativeFilePart` helper eliminates the duplicated `data:` URL guard and the `as "image" | "file"` cast — single source of truth for the URL validation logic
- `convertStructuredPart` now falls back to `null` cleanly for data: URLs; annotation collection path is explicit with `continue`
- `collectAgentRuntimeProviderContentParts` symmetric with the provider→runtime direction — both sides filter `data:` URLs consistently
- `convertAgentRuntimePartToChildForkMessagePart` has explicit `image`/`file` guard + `never` throw for future variants
- Single clean commit, bisectable history
- Tests cover: native passthrough, annotation fallback, file-only not dropped, `data:` URL dropped on round-trip, resolver returning `undefined`
Remaining debt (pre-existing, not introduced here)
`AgentRuntimeMessageLikePart` uses `{ type: string; toolCallId: ... }` instead of ```tool-${string}```
This prevents TypeScript from discriminating `type === "image"` cleanly, which is why `toNativeFilePart` takes `type` as an explicit parameter rather than reading `part.type` after narrowing. The helper is the right workaround. The fix for the root cause would be changing the tool-call variants in the union to `type: `tool-${string}``, but that is a separate PR.
Verdict: approve once CI is green.
Code Review — 92/100Fix is correct and complete. The three-layer change (type → convert → collect) is logically sound and all paths are covered by tests. What's well done
Remaining debt (pre-existing, not introduced here)
Verdict: approve once CI is green. |
af857bd to
6a36896
Compare
#3499) - Add image/file variants to AgentRuntimeMessagePart and AgentRuntimeMessageLikePart - convertStructuredPart: return native parts for URL-based attachments; fall back to XML annotation for data: URLs or parts with no URL - collectAgentRuntimeProviderContentParts: collect file parts; guard against data: URLs - createProviderMessageFromAgentRuntimeMessage: emit structured ChatUserContentPart[] for user messages with file parts; no longer drops file-only messages - Add image/file variants to MessagePartSchema in agent.schema.ts - convertAgentRuntimePartToChildForkMessagePart: explicit image/file guard with exhaustive never-check for future variants - Bump version 0.1.403 → 0.1.404; sync version-constant.ts
6a36896 to
2134998
Compare
Problem
When a user sends a file attachment (image or PDF), two things break:
EMPTY_COMPLETION— the LLM receives only an XML annotation with no user intent, produces a minimal/empty response, and the error panel is shown.Root cause:
convertContentToAgentRuntimePartsinagent-runtime-message-adapter.tsstripped allimage/fileparts fromProviderModelMessage → AgentRuntimeMessageconversion, replacing them with an XML annotation:"Attached files from earlier conversation context: ...". When converting back (AgentRuntimeMessage → ProviderModelMessage), user messages only received string content, so the LLM never got multimodal input.Fix
{ type: "image"; url: string; mediaType: string }and{ type: "file"; url: string; mediaType: string }variants toAgentRuntimeMessagePartandAgentRuntimeMessageLikePartconvertStructuredPart: return native image/file parts for URL-based attachments (signed URLs from veryfront-agent)convertContentToAgentRuntimeParts: try native conversion first; fall back to XML annotation only when URL is missing or is adata:base64 URLcollectAgentRuntimeProviderContentParts+createProviderMessageFromAgentRuntimeMessage: collect file parts and emit structuredChatUserContentPart[]content for user messagesMessagePartSchemain agent.schema.tshosted-child-fork-step-message-preparation.tsagainst image/file parts (convert to text annotation for child fork messages)What's preserved
The XML annotation fallback is kept for:
data:URLs (can't pass as URL to provider)rewriteUnsupportedFilePartsAsAnnotationsProviders
All three supported providers (Anthropic, OpenAI, Google) support native URL-based multimodal input. No provider-specific handling required.
Reproduction script
Evidence
Related
Test plan
agent-runtime-message-adapter.test.ts— 8 tests: native passthrough, annotation fallback, round-trip, file-only not droppedruntime-message-preparation.test.ts— updated to expect native file part after URL resolutionhosted-child-fork-step-message-preparation.test.ts— 3 existing tests passsrc/agent/suite: 280 passed, 0 failedscripts/repro-3499-file-attachment.ts— 4 end-to-end scenarios all pass