[ACTION] Update Google Gemini actions #16358
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces significant enhancements and refactoring to the Google Gemini integration. It adds a new "Generate Embeddings" action using the Gemini embedding API, introduces robust handling and validation for conversation history and safety settings in content generation actions, and unifies media input handling for multimodal actions. Constants for harm categories, block thresholds, and task types are defined for improved input validation and configuration. The app's methods and property definitions are updated to support these features, and the package version is incremented to 0.3.0. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant App
participant GeminiAPI
User->>Action: Provide text, (mediaPaths), history, safetySettings
Action->>Action: Validate and format history & safetySettings
Action->>App: Call generateContent or embedContent with formatted data
App->>GeminiAPI: POST request with payload
GeminiAPI-->>App: Return response
App-->>Action: Pass API response
Action-->>User: Output result
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/google_gemini/actions/common/generate-content.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/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.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/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (7)
✨ Finishing Touches
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. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (1)
57-63: Error message still references “Image/Video”The validation now targets the generic
mediaPathsprop, but the exception text still says “Image/Video paths”.-throw new ConfigurationError("Image/Video paths must be an array."); +throw new ConfigurationError("Media paths must be an array.");
📜 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 (8)
components/google_gemini/actions/common/generate-content.mjs(2 hunks)components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs(4 hunks)components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs(2 hunks)components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs(1 hunks)components/google_gemini/common/constants.mjs(1 hunks)components/google_gemini/google_gemini.app.mjs(2 hunks)components/google_gemini/package.json(1 hunks)components/hubspot/common/constants.mjs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (16)
components/hubspot/common/constants.mjs (2)
242-243: Updated exports to include new constantsThe exports have been appropriately updated to include the new HARM_CATEGORIES and BLOCK_THRESHOLDS constants.
217-222:❓ Verification inconclusive
New block threshold constants added to HubSpot component
The BLOCK_THRESHOLDS constant defines content moderation thresholds. Similar to HARM_CATEGORIES, placing these in the HubSpot component is unusual if they're primarily for Google Gemini functionality.
🏁 Script executed:
#!/bin/bash # Check if these constants are used elsewhere in the HubSpot component rg "BLOCK_THRESHOLDS" --type js components/hubspot/ # Check if the same constants exist in Google Gemini component rg "BLOCK_NONE" --type js components/google_gemini/Length of output: 103
Confirm placement of BLOCK_THRESHOLDS constants
I wasn’t able to find any references to
BLOCK_THRESHOLDSin the HubSpot component nor definitions of these same values in the Google Gemini component. Please verify whether these thresholds are actually consumed by HubSpot logic or if they belong in the Google Gemini module instead.• components/hubspot/common/constants.mjs (lines 217–222): check if
BLOCK_THRESHOLDSis ever imported or used
• components/google_gemini/: if these values power Gemini’s moderation, move them intocomponents/google_gemini/common/constants.mjsand update imports accordinglycomponents/google_gemini/package.json (1)
3-3: Version bump from 0.2.2 to 0.3.0The version has been increased from 0.2.2 to 0.3.0, which follows semantic versioning principles and correctly indicates the addition of new features (embedding generation functionality) while maintaining backward compatibility.
components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs (3)
4-9: Well-structured action metadata for the new embeddings featureThe action metadata is clear and includes a helpful link to the official documentation, which is good practice.
10-43: Comprehensive props configuration for embedding generationThe props are well-defined with appropriate descriptions. The model selector efficiently filters out discontinued/deprecated models and only includes those supporting embedding generation.
44-61: Clean implementation of the embedding generation run methodThe run method correctly uses the app's embedContent method and returns a helpful summary message along with the full response.
components/google_gemini/google_gemini.app.mjs (2)
13-16: Generalized media input handlingThe property has been updated from likely "imagePaths" to "mediaPaths" with an expanded description to support multiple media types (images, audio, video), which increases the flexibility of the component.
106-116:✅ Verification successful
New embedContent method for embedding generation
This method follows the same pattern as the existing generateContent method, appropriately constructing the request path based on the model and making a POST request to the embedding endpoint. The implementation is clean and consistent with existing code patterns.
🏁 Script executed:
#!/bin/bash # Check if MODEL_METHODS.EMBED_CONTENT is defined in the constants file rg "EMBED_CONTENT" --type js components/google_gemini/common/constants.mjsLength of output: 107
EmbedContent implementation is correct and ready to merge
Confirmed that
MODEL_METHODS.EMBED_CONTENTis defined as"embedContent"incomponents/google_gemini/common/constants.mjs. The newembedContentmethod follows the establishedgenerateContentpattern and correctly constructs the request path.components/google_gemini/common/constants.mjs (1)
9-22: Enumerations improve safety‑filter ergonomics – looks goodIntroducing
HARM_CATEGORIESandBLOCK_THRESHOLDSas named exports makes downstream validation much clearer and less error‑prone than magic strings.
No issues spotted here.components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs (2)
27-37: Conversation history prepending is correctPrepending
formatHistoryToContent(history)before the current user message matches Gemini’s expected chronological order (oldest → newest). Nice job keeping the API payload consistent.
46-57:responseFormatis checked but never forwarded – clarify intent
responseFormatgates the inclusion ofgenerationConfig, yet the resulting object only setsresponseMimeType, not the requestedresponseFormat.
If callers expect the field to affect the output (e.g."JSON" | "TEXT"), the current implementation has no effect.Please confirm whether:
responseFormatis obsolete (remove the prop and the conditional), or- Gemini expects a different field name (e.g.
response_format) that should be forwarded.This mismatch could lead to silent degradations in existing workflows.
components/google_gemini/actions/common/generate-content.mjs (5)
3-7: Necessary imports added for validation and safety configurations.The imports for
ConfigurationErrorand the safety-related constants are appropriate for supporting the new validation functionality.
28-39: Properties restructured correctly.The properties for
textandresponseFormathave been properly maintained and structured in a consistent format.
40-51: Well-documented conversation history and safety settings properties.The new properties for conversation history and safety settings include clear descriptions and examples, which will help users understand the expected format.
54-83: Robust history formatting method with proper validation.The
formatHistoryToContentmethod appropriately:
- Handles empty/undefined input
- Validates JSON format
- Checks for required fields and valid role values
- Transforms data to the API-expected format
This implementation helps prevent runtime errors when users provide invalid history data.
84-115: Comprehensive safety settings validation and formatting.The
formatSafetySettingsmethod includes thorough validation:
- Checks for properly formatted JSON
- Validates required fields existence
- Ensures categories and thresholds match allowed values from constants
- Transforms input to the format expected by the Gemini API
The detailed error messages will assist users in providing valid configuration.
...gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs
Show resolved
Hide resolved
...gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs
Show resolved
Hide resolved
...gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
google-gemini-update-actions
branch
2 times, most recently
from
27b098e to
984351a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (3)
2-2: Verify the mime dependency is declared in package.jsonThe code now imports the
mimepackage, but there's no indication that it's been added as a dependency in the component's manifest or package.json.#!/bin/bash # Check if mime is declared in the component's package.json grep -r "mime" --include="package.json" components/google_gemini/
24-38: 🛠️ Refactor suggestionHandle unknown MIME types and add error handling
The
fileToGenerativePartmethod has been updated to infer MIME types dynamically, but it still lacks handling for:
- Unknown file extensions where
mime.getType()returnsnull- Potential errors when reading files
fileToGenerativePart(filePath) { if (!filePath) { return; } - const mimeType = mime.getType(filePath); + const mimeType = mime.getType(filePath) ?? "application/octet-stream"; + + try { + return { + inline_data: { + mime_type: mimeType, + data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), + }, + }; + } catch (error) { + console.error(`Failed to read file: ${filePath}`, error); + return; + } - return { - inline_data: { - mime_type: mimeType, - data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), - }, - }; }
65-75: 🛠️ Refactor suggestionFilter out undefined parts to prevent API errors
When mapping media paths to generative parts, failures in
fileToGenerativePartcould result inundefinedelements in the parts array, which would cause the Gemini API to reject the request.const contents = [ ...this.formatHistoryToContent(history), { parts: [ - ...mediaPaths.map((path) => this.fileToGenerativePart(path)), + ...mediaPaths + .map((path) => this.fileToGenerativePart(path)) + .filter(Boolean), { text, }, ], }, ];
🧹 Nitpick comments (1)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (1)
57-63: Update error message to reflect broader media supportThe error message still refers specifically to "Image/Video paths" even though the property now supports other media types (as indicated by the rename to
mediaPaths).if (!Array.isArray(mediaPaths)) { - throw new ConfigurationError("Image/Video paths must be an array."); + throw new ConfigurationError("Media paths must be an array."); } if (!mediaPaths.length) { - throw new ConfigurationError("At least one media path must be provided."); + throw new ConfigurationError("At least one media file path must be provided."); }
📜 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/google_gemini/actions/common/generate-content.mjs(2 hunks)components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs(4 hunks)components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs(2 hunks)components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs(1 hunks)components/google_gemini/common/constants.mjs(1 hunks)components/google_gemini/google_gemini.app.mjs(2 hunks)components/google_gemini/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/google_gemini/package.json
- components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs
- components/google_gemini/google_gemini.app.mjs
- components/google_gemini/common/constants.mjs
- components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs
- components/google_gemini/actions/common/generate-content.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (3)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (3)
12-12: LGTM: Version incremented appropriatelyThe version has been incremented from 0.1.2 to 0.2.0, which is appropriate given the non-breaking changes to the API.
16-21: LGTM: Props refactored to use unified mediaPaths propertyGood refactoring to use a single
mediaPathsproperty instead of separateimagePaths, aligning with the updated app property definition.
81-82: LGTM: Proper integration of new safety settings and conversation historyGood implementation of the new content structure and safety settings, utilizing the helper methods from the common module.
jcortes
force-pushed
the
google-gemini-update-actions
branch
from
984351a to
e81e448
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/google_gemini/actions/common/generate-content.mjs (1)
42-47: Update safety settings description to include HARM_BLOCK_THRESHOLD_UNSPECIFIEDThe safety settings property description should include the "HARM_BLOCK_THRESHOLD_UNSPECIFIED" threshold option which was mentioned in the previous review comments.
- description: "Configure content filtering for different harm categories. Each item must be a valid JSON string with `category` (one of: `HARASSMENT`, `HATE_SPEECH`, `SEXUALLY_EXPLICIT`, `DANGEROUS`, `CIVIC`) and `threshold` (one of: `BLOCK_NONE`, `BLOCK_ONLY_HIGH`, `BLOCK_MEDIUM_AND_ABOVE`, `BLOCK_LOW_AND_ABOVE`). Example: `{\"category\": \"HARASSMENT\", \"threshold\": \"BLOCK_MEDIUM_AND_ABOVE\"}`", + description: "Configure content filtering for different harm categories. Each item must be a valid JSON string with `category` (one of: `HARASSMENT`, `HATE_SPEECH`, `SEXUALLY_EXPLICIT`, `DANGEROUS`, `CIVIC`) and `threshold` (one of: `BLOCK_NONE`, `BLOCK_ONLY_HIGH`, `BLOCK_MEDIUM_AND_ABOVE`, `BLOCK_LOW_AND_ABOVE`, `HARM_BLOCK_THRESHOLD_UNSPECIFIED`). Example: `{\"category\": \"HARASSMENT\", \"threshold\": \"BLOCK_MEDIUM_AND_ABOVE\"}`",
📜 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/google_gemini/actions/common/generate-content.mjs(2 hunks)components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs(4 hunks)components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs(2 hunks)components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs(1 hunks)components/google_gemini/common/constants.mjs(1 hunks)components/google_gemini/google_gemini.app.mjs(2 hunks)components/google_gemini/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/google_gemini/package.json
- components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs
- components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs
- components/google_gemini/google_gemini.app.mjs
- components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs
- components/google_gemini/common/constants.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (6)
components/google_gemini/actions/common/generate-content.mjs (6)
3-3: Good addition of ConfigurationError importAdding the ConfigurationError import from the platform is appropriate for the validation logic in the new methods.
24-29: LGTM: Text property definitionThe text property is properly defined with appropriate propDefinition referencing the app.
30-35: LGTM: Response format property definitionThe responseFormat property is properly defined with appropriate propDefinition referencing the app.
36-41: LGTM: Well-structured history property with clear documentationThe history property is well defined with appropriate type, label, and descriptive documentation that includes example format.
50-79: LGTM: Robust history formatting method with appropriate validationThe
formatHistoryToContentmethod is well implemented with:
- Proper handling of empty/null history
- Strong JSON validation with clear error messages
- Validation of required fields (text and role)
- Validation of role values (user or model)
- Correct transformation to the expected structure with parts and role
This provides robust validation for user input and appropriate error feedback.
80-111: LGTM: Well-structured safety settings formatting with thorough validationThe
formatSafetySettingsmethod is well implemented with:
- Proper handling of empty/null settings
- Strong JSON validation with clear error messages
- Validation of required fields (category and threshold)
- Validation against predefined constants with descriptive error messages
- Appropriate transformation to the expected structure
This provides robust validation for user input and appropriate error feedback.
jcortes
force-pushed
the
google-gemini-update-actions
branch
from
e81e448 to
042c460
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (4)
2-2: Ensure the mime dependency is added to package.jsonThe action now imports the
mimepackage, but it's important to verify that this dependency is also listed in the component'spackage.jsonto avoid runtime "module not found" errors.#!/bin/bash # Check if mime is listed in package.json dependencies grep -r "\"mime\":" components/google_gemini/package.json || echo "mime dependency not found"
30-31:⚠️ Potential issueHandle unknown MIME types to avoid API errors
mime.getType(filePath)may returnnullfor uncommon file extensions. Passingnullto the API as a MIME type would result in an invalid request.-const mimeType = mime.getType(filePath); +const mimeType = mime.getType(filePath) ?? "application/octet-stream";
69-69:⚠️ Potential issueFilter out failed conversions to prevent undefined parts
If
fileToGenerativePartreturnsundefined(e.g., for unreadable files), thepartsarray will containundefinedelements, causing Gemini to reject the request.-...mediaPaths.map((path) => this.fileToGenerativePart(path)), +...mediaPaths + .map((path) => this.fileToGenerativePart(path)) + .filter(Boolean),
25-38: 🛠️ Refactor suggestionAdd error handling for file reading operations
The current implementation doesn't handle potential errors when reading files, which could lead to unhandled exceptions if a file is missing or not readable.
fileToGenerativePart(filePath) { if (!filePath) { return; } const mimeType = mime.getType(filePath) ?? "application/octet-stream"; + try { + const fileData = fs.readFileSync(filePath); + return { + inline_data: { + mime_type: mimeType, + data: Buffer.from(fileData).toString("base64"), + }, + }; + } catch (error) { + console.error(`Error reading file ${filePath}: ${error.message}`); + return undefined; + } - return { - inline_data: { - mime_type: mimeType, - data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), - }, - }; },
🧹 Nitpick comments (2)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (2)
57-59: Update error message to match expanded media typesThe error message still references "Image/Video paths" but according to the AI summary,
mediaPathsnow supports images, audio, and videos.-throw new ConfigurationError("Image/Video paths must be an array."); +throw new ConfigurationError("Media paths (images, audio, or videos) must be an array.");
61-63: Update error message to match expanded media typesSimilar to the previous comment, update this error message to reflect the broader range of supported media types.
-throw new ConfigurationError("At least one media path must be provided."); +throw new ConfigurationError("At least one media file path (image, audio, or video) must be provided.");
📜 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/google_gemini/actions/common/generate-content.mjs(2 hunks)components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs(4 hunks)components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs(2 hunks)components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs(1 hunks)components/google_gemini/common/constants.mjs(1 hunks)components/google_gemini/google_gemini.app.mjs(2 hunks)components/google_gemini/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/google_gemini/package.json
- components/google_gemini/google_gemini.app.mjs
- components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs
- components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs
- components/google_gemini/common/constants.mjs
- components/google_gemini/actions/common/generate-content.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (4)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (4)
12-12: Version increment is appropriateThe version bump from 0.1.2 to 0.2.0 correctly follows semantic versioning since you're adding new functionality without breaking backward compatibility.
65-75: Content generation structure looks goodThe updated content generation structure properly incorporates conversation history and combines multiple media inputs with text into a single request, which aligns with the Gemini API's capabilities for multimodal inputs.
81-82: Safety settings integration is well implementedThe addition of safety settings through the common formatSafetySettings method is a good improvement that allows for better control over content generation safety parameters.
16-21: Streamlined media input handlingConsolidating different media types into a single
mediaPathsproperty simplifies the interface and makes the component more flexible.
...gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs
Show resolved
Hide resolved
...gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
google-gemini-update-actions
branch
from
ff53301 to
c04c3f6
Compare
jcortes
force-pushed
the
google-gemini-update-actions
branch
from
c04c3f6 to
b6df669
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (4)
2-2: Remember to declare the newmimedependencyThe action now imports
mime, but ensuremimeis listed inpackage.jsonor the component's manifest to avoid runtime "module not found" errors.
30-36: 🛠️ Refactor suggestionHandle unknown MIME types to avoid 4xx responses
mime.getType(filePath)may returnnullfor uncommon extensions, which would lead to an invalid request.-const mimeType = mime.getType(filePath); +const mimeType = mime.getType(filePath) ?? "application/octet-stream";Also consider adding error handling for file reading operations:
-data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), +try { + data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), +} catch (error) { + throw new ConfigurationError(`Failed to read file at ${filePath}: ${error.message}`); +}
69-69: 🛠️ Refactor suggestionFilter out failed conversions to prevent
undefinedpartsIf
fileToGenerativePartreturnsundefined(e.g., unreadable file), thepartsarray will containundefinedelements and Gemini will reject the request.-...mediaPaths.map((path) => this.fileToGenerativePart(path)), +...mediaPaths + .map((path) => this.fileToGenerativePart(path)) + .filter(Boolean),
70-72: 🛠️ Refactor suggestionEnsure text value is properly defined
The code references
textin the parts array, but there's no validation to ensure it's properly defined.{ - text, + text: text ?? "", // Ensure text has a default value if not defined },
🧹 Nitpick comments (1)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (1)
57-59: Update error message to match prop nameThe error message refers to "Image/Video paths" but the prop is named "mediaPaths", which could include other media types.
-throw new ConfigurationError("Image/Video paths must be an array."); +throw new ConfigurationError("Media paths must be an array.");
📜 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/google_gemini/actions/common/generate-content.mjs(2 hunks)components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs(4 hunks)components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs(2 hunks)components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs(1 hunks)components/google_gemini/common/constants.mjs(1 hunks)components/google_gemini/google_gemini.app.mjs(2 hunks)components/google_gemini/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/google_gemini/package.json
- components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs
- components/google_gemini/google_gemini.app.mjs
- components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs
- components/google_gemini/common/constants.mjs
- components/google_gemini/actions/common/generate-content.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (3)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (3)
16-21: LGTM: Improved media handling with unified propThe change from
imagePathstomediaPathsaligns with the app-level changes and improves the component's ability to handle different types of media uniformly.
65-75: LGTM: Enhanced content structure with history supportThe new content structure that incorporates conversation history through
formatHistoryToContentis a good improvement that enables more sophisticated interactions with the Gemini API.
82-82: LGTM: Added safety settingsIncluding formatted safety settings in the API request is a good enhancement for controlling the content generation process.
jcortes
force-pushed
the
google-gemini-update-actions
branch
from
b6df669 to
9932d26
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (5)
2-2:⚠️ Potential issueRemember to declare the new
mimedependency.This file imports
mime, but it's not clear if it has been added topackage.jsonas a dependency. Without this, the code will fail at runtime with "module not found" errors.
30-31: 🛠️ Refactor suggestionHandle unknown MIME types to avoid API errors.
mime.getType(filePath)may returnnullfor uncommon file extensions, causing the API to reject the request with a 400 error.-const mimeType = mime.getType(filePath); +const mimeType = mime.getType(filePath) ?? "application/octet-stream";
69-69:⚠️ Potential issueFilter out failed conversions to prevent undefined parts.
If
fileToGenerativePartreturnsundefined(e.g., unreadable file), thepartsarray will containundefinedelements and Gemini will reject the request.-...mediaPaths.map((path) => this.fileToGenerativePart(path)), +...mediaPaths + .map((path) => this.fileToGenerativePart(path)) + .filter(Boolean),
70-72: 🛠️ Refactor suggestionEnsure text value is properly defined.
The code references
textin the parts array, but there's no validation to ensure it's properly defined.{ - text, + text: text ?? "", // Ensure text has a default value if not defined },
35-35: 🛠️ Refactor suggestionAdd error handling for file reading operations.
The current implementation doesn't handle errors that might occur when reading files (e.g., file not found, permission issues).
- data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), + try { + data: Buffer.from(fs.readFileSync(filePath)).toString("base64"), + } catch (error) { + throw new ConfigurationError(`Failed to read file at path: ${filePath}. Error: ${error.message}`); + }components/google_gemini/actions/common/generate-content.mjs (3)
118-121: Address inconsistency in safety settings category/threshold handling.There's an inconsistency in how categories and thresholds are handled: the category is transformed by looking up its value in the constants object, while the threshold is used directly from the input.
For consistency, both should use the same approach. Either:
return { category, - threshold: setting.threshold, + threshold: constants.BLOCK_THRESHOLDS[setting.threshold], };Or:
return { - category, + category: setting.category, threshold: setting.threshold, };
63-71: 🛠️ Refactor suggestionProcess objects directly in history array.
The current implementation assumes every history item needs to be parsed from JSON, but we should allow for the case where an item in the array is already an object.
return history.map((itemStr) => { - let item = itemStr; - if (typeof item === "string") { - try { - item = JSON.parse(itemStr); - } catch (e) { - throw new ConfigurationError(`Invalid JSON in history item: ${itemStr}`); - } + let item = itemStr; + if (typeof item === "string") { + try { + item = JSON.parse(itemStr); + } catch (e) { + throw new ConfigurationError(`Invalid JSON in history item: ${itemStr}`); + } }
95-103: 🛠️ Refactor suggestionProcess objects directly in safety settings array.
Similar to the history array, we should allow for the case where a safety setting in the array is already an object.
return safetySettings.map((settingStr) => { - let setting = settingStr; - if (typeof setting === "string") { - try { - setting = JSON.parse(settingStr); - } catch (e) { - throw new ConfigurationError(`Invalid JSON in safety setting: ${settingStr}`); - } + let setting = settingStr; + if (typeof setting === "string") { + try { + setting = JSON.parse(settingStr); + } catch (e) { + throw new ConfigurationError(`Invalid JSON in safety setting: ${settingStr}`); + } }
🧹 Nitpick comments (1)
components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs (1)
57-58: Update error message to match the prop name.The error message refers to "Image/Video paths" but the prop is named
mediaPaths, which is more general and could include other media types.- throw new ConfigurationError("Image/Video paths must be an array."); + throw new ConfigurationError("Media paths must be an array.");
📜 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/google_gemini/actions/common/generate-content.mjs(2 hunks)components/google_gemini/actions/generate-content-from-text-and-image/generate-content-from-text-and-image.mjs(4 hunks)components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs(2 hunks)components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs(1 hunks)components/google_gemini/common/constants.mjs(1 hunks)components/google_gemini/google_gemini.app.mjs(2 hunks)components/google_gemini/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/google_gemini/package.json
- components/google_gemini/google_gemini.app.mjs
- components/google_gemini/actions/generate-embeddings/generate-embeddings.mjs
- components/google_gemini/actions/generate-content-from-text/generate-content-from-text.mjs
- components/google_gemini/common/constants.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
jcortes
force-pushed
the
google-gemini-update-actions
branch
from
9932d26 to
3dd50d7
Compare
WHY
Resolves #16262
Summary by CodeRabbit