-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New Components - alttextify #17029
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: master
Are you sure you want to change the base?
New Components - alttextify #17029
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe changes introduce a new integration for Alttextify, including the main app module, utility functions, three new actions (submit image, retrieve alt text by asset ID, and delete image alt text), and a polling source that emits events when new alt text is generated. Supporting files and test event fixtures are also added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SubmitImageAction
participant AlttextifyApp
participant AlttextifyAPI
User->>SubmitImageAction: Provide image and metadata
SubmitImageAction->>AlttextifyApp: uploadImage(params)
AlttextifyApp->>AlttextifyAPI: POST /images (with payload)
AlttextifyAPI-->>AlttextifyApp: asset_id, status, etc.
AlttextifyApp-->>SubmitImageAction: Response
SubmitImageAction-->>User: asset_id and summary
sequenceDiagram
participant User
participant GetAltTextByAssetIdAction
participant AlttextifyApp
participant AlttextifyAPI
User->>GetAltTextByAssetIdAction: Provide asset ID
GetAltTextByAssetIdAction->>AlttextifyApp: retrieveAltTextByAssetId(assetId)
AlttextifyApp->>AlttextifyAPI: GET /images/{assetId}
AlttextifyAPI-->>AlttextifyApp: alt text data
AlttextifyApp-->>GetAltTextByAssetIdAction: Response
GetAltTextByAssetIdAction-->>User: alt text and summary
sequenceDiagram
participant User
participant DeleteImageAction
participant AlttextifyApp
participant AlttextifyAPI
User->>DeleteImageAction: Provide asset ID
DeleteImageAction->>AlttextifyApp: deleteAltTextByAssetId(assetId)
AlttextifyApp->>AlttextifyAPI: DELETE /images/{assetId}
AlttextifyAPI-->>AlttextifyApp: deletion confirmation
AlttextifyApp-->>DeleteImageAction: Response
DeleteImageAction-->>User: summary
sequenceDiagram
participant PollingSource
participant AlttextifyApp
participant AlttextifyAPI
participant User
PollingSource->>AlttextifyApp: listAltTexts()
AlttextifyApp->>AlttextifyAPI: GET /images (with pagination)
AlttextifyAPI-->>AlttextifyApp: list of alt texts
AlttextifyApp-->>PollingSource: alt text data
PollingSource-->>User: Emit new alt text events
Assessment against linked issues
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/alttextify/alttextify.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs ✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Sources - New Image Processed Actions - Submit Image - Submit Image From URL - Get AltText By Asset ID - Get AltText By Job ID - Delete Image
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 (6)
components/alttextify/common/utils.mjs (1)
1-6
: Add input validation for robustness.The function logic is correct, but consider adding input validation to handle edge cases and improve reliability.
export const checkTmp = (filename) => { + if (!filename || typeof filename !== 'string') { + throw new Error('Filename must be a non-empty string'); + } if (!filename.startsWith("/tmp")) { return `/tmp/${filename}`; } return filename; };components/alttextify/actions/get-alttext-by-job-id/get-alttext-by-job-id.mjs (1)
11-15
: Consider adding validation for the Job ID.The jobId prop could benefit from validation to ensure it's not empty and matches expected format patterns.
jobId: { type: "string", label: "Job ID", description: "The ID of the job for retrieving alt text.", + optional: false, },
components/alttextify/actions/delete-image/delete-image.mjs (1)
11-15
: Consider making the assetId description more specific to this action.The description mentions both "retrieving or deleting" but this action only performs deletion. Consider updating for clarity.
- description: "The ID of the asset for retrieving or deleting alt text.", + description: "The ID of the asset to delete alt text for.",components/alttextify/alttextify.app.mjs (2)
10-10
: Capitalize the description for consistency.The description starts with a lowercase letter, which is inconsistent with other property descriptions in this file.
- description: "whether to add the image in the background or immediately (synchronously). If async is set to true, the API response will always be successful with an empty response body.", + description: "Whether to add the image in the background or immediately (synchronously). If async is set to true, the API response will always be successful with an empty response body.",
150-150
: Improve pagination logic for clarity and robustness.The current implementation assigns a number to
hasMore
instead of a boolean, which works but is not semantically clear. Additionally, this assumes that an empty response means no more pages, which might not be true for all API implementations.Consider improving the pagination logic:
- hasMore = data.length; + hasMore = data.length > 0;Or implement a more robust check based on the API's pagination response structure if available (e.g., checking for a
hasNextPage
field or comparing against total count).components/alttextify/actions/submit-image/submit-image.mjs (1)
88-107
: Consider conditionally including the ecommerce object.The current implementation always sends the
ecommerce
object even when all its properties are undefined, which might send unnecessary data to the API.Only include the ecommerce object if at least one property is defined:
+ const ecommerceData = {}; + if (this.ecommerceRunOCR !== undefined) { + ecommerceData.run_ocr = this.ecommerceRunOCR; + } + + const productData = {}; + if (this.ecommerceProductName) productData.name = this.ecommerceProductName; + if (this.ecommerceProductBrand) productData.brand = this.ecommerceProductBrand; + if (this.ecommerceProductColor) productData.color = this.ecommerceProductColor; + if (this.ecommerceProductSize) productData.size = this.ecommerceProductSize; + + if (Object.keys(productData).length > 0) { + ecommerceData.product = productData; + } + const response = await this.alttextify.uploadImage({ $, data: { async: this.async, image: `data:image/${mimeSubtype};base64,${image}`, lang: this.lang, maxChars: this.maxChars, assetId: this.assetId, keywords: this.keywords, - ecommerce: { - run_ocr: this.ecommerceRunOCR, - product: { - name: this.ecommerceProductName, - brand: this.ecommerceProductBrand, - color: this.ecommerceProductColor, - size: this.ecommerceProductSize, - }, - }, + ...(Object.keys(ecommerceData).length > 0 && { ecommerce: ecommerceData }), }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
components/alttextify/actions/delete-image/delete-image.mjs
(1 hunks)components/alttextify/actions/get-alttext-by-asset-id/get-alttext-by-asset-id.mjs
(1 hunks)components/alttextify/actions/get-alttext-by-job-id/get-alttext-by-job-id.mjs
(1 hunks)components/alttextify/actions/submit-image-from-url/submit-image-from-url.mjs
(1 hunks)components/alttextify/actions/submit-image/submit-image.mjs
(1 hunks)components/alttextify/alttextify.app.mjs
(1 hunks)components/alttextify/common/utils.mjs
(1 hunks)components/alttextify/package.json
(2 hunks)components/alttextify/sources/new-alttext-generated/new-alttext-generated.mjs
(1 hunks)components/alttextify/sources/new-alttext-generated/test-event.mjs
(1 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 (10)
components/alttextify/package.json (2)
3-3
: Appropriate version bump for new integration.The version increment from "0.0.1" to "0.1.0" follows semantic versioning conventions for introducing new functionality.
15-17
: Good addition of required platform dependency.The "@pipedream/platform" dependency is correctly added and uses an appropriate version constraint for the new Alttextify integration components.
components/alttextify/sources/new-alttext-generated/test-event.mjs (1)
1-7
: Well-structured test event data.The test event structure includes all necessary fields with appropriate data types. The alt_text array format allows for multiple descriptions, and the ISO 8601 timestamp format is standard.
components/alttextify/actions/get-alttext-by-job-id/get-alttext-by-job-id.mjs (1)
17-25
: Well-implemented action following Pipedream patterns.The async run method correctly:
- Calls the appropriate app method with proper parameters
- Uses the $ context for error handling
- Exports a descriptive summary message
- Returns the API response
The implementation follows Pipedream action conventions effectively.
components/alttextify/actions/delete-image/delete-image.mjs (1)
17-25
: LGTM! Clean implementation of the delete action.The async run method correctly calls the alttextify app method, exports an appropriate summary, and returns the response. The error handling is delegated to the app module as expected.
components/alttextify/actions/get-alttext-by-asset-id/get-alttext-by-asset-id.mjs (2)
11-15
: LGTM! Well-defined assetId property.The description is specific to this action's purpose, which is clearer than the delete action's description.
17-25
: LGTM! Correct implementation of the retrieval action.The method correctly calls retrieveAltTextByAssetId, exports an appropriate summary, and returns the response.
components/alttextify/sources/new-alttext-generated/new-alttext-generated.mjs (2)
58-62
: LGTM! Appropriate deploy hook implementation.The deploy hook correctly emits an initial batch of 25 events to initialize the source state.
29-56
:❓ Verification inconclusive
Review the date comparison and pagination logic.
The implementation looks generally correct, but there are a few considerations:
- The date comparison
Date.parse(item.created_at) <= lastDate
assumes the API returns items in descending order (newest first)- The array is reversed before emitting, confirming this assumption
- The lastDate is set to the first (newest) item's date
Please verify that the Alttextify API returns alt text entries in descending chronological order (newest first) as assumed by this logic.
🏁 Script executed:
#!/bin/bash # Description: Search for API documentation or other sources that mention the ordering of listAltTexts results ast-grep --pattern 'listAltTexts($$$)'Length of output: 40
🏁 Script executed:
#!/bin/bash # Search for the definition and usage of listAltTexts and paginate in the codebase rg -n "listAltTexts" . rg -n "paginate" .Length of output: 65681
🏁 Script executed:
#!/bin/bash # Extract the implementation of listAltTexts from alttextify.app.mjs sed -n '100,150p' components/alttextify/alttextify.app.mjsLength of output: 1104
Verify Alttextify API result ordering
Please confirm that the
listAltTexts
endpoint returns items sorted bycreated_at
in descending order (newest first). The currentemitEvent
logic in
components/alttextify/sources/new-alttext-generated/new-alttext-generated.mjs
assumes:
- Pages are fetched newest-first
- Loop breaks when
Date.parse(item.created_at) <= lastDate
lastDate
is set from the first (newest) item- Emission is reversed to deliver oldest-to-newest in this batch
If the API does not guarantee descending order, please add an explicit sort parameter to the request or sort the returned array client-side before filtering.
components/alttextify/actions/submit-image-from-url/submit-image-from-url.mjs (1)
82-106
: LGTM! Well-structured data building and API call.The run method correctly builds the nested data structure with all the provided props and calls the uploadImageFromUrl method. The summary export includes the returned asset_id which is helpful for users.
components/alttextify/actions/submit-image-from-url/submit-image-from-url.mjs
Outdated
Show resolved
Hide resolved
components/alttextify/actions/get-alttext-by-asset-id/get-alttext-by-asset-id.mjs
Outdated
Show resolved
Hide resolved
components/alttextify/actions/submit-image-from-url/submit-image-from-url.mjs
Outdated
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: 1
🧹 Nitpick comments (2)
components/alttextify/actions/submit-image/submit-image.mjs (1)
115-123
: Optimize ecommerce object construction.The ecommerce object is always included even when no ecommerce fields are provided, which may unnecessarily bloat the request payload.
- ecommerce: { - run_ocr: this.ecommerceRunOCR, - product: { - name: this.ecommerceProductName, - brand: this.ecommerceProductBrand, - color: this.ecommerceProductColor, - size: this.ecommerceProductSize, - }, - }, + ...(this.ecommerceRunOCR !== undefined || this.ecommerceProductName || this.ecommerceProductBrand || this.ecommerceProductColor || this.ecommerceProductSize ? { + ecommerce: { + ...(this.ecommerceRunOCR !== undefined && { run_ocr: this.ecommerceRunOCR }), + product: { + ...(this.ecommerceProductName && { name: this.ecommerceProductName }), + ...(this.ecommerceProductBrand && { brand: this.ecommerceProductBrand }), + ...(this.ecommerceProductColor && { color: this.ecommerceProductColor }), + ...(this.ecommerceProductSize && { size: this.ecommerceProductSize }), + }, + }, + } : {}),components/alttextify/alttextify.app.mjs (1)
90-113
: Review pagination termination logic.The pagination logic assumes that an empty response (
data.length === 0
) indicates no more results. While this is a common pattern, it's worth verifying that the AltTextify API follows this convention.Also consider adding error handling for cases where the API returns an unexpected response format.
async *paginate({ fn, params = {}, maxResults = null, ...opts }) { let hasMore = false; let count = 0; let page = 0; do { params.page = ++page; const data = await fn({ params, ...opts, }); + + // Handle case where API returns non-array response + if (!Array.isArray(data)) { + console.warn("API returned non-array response:", data); + break; + } + for (const d of data) { yield d; if (maxResults && ++count === maxResults) { return count; } } hasMore = data.length; } while (hasMore); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/alttextify/actions/delete-image/delete-image.mjs
(1 hunks)components/alttextify/actions/get-alttext-by-asset-id/get-alttext-by-asset-id.mjs
(1 hunks)components/alttextify/actions/get-alttext-by-job-id/get-alttext-by-job-id.mjs
(1 hunks)components/alttextify/actions/submit-image/submit-image.mjs
(1 hunks)components/alttextify/alttextify.app.mjs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/alttextify/actions/delete-image/delete-image.mjs
- components/alttextify/actions/get-alttext-by-job-id/get-alttext-by-job-id.mjs
- components/alttextify/actions/get-alttext-by-asset-id/get-alttext-by-asset-id.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 (9)
components/alttextify/alttextify.app.mjs (8)
6-26
: LGTM - Asset ID async options properly implemented.The async options implementation correctly fetches paginated asset data and maps to the expected label-value format.
28-44
: LGTM - Clean API request infrastructure.The helper methods provide a clean abstraction for API calls with proper URL construction and header management.
45-89
: LGTM - Comprehensive API method coverage.All necessary API endpoints are properly implemented with consistent parameter handling and method signatures.
90-113
: LGTM - Robust pagination implementation.The pagination logic correctly handles page incrementing, result limiting, and termination conditions.
1-2
: LGTM: Clean import statement.The axios import follows the standard Pipedream pattern.
6-26
: Good implementation of async options for asset selection.The
assetId
prop definition correctly implements async options as suggested in the previous review. The implementation properly maps API results to label-value pairs for user selection.
28-44
: LGTM: Standard helper methods pattern.The helper methods follow Pipedream conventions with proper base URL construction, header management, and request wrapping using axios.
45-89
:❓ Verification inconclusive
Verify API endpoints match the official documentation.
The API methods implement standard RESTful patterns. Please ensure these endpoints align with the AltTextify API documentation, particularly:
- Image upload endpoints (
/image/raw
and/image/url
)- Asset management endpoints (
/image/{assetId}
and/image/job/{jobId}
)
🌐 Web query:
What are the correct API endpoints for AltTextify image upload, retrieval, and deletion operations?
💡 Result:
AltText.ai provides a RESTful API that allows you to generate alternative text (alt text) for images, enhancing web accessibility and SEO. While the API documentation does not explicitly detail endpoints for image upload, retrieval, and deletion, it offers a comprehensive guide for integrating alt text generation into your applications.
Generating Alt Text for an Image:
To generate alt text for an image, you can make a POST request to the
/api/v1/images
endpoint with your API key and the image URL. Here's an example usingcurl
:curl --location 'https://alttext.ai/api/v1/images' \ --header "X-API-Key: your_api_key" \ --header "Content-Type: application/json" \ --data '{ "image": { "url": "https://www.example.com/image.jpg" } }'Replace
your_api_key
with your actual API key and provide the URL of the image you want to process. This request will return the generated alt text for the specified image. (assets.alttext.ai)Asynchronous Processing with Webhooks:
For asynchronous processing, you can submit a POST request to the
/api/v1/images
endpoint with your API key and the image URL. AltText.ai will process the image in the background and notify you via webhooks when the alt text has been generated. This approach is useful for handling large volumes of images or when you prefer non-blocking operations. (assets.alttext.ai)Integration with Other Platforms:
AltText.ai offers integrations with platforms like Etsy, allowing you to automate the generation of alt text for product images. For instance, you can set up a workflow that triggers when a new listing is updated on Etsy, sending the image to AltText.ai and updating the listing with the generated alt text. (pipedream.com)
Note:
The API documentation does not provide explicit endpoints for image upload, retrieval, or deletion. For detailed information on these operations, it's recommended to contact AltText.ai support or refer to their official API documentation.
Citations:
- 1: https://assets.alttext.ai/solutions/custom?utm_source=openai
- 2: https://assets.alttext.ai/solutions/custom?utm_source=openai
- 3: https://pipedream.com/apps/etsy/integrations/alttext-ai/generate-alt-text-with-alttext-ai-api-on-listing-updated-from-etsy-api-int_VysOnxz8?utm_source=openai
Confirm API endpoint paths with official documentation
The client methods in components/alttextify/alttextify.app.mjs are currently using paths like
/image/raw
,/image/url
,/image/{assetId}
,/image/job/{jobId}
, and/image
. However, the publicly available docs only describe a single endpoint—POST https://alttext.ai/api/v1/images
—for generating alt text, and don’t detail any raw upload, URL‐based upload, retrieval, deletion, or listing operations.Please verify and align these methods against the official AltTextify API reference or consult their support team for:
- The correct base path (e.g.
/api/v1/images
vs/image
)- Endpoints and HTTP methods for synchronous vs. asynchronous (job) processing
- Retrieval, listing, and deletion endpoints (paths, query params, expected responses)
Once the exact routes are confirmed, update each method to match the documented URL and payload structure.
components/alttextify/actions/submit-image/submit-image.mjs (1)
1-27
: LGTM - Proper file handling approach adopted.The implementation correctly uses
getFileStreamAndMetadata
which addresses the previous concern about unsafe MIME type extraction.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
I think coderabbit's suggestion about base64 encoding is correct, so I went ahead and committed it. Approved and Ready for QA!
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/alttextify/alttextify.app.mjs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
/approve |
Resolves #16964.
Summary by CodeRabbit