-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New components openrouter #16286
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
New components openrouter #16286
Conversation
Actions - Send Completion Request - Send Chat Completion Request - Retrieve Available Models
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces several new action modules for the OpenRouter component. The new actions enable retrieval of available models, sending chat completion requests, and sending plain text completion requests. Additionally, a new constant and a parsing utility function have been added in the common utilities. The primary OpenRouter application has been expanded with additional property definitions and methods to facilitate API interactions. Finally, the package metadata has been updated with a new version number and dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant R as RetrieveModelsAction
participant A as OpenRouterApp (listModels)
participant API as OpenRouter API
U->>R: Trigger retrieve available models
R->>A: Call listModels(context)
A->>API: Request list of models
API-->>A: Return models list
A-->>R: Forward models list summary
R-->>U: Return response with summary message
sequenceDiagram
participant U as User
participant S as SendRequestAction
participant A as OpenRouterApp (sendRequest)
participant API as OpenRouter API
U->>S: Trigger chat/completion request with parameters
S->>A: Call sendChatCompetionRequest / sendCompetionRequest(data)
A->>API: Send POST request with configuration
API-->>A: Return success/error response
A-->>S: Relay response with request ID
S-->>U: Return summary message
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/openrouter/actions/send-completion-request/send-completion-request.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (6)
components/openrouter/common/utils.mjs (1)
1-24: Consider refactoring duplicate JSON parsing logicThe parseObject utility function is well-structured and handles edge cases appropriately. However, there's duplicate JSON parsing logic for both arrays and strings.
export const parseObject = (obj) => { if (!obj) return undefined; + const tryParseJSON = (str) => { + try { + return JSON.parse(str); + } catch (e) { + return str; + } + }; + if (Array.isArray(obj)) { return obj.map((item) => { if (typeof item === "string") { - try { - return JSON.parse(item); - } catch (e) { - return item; - } + return tryParseJSON(item); } return item; }); } if (typeof obj === "string") { - try { - return JSON.parse(obj); - } catch (e) { - return obj; - } + return tryParseJSON(obj); } return obj; };components/openrouter/actions/retrieve-available-models/retrieve-available-models.mjs (1)
1-20: Consider adding explicit error handlingThe action is well-implemented with proper metadata and API integration. The summary export provides good user feedback.
You might want to add explicit error handling to provide a more user-friendly error message:
async run({ $ }) { - const response = await this.openrouter.listModels({ - $, - }); + try { + const response = await this.openrouter.listModels({ + $, + }); + + $.export("$summary", `Successfully retrieved ${response.data.length} available model(s)!`); + return response; + } catch (error) { + $.export("$summary", `Failed to retrieve models: ${error.message}`); + throw error; + } - - $.export("$summary", `Successfully retrieved ${response.data.length} available model(s)!`); - return response; },components/openrouter/actions/send-completion-request/send-completion-request.mjs (2)
10-134: Consider validating numerical properties to prevent NaN values
Properties such astemperature,topP,frequencyPenalty, etc., undergoparseFloatin therunmethod. If the user inadvertently supplies non-numerical inputs, this can lead toNaN, unexpectedly passing invalid data to the API. Consider adding a quick validation step before or after parsing to ensure these values are valid numbers.// Example of a quick validation approach within props: temperature: { ... + async validate(value) { + const parsed = parseFloat(value); + if (Number.isNaN(parsed)) { + throw new ConfigurationError("Temperature must be a valid number"); + } + }, },
135-189: Clarify or merge provider data when applying sort
Overwritingdata.providerwith{ sort: this.sort }(lines 160-164) may remove existingthis.providerinformation if any. If you intend to merge or preserve earlier provider properties, consider deep-merging instead of full overwrite.if (this.sort) { - data.provider = { - sort: this.sort, - }; + data.provider = { + ...data.provider, + sort: this.sort, + }; }components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (1)
136-190: Check for overwriting provider like in the completion request
As with the completion request action, be cautious that settingdata.providerto{ sort: this.sort }removes all otherproviderfields. Merge if needed, or confirm this overwrite is intended.if (this.sort) { - data.provider = { - sort: this.sort, - }; + data.provider = { + ...data.provider, + sort: this.sort, + }; }components/openrouter/openrouter.app.mjs (1)
152-165: Consider renaming methods for clarity
Methods are spelled assendChatCompetionRequestandsendCompetionRequest. While consistent in usage, these might be typos for “Completion.” Renaming could improve clarity and maintain a professional API.-sendChatCompetionRequest(opts = {}) { +sendChatCompletionRequest(opts = {}) { ... } -sendCompetionRequest(opts = {}) { +sendCompletionRequest(opts = {}) { ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
components/openrouter/actions/retrieve-available-models/retrieve-available-models.mjs(1 hunks)components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs(1 hunks)components/openrouter/actions/send-completion-request/send-completion-request.mjs(1 hunks)components/openrouter/common/constants.mjs(1 hunks)components/openrouter/common/utils.mjs(1 hunks)components/openrouter/openrouter.app.mjs(1 hunks)components/openrouter/package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (3)
components/openrouter/package.json (1)
3-3: Version and dependency update looks appropriateThe version increment from 0.0.1 to 0.1.0 follows semantic versioning principles, indicating a minor release with new features but no breaking changes. The addition of the @pipedream/platform dependency is properly structured.
Also applies to: 14-16
components/openrouter/common/constants.mjs (1)
1-5: Well-structured constant definitionThe EFFORT_OPTIONS constant provides a clear list of available effort levels for the OpenRouter API. The values and formatting follow best practices.
components/openrouter/actions/send-completion-request/send-completion-request.mjs (1)
1-9: Good initial structure and imports
Everything looks consistent for creating a new action module. TheConfigurationErrorimport is correctly utilized for error handling, and theopenrouter.app.mjsimport is referenced properly for further usage.
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (3)
171-171: Add null/undefined check for parseFloat operationAll other parseFloat operations in this file include a null/undefined check using the
&&operator, but this one doesn't. To maintain consistency and avoid potential errors, add the check here as well.- reasoning.max_tokens = parseFloat(this.reasoningMaxTokens); + reasoning.max_tokens = this.reasoningMaxTokens && parseFloat(this.reasoningMaxTokens);
6-9: Consider version increment for new actionFor a new action component, consider starting with version "0.0.1" instead of "0.0.1". This follows semantic versioning best practices where the first public release is typically 0.1.0, with 0.0.x reserved for development versions.
key: "openrouter-send-chat-completion-request", name: "Send Chat Completion Request", - version: "0.0.1", + version: "0.1.0", description: "Send a chat completion request to a selected model. [See the documentation](https://openrouter.ai/docs/api-reference/chat-completion)",
19-23: Add validation for messages array structureThe
messagesprop is described as expecting objects withroleandcontentfields, but there's no validation to ensure that the parsed input follows this structure. Consider adding validation to therunmethod to check the structure of the messages array.async run({ $ }) { if (this.effort && this.reasoningMaxTokens) { throw new ConfigurationError("**Reasoning Effort** and **Reasoning Max Tokens** cannot be used simultaneously."); } + + const messages = parseObject(this.messages); + // Validate messages structure + if (!Array.isArray(messages) || !messages.every(msg => + typeof msg === 'object' && msg.role && msg.content)) { + throw new ConfigurationError("Messages must be an array of objects with role and content properties"); + } + const data = { model: this.model, - messages: parseObject(this.messages), + messages,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (2)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (2)
142-142: Good correction on the messages parsingThe code now correctly uses
parseObject(this.messages)instead ofparseObject(this.prompt), addressing the issue identified in a previous review.
179-179:✅ Verification successful
Possible typo in method name
There appears to be a typo in the method name
sendChatCompetionRequest. It's missing the letter 'l' in "Completion". Verify that this matches the method name defined in theopenrouter.app.mjsfile.
🏁 Script executed:
#!/bin/bash # Check if the method name in openrouter.app.mjs matches the one being called grep -n "sendChatCompetion\|sendChatCompletion" "components/openrouter/openrouter.app.mjs"Length of output: 136
Method Naming Verified: No Typo Found
After verifying the definition in
components/openrouter/openrouter.app.mjs, it is clear that the method is defined assendChatCompetionRequest(line 152), which matches the call incomponents/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs(line 179). There is no typo, so no changes are needed.
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs
Outdated
Show resolved
Hide resolved
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs
Show resolved
Hide resolved
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.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (1)
136-189: 🛠️ Refactor suggestionAdd comprehensive error handling
While there is error handling for API response errors, other potential errors during execution are not caught and handled gracefully.
Wrap the entire execution logic in a try-catch block to handle other potential errors:
async run({ $ }) { + try { if (this.effort && this.reasoningMaxTokens) { throw new ConfigurationError("**Reasoning Effort** and **Reasoning Max Tokens** cannot be used simultaneously."); } // ... rest of the code ... if (response.error) { throw new ConfigurationError(response.error.message); } $.export("$summary", `A new chat completion request with Id: ${response.id} was successfully created!`); return response; + } catch (error) { + if (error instanceof ConfigurationError) { + throw error; + } + throw new Error(`Error sending chat completion request: ${error.message}`); + } },
♻️ Duplicate comments (2)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (2)
102-110: Update the reused propDefinition for the "models" propIn Pipedream, a propDefinition's type is fixed when defined and cannot be overridden when reused. The current implementation attempts to change the type of the reused
modelpropDefinition to"string[]"for themodelsprop, which won't work correctly.You need to create a new propDefinition in the OpenRouter app file specifically for the
modelsarray property, rather than trying to override the type of an existing one.
140-158: Remove unnecessary provider and reasoning properties from initial data objectThe
dataobject is initially constructed with all the properties, but there are references toproviderandreasoningthat are later handled separately in the code.Since
provideris conditionally set ifthis.sortexists (lines 159-163) andreasoningis constructed separately and conditionally added (lines 164-176), these properties should not be included in the initial data object construction.
🧹 Nitpick comments (1)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (1)
179-181: Consider making timeout configurableThe timeout is hardcoded to 5 minutes (300,000 ms), which might be reasonable for most LLM requests but could be too short for complex prompts or too long for simpler ones.
Consider adding a configurable timeout property with a reasonable default value:
+ timeout: { + type: "integer", + label: "Timeout", + description: "Maximum time in seconds to wait for a response (default: 300)", + optional: true, + default: 300, + }, // In the run method const response = await this.openrouter.sendChatCompetionRequest({ $, data, - timeout: 1000 * 60 * 5, + timeout: 1000 * (this.timeout || 300), });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs(1 hunks)components/openrouter/actions/send-completion-request/send-completion-request.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/openrouter/actions/send-completion-request/send-completion-request.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (1)
components/openrouter/actions/send-chat-completion-request/send-chat-completion-request.mjs (1)
142-142: Messages are correctly parsed nowThe code correctly parses the
messagesproperty, which addresses the previous issue wherethis.promptwas incorrectly referenced.
| if (Object.entries(reasoning).length) { | ||
| data.reasoning = reasoning; | ||
| } | ||
| const response = await this.openrouter.sendChatCompetionRequest({ |
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.
Fix the typo in method name
There appears to be a typo in the method name sendChatCompetionRequest - it's missing an "l" and should be sendChatCompletionRequest.
- const response = await this.openrouter.sendChatCompetionRequest({
+ const response = await this.openrouter.sendChatCompletionRequest({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await this.openrouter.sendChatCompetionRequest({ | |
| const response = await this.openrouter.sendChatCompletionRequest({ |
| messages: { | ||
| type: "string[]", | ||
| label: "Messages", | ||
| description: "A list of objects containing role and content. E.g. **{\"role\":\"user\", \"content\":\"text\"}**. [See the documentation](https://openrouter.ai/docs/api-reference/chat-completion#request.body.messages) for further details.", | ||
| }, |
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.
🛠️ Refactor suggestion
Add validation for messages format
The messages property is crucial for chat completion but doesn't have any validation to ensure the correct format.
Add validation in the run method to ensure each message has the required role and content properties:
async run({ $ }) {
+ // Validate message format
+ const messages = parseObject(this.messages);
+ if (!Array.isArray(messages) || !messages.length) {
+ throw new ConfigurationError("Messages must be a non-empty array");
+ }
+
+ for (const msg of messages) {
+ if (!msg.role || !msg.content) {
+ throw new ConfigurationError("Each message must have 'role' and 'content' properties");
+ }
+
+ if (!["system", "user", "assistant", "function"].includes(msg.role)) {
+ throw new ConfigurationError(`Invalid role: ${msg.role}. Valid roles are: system, user, assistant, function`);
+ }
+ }
// Rest of the run method...
const data = {
model: this.model,
- messages: parseObject(this.messages),
+ messages,
// Other properties...
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| messages: { | |
| type: "string[]", | |
| label: "Messages", | |
| description: "A list of objects containing role and content. E.g. **{\"role\":\"user\", \"content\":\"text\"}**. [See the documentation](https://openrouter.ai/docs/api-reference/chat-completion#request.body.messages) for further details.", | |
| }, | |
| async run({ $ }) { | |
| // Validate message format | |
| const messages = parseObject(this.messages); | |
| if (!Array.isArray(messages) || messages.length === 0) { | |
| throw new ConfigurationError("Messages must be a non-empty array"); | |
| } | |
| for (const msg of messages) { | |
| if (!msg.role || !msg.content) { | |
| throw new ConfigurationError("Each message must have 'role' and 'content' properties"); | |
| } | |
| if (!["system", "user", "assistant", "function"].includes(msg.role)) { | |
| throw new ConfigurationError(`Invalid role: ${msg.role}. Valid roles are: system, user, assistant, function`); | |
| } | |
| } | |
| // Rest of the run method... | |
| const data = { | |
| model: this.model, | |
| messages, | |
| // Other properties... | |
| }; | |
| // Continue with the rest of the logic... | |
| } |
jcortes
left a comment
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.
hi @luancazarine lgtm! Ready for QA!
Resolves #15025
Summary by CodeRabbit
New Features
Chores