Conversation
mike-inkeep
commented
Feb 5, 2026
mike-inkeep
commented
- Add image handling support (without persistence to conversation history)
- Tweak image URL schema and tighten up tests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
|
Claude finished @mike-inkeep's task in 4m 57s —— View job PR Review Summary5 Key Findings | Risk: Medium 🟠🔶
|
| Location | Issue | Reason Excluded |
|---|---|---|
message-parts.ts:5-17 |
No size limit on base64 data URIs (unlike schemas.ts which has 1MB limit) |
Different use case - request-level validation vs storage; AI providers handle large payloads |
imageUrlSchema |
No internal IP blocklist for SSRF protection | URLs are passed to AI providers who fetch them; SSRF is provider's responsibility |
Agent.ts:3290 |
new URL(file.uri) could throw if URI is malformed |
Covered by upstream validation in route handlers |
chat.ts:343-348 |
Images not persisted to conversation history | Intentional per PR description |
chatDataStream.ts:68 |
Vercel schema uses text field for image URL (unconventional naming) |
Matches Vercel AI SDK v5 convention |
Discarded as invalid or not applicable
| Location | Issue | Reason Excluded |
|---|---|---|
Agent.ts:3288-3291 |
file.bytes access not narrowed after union check |
TypeScript narrowing via 'uri' in file is correct; the discriminated union ensures mutual exclusivity |
| Route handlers | Missing rate limiting for image uploads | Pre-existing concern not introduced by this PR |
generateTaskHandler.ts |
No image count limits | Reasonable for MVP; can be added if abuse occurs |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| export const isImageContentItem = ( | ||
| item: ContentItem | ||
| ): item is { type: 'image_url'; image_url: { url: string; detail?: 'auto' | 'low' | 'high' } } => { | ||
| return ( | ||
| item.type === 'image_url' && | ||
| 'image_url' in item && | ||
| imageUrlSchema.safeParse(item.image_url?.url).success | ||
| ); | ||
| }; |
There was a problem hiding this comment.
MAJOR Type guard uses inline type instead of imported ImageContentItem
The type predicate re-declares the type inline instead of using the imported ImageContentItem type from ../types/chat. This creates maintenance risk if the type changes.
| export const isImageContentItem = ( | |
| item: ContentItem | |
| ): item is { type: 'image_url'; image_url: { url: string; detail?: 'auto' | 'low' | 'high' } } => { | |
| return ( | |
| item.type === 'image_url' && | |
| 'image_url' in item && | |
| imageUrlSchema.safeParse(item.image_url?.url).success | |
| ); | |
| }; | |
| export const isImageContentItem = ( | |
| item: ContentItem | |
| ): item is ImageContentItem => { | |
| return ( | |
| item.type === 'image_url' && | |
| 'image_url' in item && | |
| item.image_url != null && | |
| imageUrlSchema.safeParse(item.image_url.url).success | |
| ); | |
| }; |
| .string() | ||
| .regex( | ||
| /^data:image\/(png|jpeg|jpg|webp);base64,/, | ||
| 'Image must be PNG, JPEG, or WebP format (GIF not supported by all providers)' |
There was a problem hiding this comment.
can you check this? i believe we can support the gif file format. but it's just that only a single frame will work, it cannot understand animated gifs. and that is ok
There was a problem hiding this comment.
claude and chatgpt both support GIF, but Gemini only supports these: https://ai.google.dev/gemini-api/docs/image-understanding#supported-formats
Gemini supports the following image format MIME types:
PNG - image/png
JPEG - image/jpeg
WEBP - image/webp
HEIC - image/heic
HEIF - image/heif
Alternatively, we can accept GIF and let the downstream provider error bubble up if we hit this case
There was a problem hiding this comment.
I think skip gif for now until we figure out how to properly expose agent capabilities to the front end for client side validation.
|
Claude finished @mike-inkeep's task in 5m 34s —— View job PR Review Summary1 Key Finding | Risk: Low This is a follow-up review after the PR was synchronized with commit ✅ Previous Feedback AddressedThe latest commit successfully addressed the following items from the previous review:
🟠🔶
|
| Location | Issue | Reason Excluded |
|---|---|---|
chatDataStream.ts:58 |
content: z.any() is permissive |
Intentional for backwards compatibility; actual parts are validated separately |
Agent.ts:3296 |
Fallback mimeType: 'image/*' for URL-based images |
Acceptable — providers handle MIME type detection from URL |
Discarded as invalid or not applicable
| Location | Issue | Reason Excluded |
|---|---|---|
message-parts.ts:8-20 |
No size limit on base64 data URIs | Different use case than storage validation; AI providers handle payload limits |
Previous: buildFilePart URL validation |
Was flagged as missing | ✅ Now addressed in latest commit |
| Previous: Type guard using inline type | Was flagged as maintenance risk | ✅ Now uses imported ImageContentItem |
| }; | ||
|
|
||
| export type ImageContentItem = { | ||
| type: 'image_url'; |
There was a problem hiding this comment.
Are we redefining types and schemas?
I think it's a mistake made throughout our codebase, but we try to reduce schemas and types were we can.
You can infer types from zod schemas. https://zod.dev/basics?id=inferring-types
|
|
||
| export type UserInput = | ||
| | string | ||
| | { |
There was a problem hiding this comment.
I think at this point we can follow the A2A protocol for parts.
I don't want to introduce too many intermediate types in the message handling.
I think we support: Vercel Streaming, A2A, OpenAI Completions, but our primary pipeline (our widget to our agent framework) does Vercel Streaming (Client Widget to Agents Server) to A2A (Agent Server internal messaging)
There was a problem hiding this comment.
I suggest defining types int he packages where we define A2A types or just re-using the existing by scoping down/omitting properties that we are not supporting in the A2A at this time.
| type: string; | ||
| text?: string; | ||
| image?: string | URL; | ||
| experimental_providerMetadata?: { openai?: { imageDetail?: 'auto' | 'low' | 'high' } }; |
There was a problem hiding this comment.
Even just the enum component can make a useful extraction as a schema so we don't have to redefine the enum set.
Pattern extracted from PR #1737 human reviewer feedback (amikofalvy): - Types should derive from Zod schemas using z.infer<typeof schema> - Use Pick/Omit/Partial instead of manually redefining type subsets - Extract shared enum/union schemas instead of inline string literals Changes: - pr-review-types.md: New anti-pattern + analysis step 6 with detection patterns - pr-review-consistency.md: Extended "Reuse" section to cover types This demonstrates the closed-pr-review-auto-improver output — these are the exact changes the agent proposed when run against PR #1737. Co-Authored-By: Claude <noreply@anthropic.com>
Patterns extracted from human reviewer feedback: - Type Definition Discipline: use z.infer<> instead of manual type definitions - Type Composition Safety: discriminated unions for mutually exclusive states Co-Authored-By: Claude <noreply@anthropic.com>
3 participants
