fix: don’t treat SVG uploads as vision images#2728
fix: don’t treat SVG uploads as vision images#2728KerwinLarrobis wants to merge 1 commit intogiselles-ai:mainfrom
Conversation
|
|
@KerwinLarrobis is attempting to deploy a commit to the Giselle Team on Vercel. A member of the Team first needs to authorize it. |
|
Finished running flow.
|
||||||||||||||||||
Review Summary by QodoPrevent SVG files from being treated as vision images
WalkthroughsDescription• Remove SVG from supported vision image formats • Log warning and skip SVG files instead of sending them • Add WebP as supported image format for vision input Diagramflowchart LR
SVG["SVG file upload"] --> Check{"MIME type check"}
Check -->|JPEG/PNG/GIF/WebP| ImagePart["Return ImagePart"]
Check -->|SVG| Warn["Log warning message"]
Warn --> Skip["Return undefined"]
Check -->|PDF| FilePart["Return FilePart"]
File Changes1. packages/giselle/src/generations/internal/use-generation-executor.ts
|
📝 WalkthroughWalkthroughModified the vision input processing to add support for WebP images and properly reject SVG images by returning undefined with a logger warning. These changes are applied consistently across two code paths: the main file resolution and the app-entry parameter resolution paths. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review by Qodo
1. Trailing whitespace line added
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| "SVG is not supported for vision input. Please upload PNG/JPEG/GIF/WebP instead.", | ||
| ); | ||
| return undefined; | ||
|
|
There was a problem hiding this comment.
1. Trailing whitespace line added 📘 Rule violation ✓ Correctness
A line with trailing whitespace was introduced, which violates the repo’s Biome formatting conventions and can cause formatting churn in diffs/CI. This should be removed to keep formatting consistent.
Agent Prompt
## Issue description
A whitespace-only line was introduced, which violates Biome formatting conventions.
## Issue Context
Biome format/lint may reformat or fail checks due to trailing whitespace.
## Fix Focus Areas
- packages/giselle/src/generations/internal/use-generation-executor.ts[581-583]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| case "image/svg+xml": | ||
| args.context.logger.warn( | ||
| { id: fileParameter.id, type: fileParameter.type, name: fileParameter.name }, | ||
| "SVG is not supported for vision input. Please upload PNG/JPEG/GIF/WebP instead.", | ||
| ); | ||
| return undefined; |
There was a problem hiding this comment.
2. Svg still reaches vision 🐞 Bug ✓ Correctness
The PR only skips SVGs for appEntryResolver parameters; SVGs uploaded via File/Image nodes are still converted to ImagePart (mediaType=image/svg+xml) and included in outgoing ModelMessage content, so providers can still error on unsupported SVG input.
Agent Prompt
### Issue description
SVG is only skipped in `appEntryResolver` parameter handling. SVGs uploaded through File/Image nodes can still be converted to `ImagePart` and included in outbound `ModelMessage.content`, which likely reintroduces the original provider error (`Unsupported MIME type: image/svg+xml`).
### Issue Context
The workflow designer UI currently allows SVG uploads in the `image` file category, and `getFileContents()` maps any `file.type` under `image` category to an `ImagePart` without filtering.
### Fix Focus Areas
- packages/giselle/src/generations/utils.ts[440-472]
- packages/giselle/src/generations/utils.ts[184-226]
- internal-packages/workflow-designer-ui/src/editor/properties-panel/file-node-properties-panel/index.tsx[8-21]
- packages/giselle/src/generations/internal/use-generation-executor.ts[566-582]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
## 🔍 QA Testing Assistant by Giselle ### 📋 Manual QA Checklist Based on the changes in this PR, here are the key areas to test manually:
### ✨ Prompt for AI Agents Use the following prompts with Cursor or Claude Code to automate E2E testing: 📝 E2E Test Generation Prompt``` Hello! You are an expert automated testing engineer. Your task is to write a new Playwright E2E test suite based on the context and code changes from a recent Pull Request. The goal is to verify that SVG files are no longer treated as vision inputs, a warning is logged, and other supported image formats continue to work correctly. 1. Context Summary
2. Test ScenariosPlease create E2E tests covering the following scenarios in a new test file named
3. Playwright Implementation InstructionsUse the following guidelines to implement the tests.
4. MCP Integration GuidelinesWhile you don't need to write the config file, structure the tests to be compatible with Playwright MCP (Multi-threaded Coordinator and Player).
4. MCP Integration Guidelines
5. CI-Ready Code Requirements
--> ``` --- |
shige
left a comment
shige
left a comment
There was a problem hiding this comment.
Review
Thanks for fixing the SVG handling issue! The core fix is correct — returning undefined for SVG and adding WebP support are both good changes. However, there are a couple of things I'd like to see addressed before merging.
1. Trailing whitespace (nit)
packages/giselle/src/generations/internal/use-generation-executor.ts line 582
There's trailing whitespace after return undefined;. Could you clean that up?
2. UI still accepts SVG uploads for image nodes
internal-packages/workflow-designer-ui/src/editor/properties-panel/file-node-properties-panel/index.tsx line 18
The image upload accept list still includes image/svg+xml:
accept: ["image/png", "image/jpeg", "image/gif", "image/svg+xml"],With this PR, SVG files uploaded through the Image node will be silently dropped at generation time — the user won't see the warning log. This creates a confusing experience where the upload succeeds but the image is quietly ignored.
Could you remove image/svg+xml from this accept list so users can't upload SVGs as images in the first place? Also, since the backend now supports image/webp, it would be good to add it here too:
accept: ["image/png", "image/jpeg", "image/gif", "image/webp"],3. Misleading validation message for SVG in file-panel.tsx
internal-packages/workflow-designer-ui/src/editor/properties-panel/file-node-properties-panel/file-panel.tsx line 53
This validation returns "Please use Image node to upload this file." for image/svg+xml. Since SVGs will no longer work as image inputs, this message becomes misleading. If SVG is removed from the image accept list (as suggested above), this case can also be removed or updated to something like "SVG is not supported. Please use PNG/JPEG/GIF/WebP instead.".
2 participants
Summary
Prevent
image/svg+xmlfiles from being treated as visionImagePartinput.Instead of sending SVG files to LLM providers (which do not support SVG), this change logs a warning and skips the file.
Why
LLM vision providers (OpenAI, Anthropic, Google) do not support SVG image input and return:
Unsupported MIME type: image/svg+xml.This prevents SVG files from being passed as ImagePart objects.
Testing
pnpm -C packages/giselle test
All tests in the giselle package pass.
Closes #2714
Summary by CodeRabbit
New Features
Bug Fixes