-
Notifications
You must be signed in to change notification settings - Fork 5.5k
302 ai momps #18843
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
302 ai momps #18843
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughThis change adds a new 302.AI integration: a base app module with API helpers and model selection props, five new action modules (chat, chat-using-functions, chat-with-302-ai, classify-items, summarize-text, create-embeddings), shared common helpers/constants, and a package version bump with a dependency addition. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Action as Action (chat / specialized)
participant Common as Common Helper
participant App as _302_ai App
participant API as 302.AI API
User->>Action: Provide inputs (model, messages, params, functions, etc.)
Action->>Common: Build messages & payload (system/user + response_format if any)
Common->>App: createChatCompletion / createEmbeddings ( { $, data } )
App->>App: _getCommonArgs() / _makeRequest() (attach Authorization, base URL)
App->>API: POST /... (chat or embeddings)
API-->>App: Response (choices, usage, etc.)
App->>Action: Return response + extracted generated_message / embeddings
Action->>Action: formatOutput() / parse JSON schema / functions results
Action-->>User: Structured output (summary, messages, categorizations, embeddings)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
Deleted context file for LLMs
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: 11
🧹 Nitpick comments (17)
.husky/pre-push (1)
6-9: The lockfile modification check is now partially redundant.With
--frozen-lockfileon line 4, the install command will fail if the lockfile needs updating, making this git diff check somewhat redundant. However, keeping it provides clearer error messaging for developers, so the redundancy is acceptable.components/_302_ai/actions/common/common.mjs (1)
55-80: Remove unnecessary manual truthiness checks.According to the guidance in
components/memory.mdcand the retrieved learnings,@pipedream/platformutilities automatically exclude undefined values from HTTP requests. The manual ternary checks fortemperature,topP,n,presencePenalty,frequencyPenalty, andmaxTokensare unnecessary and add complexity.Based on learnings.
Apply this diff to simplify the method:
_getCommonArgs() { const args = { model: this.modelId, - temperature: this.temperature - ? parseFloat(this.temperature) - : undefined, - top_p: this.topP - ? parseFloat(this.topP) - : undefined, - n: this.n - ? parseInt(this.n) - : undefined, + temperature: this.temperature ? parseFloat(this.temperature) : undefined, + top_p: this.topP ? parseFloat(this.topP) : undefined, + n: this.n ? parseInt(this.n) : undefined, stop: this.stop, - presence_penalty: this.presencePenalty - ? parseFloat(this.presencePenalty) - : undefined, - frequency_penalty: this.frequencyPenalty - ? parseFloat(this.frequencyPenalty) - : undefined, - max_tokens: this.maxTokens - ? parseInt(this.maxTokens) - : undefined, + presence_penalty: this.presencePenalty ? parseFloat(this.presencePenalty) : undefined, + frequency_penalty: this.frequencyPenalty ? parseFloat(this.frequencyPenalty) : undefined, + max_tokens: this.maxTokens ? parseInt(this.maxTokens) : undefined, user: this.user, }; return args; }Or even simpler, rely on the platform to filter undefined values and just parse when values exist:
_getCommonArgs() { - const args = { + return { model: this.modelId, temperature: this.temperature ? parseFloat(this.temperature) : undefined, top_p: this.topP ? parseFloat(this.topP) : undefined, n: this.n ? parseInt(this.n) : undefined, stop: this.stop, presence_penalty: this.presencePenalty ? parseFloat(this.presencePenalty) : undefined, frequency_penalty: this.frequencyPenalty ? parseFloat(this.frequencyPenalty) : undefined, max_tokens: this.maxTokens ? parseInt(this.maxTokens) : undefined, user: this.user, }; - return args; }components/_302_ai/actions/create-embeddings/create-embeddings.mjs (1)
68-78: Remove unnecessary manual truthiness checks.According to the guidance in
components/memory.mdcand the retrieved learnings,@pipedream/platformutilities automatically exclude undefined values from HTTP requests. The manual checks on lines 68-78 are unnecessary.Based on learnings.
Apply this diff to simplify the code:
const data = { model: this.modelId, input: this.input, -}; - -if (this.user) { - data.user = this.user; -} - -if (this.encodingFormat) { - data.encoding_format = this.encodingFormat; -} - -if (this.dimensions) { - data.dimensions = parseInt(this.dimensions); + user: this.user, + encoding_format: this.encodingFormat, + dimensions: this.dimensions ? parseInt(this.dimensions) : undefined, }components/_302_ai/_302_ai.app.mjs (5)
24-32: Model filter is heuristic; prefer capability-based filtering.Relying on id regex (gpt|claude|gemini|llama|mistral|deepseek) is brittle and may miss/over‑include models. If the API exposes type/capabilities, filter on that instead, with regex fallback.
Example:
- return models - .filter((model) => model.id.match(/gpt|claude|gemini|llama|mistral|deepseek/gi)) + return models + .filter((m) => m.type === "chat" || m.capabilities?.includes?.("chat") || /gpt|claude|gemini|llama|mistral|deepseek/i.test(m.id)) .map((model) => ({ label: model.id, value: model.id, }));
39-47: Embeddings filter should use API metadata when available.Same concern as chat models. Prefer explicit type/capabilities with regex fallback.
- return models - .filter((model) => model.id.match(/embedding/gi)) + return models + .filter((m) => m.type === "embedding" || m.capabilities?.includes?.("embeddings") || /embedding/i.test(m.id)) .map((model) => ({ label: model.id, value: model.id, }));
62-71: Add sane defaults: timeout and consistent headers.External calls need timeouts to avoid hanging UI and to bound retries upstream.
return axios($, { ...args, url: `${this._baseApiUrl()}${path}`, headers: { ...args.headers, "Authorization": `Bearer ${this._apiKey()}`, "Content-Type": "application/json", }, + timeout: args.timeout ?? 30000, + validateStatus: (s) => s >= 200 && s < 300, });
101-106: Avoid accidental override of computed fields in return object.If the API ever returns
generated_textorgenerated_message, spreadingdatalast would overwrite local values. Put...datafirst.- return { - generated_text, - generated_message, - ...data, - }; + return { + ...data, + generated_text, + generated_message, + };
73-78: Consider handling variable API response shapes for resilience.The 302.ai API currently returns
{ data: models }, but adding fallbacks for alternative shapes ({ models }or direct array) is defensive programming. Evidence from similar integrations (e.g., printful_oauth) shows response shape variation exists across APIs in this codebase. The suggested refactor correctly handles all three cases and makes the$parameter default explicit, improving maintainability without breaking the three call sites at lines 12, 24, and 39—all safely consume arrays.components/_302_ai/actions/summarize-text/summarize-text.mjs (1)
53-61: Handle empty/invalid choices defensively.If the API returns no choices,
undefinedpropagates. Add a safe fallback.- if (n > 1) { - output.summaries = response.choices?.map(({ message }) => message.content); - } else { - output.summary = response.choices?.[0]?.message?.content; - } + const choices = Array.isArray(response.choices) ? response.choices : []; + if (n > 1) { + output.summaries = choices.map(({ message }) => message?.content).filter(Boolean); + } else { + output.summary = choices[0]?.message?.content ?? ""; + }components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (3)
108-114: Always include systemInstructions when provided (unless a system message already exists).Currently added only when no prior messages exist. Prepend if not present to ensure consistent behavior.
- } else { - if (this.systemInstructions) { - messages.push({ - "role": "system", - "content": this.systemInstructions, - }); - } - } + } + + if (this.systemInstructions) { + const hasSystem = messages.some((m) => m.role === "system"); + if (!hasSystem) { + messages.unshift({ role: "system", content: this.systemInstructions }); + } + }Also applies to: 116-120
54-61: Make JSON Schema prop optional for better UX.In additionalProps, add
optional: trueso the UI doesn’t require it when format switches back.props.jsonSchema = { type: "string", label: "JSON Schema", description: "Define the schema that the model's output must adhere to. Must be a valid JSON schema object.", + optional: true, };
121-137: Deduplicate response_format construction with common utilities.This logic duplicates similar code in common. Consider centralizing to avoid drift.
components/_302_ai/actions/chat-using-functions/chat-using-functions.mjs (5)
144-149: Use_getCommonArgs()to keep behavior consistent across actions.You’re manually setting tokens/temperature, omitting other shared args (e.g., n, top_p, penalties, stop, user). Reuse common to avoid drift.
- const data = { - model: this.modelId, - messages, - parallel_tool_calls: parseInt(this.parallelToolCalls) === 1, - tools: [], - }; - - if (this.maxTokens) { - data.max_tokens = parseInt(this.maxTokens); - } - - if (this.temperature) { - data.temperature = parseFloat(this.temperature); - } + const data = { + ...this._getCommonArgs(), + model: this.modelId, + messages, + parallel_tool_calls: parseInt(this.parallelToolCalls) === 1, + tools: [], + };Also applies to: 151-157
159-173: Validatefunctions/toolsshape before sending.Ensure each tool has
type: "function"andfunction.name+function.parameters.- if (Array.isArray(functions)) { - data.tools.push(...functions); - } else { - data.tools.push(functions); - } + const toolsArray = Array.isArray(functions) ? functions : [functions]; + for (const t of toolsArray) { + if (t?.type !== "function" || !t.function?.name || !t.function?.parameters) { + throw new Error("Each tool must have type=\"function\" and a function object with name and parameters."); + } + } + data.tools.push(...toolsArray);
174-183: Validate tool_choice against provided tools.If a specific function name is set, ensure it exists to prevent API errors.
- } else { - data.tool_choice = { - type: "function", - name: this.toolChoice, - }; - } + } else { + const name = this.toolChoice; + const exists = data.tools.some((t) => t.function?.name === name); + if (!exists) throw new Error(`tool_choice "${name}" does not match any provided function names`); + data.tool_choice = { type: "function", name }; + }
119-125: Make JSON Schema prop optional in additionalProps.Aligns with chat action and avoids accidental required field.
props.jsonSchema = { type: "string", label: "JSON Schema", description: "Define the schema that the model's output must adhere to.", + optional: true, };
74-90: Prefer boolean prop forparallelToolCalls.Use a boolean to avoid parseInt and reduce UI confusion.
- parallelToolCalls: { - type: "string", + parallelToolCalls: { + type: "boolean", label: "Parallel Function Calling", description: "Allow or prevent the model to call multiple functions in a single turn", - optional: true, - default: "1", - options: [ - { - label: "Enabled", - value: "1", - }, - { - label: "Disabled", - value: "0", - }, - ], + optional: true, + default: true,And:
- parallel_tool_calls: parseInt(this.parallelToolCalls) === 1, + parallel_tool_calls: !!this.parallelToolCalls,
📜 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 (16)
.husky/pre-push(1 hunks)components/_302_ai/_302_ai.app.mjs(1 hunks)components/_302_ai/actions/chat-using-functions/chat-using-functions.mjs(1 hunks)components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs(1 hunks)components/_302_ai/actions/classify-items/classify-items.mjs(1 hunks)components/_302_ai/actions/common/common-helper.mjs(1 hunks)components/_302_ai/actions/common/common.mjs(1 hunks)components/_302_ai/actions/common/constants.mjs(1 hunks)components/_302_ai/actions/create-embeddings/create-embeddings.mjs(1 hunks)components/_302_ai/actions/summarize-text/summarize-text.mjs(1 hunks)components/_302_ai/package.json(2 hunks)components/memory.mdc(1 hunks)components/pipedream-context.mdc(1 hunks)components/ticketsauce/ticketsauce_components.mdc(1 hunks)mdc_memory.mdc(1 hunks)mdc_pipedream-context.mdc(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/memory.mdcmdc_memory.mdc
🧬 Code graph analysis (8)
components/_302_ai/actions/chat-using-functions/chat-using-functions.mjs (3)
components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (3)
messages(65-65)data(139-143)response(145-148)components/_302_ai/actions/common/common-helper.mjs (3)
messages(33-42)data(43-47)response(48-51)components/_302_ai/_302_ai.app.mjs (3)
data(82-86)data(91-91)data(97-97)
components/_302_ai/actions/classify-items/classify-items.mjs (2)
components/_302_ai/actions/common/common-helper.mjs (2)
messages(33-42)response(48-51)components/_302_ai/actions/common/common.mjs (1)
messages(82-82)
components/_302_ai/actions/summarize-text/summarize-text.mjs (4)
components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (2)
messages(65-65)response(145-148)components/_302_ai/actions/common/common-helper.mjs (2)
messages(33-42)response(48-51)components/_302_ai/actions/common/common.mjs (1)
messages(82-82)components/_302_ai/actions/classify-items/classify-items.mjs (2)
output(67-69)n(70-72)
components/_302_ai/_302_ai.app.mjs (5)
components/_302_ai/actions/common/common.mjs (1)
args(56-78)components/_302_ai/actions/chat-using-functions/chat-using-functions.mjs (1)
data(144-149)components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (1)
data(139-143)components/_302_ai/actions/common/common-helper.mjs (1)
data(43-47)components/_302_ai/actions/create-embeddings/create-embeddings.mjs (1)
data(63-66)
components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (3)
components/_302_ai/actions/common/common-helper.mjs (3)
messages(33-42)data(43-47)response(48-51)components/_302_ai/actions/common/common.mjs (2)
messages(82-82)responseFormat(96-112)components/_302_ai/_302_ai.app.mjs (3)
data(82-86)data(91-91)data(97-97)
components/_302_ai/actions/common/common.mjs (2)
components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (2)
messages(65-65)responseFormat(121-137)components/_302_ai/actions/common/common-helper.mjs (1)
messages(33-42)
components/_302_ai/actions/common/common-helper.mjs (4)
components/_302_ai/actions/chat-using-functions/chat-using-functions.mjs (3)
messages(130-130)data(144-149)response(199-202)components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (3)
messages(65-65)data(139-143)response(145-148)components/_302_ai/actions/common/common.mjs (1)
messages(82-82)components/_302_ai/_302_ai.app.mjs (3)
data(82-86)data(91-91)data(97-97)
components/_302_ai/actions/create-embeddings/create-embeddings.mjs (2)
components/_302_ai/_302_ai.app.mjs (3)
data(82-86)data(91-91)data(97-97)components/_302_ai/actions/chat-using-functions/chat-using-functions.mjs (2)
data(144-149)response(199-202)
🔇 Additional comments (13)
components/ticketsauce/ticketsauce_components.mdc (1)
463-469: Clarify parameter formatting.Lines 463–469 show
total_aboveparameter description, but the formatting appears misaligned. Confirm that the parametertotal_aboveand its description are intentionally placed at this location, as the structure differs slightly from surrounding parameter blocks.components/_302_ai/package.json (1)
3-17: LGTM!The version bump to 0.1.0 and the addition of the
@pipedream/platformdependency are appropriate for this new integration.mdc_memory.mdc (1)
1-1: LGTM!The guidance is clear and aligns with established best practices for Pipedream component development.
components/memory.mdc (1)
1-1: Verify the need for duplicate memory guidance files.This file has identical content to
mdc_memory.mdc. Is this duplication intentional, or should these be consolidated?mdc_pipedream-context.mdc (1)
1-32: LGTM!This development guide provides comprehensive and clear best practices for Pipedream component development.
components/pipedream-context.mdc (1)
1-32: Verify the need for duplicate context files.This file has identical content to
mdc_pipedream-context.mdc. Is this duplication intentional, or should these be consolidated?components/_302_ai/actions/common/common.mjs (1)
81-119: LGTM!The
_getChatArgsmethod correctly constructs the messages array with optional system instructions and handles the various response format options appropriately.components/_302_ai/actions/common/constants.mjs (1)
1-27: LGTM!The constants are well-structured and provide clear options for chat response formats and summarization lengths.
components/_302_ai/_302_ai.app.mjs (1)
113-118: Embeddings endpoint wrapper looks good.components/_302_ai/actions/common/common-helper.mjs (1)
32-61: Solid scaffold for chat-style actions.Messages composition, common args usage, and summary export pattern look good.
components/_302_ai/actions/chat-with-302-ai/chat-with-302-ai.mjs (1)
139-143: Verification complete:modelis correctly included via_getCommonArgs().The
_getCommonArgs()method incomponents/_302_ai/actions/common/common.mjsincludesmodel: this.modelIdat line 57, which is then spread into the data object at line 115 of the chat-with-302-ai action. The implementation is correct and will not cause 400 errors.components/_302_ai/actions/summarize-text/summarize-text.mjs (1)
14-15: Verify documentation link accessibility and consider extracting to a constant for maintainability.The URL is a 302.ai documentation page, which aligns with the file context. However, the specific slug mapping and long-term stability cannot be confirmed remotely. Manually verify that the URL is publicly accessible and correctly references the Summarize Text feature. As an optional improvement, consider extracting this URL to a named constant (e.g.,
SUMMARIZE_TEXT_DOC_URL) to centralize maintenance and reduce future drift.components/_302_ai/actions/classify-items/classify-items.mjs (1)
70-72: Verification confirmed: thenproperty is defined in common props.The script output shows that
nis properly defined incomponents/_302_ai/actions/common/common.mjsat line 23, configured as an optional string property with description "How many completions to generate for each prompt". The code correctly converts it to an integer and defaults to 1 when undefined.
Deleted context file meant for LLMs.
Deleting context file for LLMs not intended to push.
Deleted context file meant for LLMs not intended to push.
Deleted context file meant for LLMs not intended to push.
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.
Looks good. Left one comment about a typo. And you'll need to merge w/ master to resolve the merge conflict with pnpm-lock.yaml. Moving to QA.
Correct typo in classify-items.mjs Co-authored-by: michelle0927 <[email protected]>
Resolves #17749
Related to 302 AI components.
Summary by CodeRabbit