-
-
Notifications
You must be signed in to change notification settings - Fork 23.1k
feat: Add image upload support to ChatOpenRouter node #5471
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?
feat: Add image upload support to ChatOpenRouter node #5471
Conversation
- Create FlowiseChatOpenRouter wrapper class implementing IVisionChatModal - Add allowImageUploads and imageResolution input fields - Support multimodal image inputs for OpenRouter models - Follows same pattern as ChatOpenAI implementation Resolves FlowiseAI#5143
Summary of ChangesHello @aibysid, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the ChatOpenRouter node by integrating multimodal capabilities, specifically allowing users to provide image inputs to OpenRouter models. It introduces new configuration options for image handling and leverages a dedicated wrapper class to manage this functionality, aligning with existing multimodal implementations within the system. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request adds image upload support to the ChatOpenRouter node, following a similar pattern to the existing ChatOpenAI implementation. The changes are well-structured and introduce new UI fields for enabling image uploads and setting resolution. My review includes a couple of suggestions to enhance error message clarity and improve TypeScript type safety for better robustness. Overall, this is a solid contribution.
| parsedBaseOptions = typeof baseOptions === 'object' ? baseOptions : JSON.parse(baseOptions) | ||
| } catch (exception) { | ||
| throw new Error("Invalid JSON in the ChatCerebras's BaseOptions: " + exception) | ||
| throw new Error("Invalid JSON in the ChatOpenRouter's BaseOptions: " + exception) |
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.
Concatenating the exception object directly with a string can lead to unhelpful error messages like [object Object]. It's better to use the exception's message property if it's an Error instance, or convert it to a string otherwise, to provide more informative error details.
| throw new Error("Invalid JSON in the ChatOpenRouter's BaseOptions: " + exception) | |
| throw new Error("Invalid JSON in the ChatOpenRouter's BaseOptions: " + (exception instanceof Error ? exception.message : String(exception))) |
| export class ChatOpenRouter extends LangchainChatOpenAI implements IVisionChatModal { | ||
| configuredModel: string | ||
| configuredMaxToken?: number | ||
| multiModalOption: IMultiModalOption |
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 multiModalOption property is declared but not initialized in the constructor. It's set later via setMultiModalOption. This can lead to runtime errors if other methods access it before it's set. Using a definite assignment assertion (!) makes it clear that this property will be initialized elsewhere, improving type safety and code clarity.
| multiModalOption: IMultiModalOption | |
| multiModalOption!: IMultiModalOption |
Resolves #5143