-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Arxiv - new component #18990
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?
Arxiv - new component #18990
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a new arXiv search action that calls the app client to fetch articles and optionally saves results to a file. Refactors the arXiv app to centralize HTTP requests with axios. Bumps package version and adds a platform dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchAction as search-articles.mjs
participant ArxivApp as arxiv.app.mjs
participant Axios
participant ArxivAPI as arXiv API
User->>SearchAction: invoke run({ $ })
SearchAction->>ArxivApp: search({search_query, id_list, start, max_results})
ArxivApp->>ArxivApp: _baseUrl()
ArxivApp->>Axios: _makeRequest(path="/query", params...)
Axios->>ArxivAPI: HTTP GET /query?...
ArxivAPI-->>Axios: Response
Axios-->>ArxivApp: Return response
ArxivApp-->>SearchAction: search results
alt filename provided
SearchAction->>SearchAction: write file -> /tmp/{filename}
SearchAction-->>User: { filePath, fileContent } + summary
else filename not provided
SearchAction-->>User: raw response + summary
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
components/arxiv/actions/search-articles/search-articles.mjs(1 hunks)components/arxiv/arxiv.app.mjs(1 hunks)components/arxiv/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 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/arxiv/arxiv.app.mjs
📚 Learning: 2025-10-20T00:28:38.772Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack/slack.app.mjs:0-0
Timestamp: 2025-10-20T00:28:38.772Z
Learning: In components/slack/slack.app.mjs, the assistantSearch method uses sdk().apiCall() directly instead of makeRequest because the slack/web-api npm package doesn't expose assistant.search.context as a method on WebClient. This direct usage is intentional and necessary when Slack API methods are not yet exposed in the SDK.
Applied to files:
components/arxiv/arxiv.app.mjs
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/arxiv/package.json
⏰ 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)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/arxiv/package.json (1)
3-3: LGTM! Version bump and dependency addition are appropriate.The minor version bump (0.0.1 → 0.1.0) correctly reflects the addition of new functionality. The @pipedream/platform dependency is necessary for the axios usage in the updated app module.
Also applies to: 15-17
components/arxiv/arxiv.app.mjs (2)
1-1: LGTM! Correct import for HTTP requests.The axios import from @pipedream/platform is the appropriate way to make HTTP requests in Pipedream components.
8-24: LGTM! Clean HTTP request infrastructure.The implementation follows Pipedream best practices:
_baseUrl()provides the correct arXiv API endpoint_makeRequest()properly uses axios with URL compositionsearch()correctly delegates to the query endpointThe axios implementation will automatically exclude undefined parameters from requests.
components/arxiv/actions/search-articles/search-articles.mjs (1)
2-2: LGTM! Correct use of built-in fs module.The fs import is correct. Built-in Node.js modules should be imported but not added to package.json dependencies.
Based on learnings.
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 @michelle0927 lgtm! Ready for QA!
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
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
♻️ Duplicate comments (3)
components/arxiv/actions/search-articles/search-articles.mjs (3)
17-28: Add validation that at least one search parameter is required.Both
searchQueryandidListare optional, but the arXiv API requires at least one to perform a search. Without this validation, the action makes an invalid API call.Apply this diff to add validation:
async run({ $ }) { + if (!this.searchQuery && (!this.idList || this.idList.length === 0)) { + throw new Error("Either searchQuery or idList must be provided"); + } + const response = await this.arxiv.search({
47-51: Remove unusedsyncDirprop or make it optional.The
syncDirprop is required but the action only writes files conditionally (whenfilenameis provided). This forces users to provide a sync directory even when not saving files. Based on learnings, since this action doesn't always write files, the prop should either be optional or removed.If sync functionality is not needed, apply this diff to remove the prop:
- syncDir: { - type: "dir", - accessMode: "write", - sync: true, - },Otherwise, make it optional:
syncDir: { type: "dir", accessMode: "write", sync: true, + optional: true, },Based on learnings.
71-72: Add error handling for file operations.The file write operation lacks error handling. If the write fails, the error message may not be clear to users.
Apply this diff to add error handling:
const filePath = `/tmp/${this.filename}`; -fs.writeFileSync(filePath, response, "utf8"); +try { + fs.writeFileSync(filePath, response, "utf8"); +} catch (error) { + throw new Error(`Failed to write file to ${filePath}: ${error.message}`); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/arxiv/actions/search-articles/search-articles.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-01T17:07:48.193Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/zerobounce/actions/get-validation-results-file/get-validation-results-file.mjs:23-27
Timestamp: 2025-07-01T17:07:48.193Z
Learning: For "dir" props in Pipedream components, whether to mark them as optional depends on the action's file I/O behavior - if an action always writes files as output, the "dir" prop should not be marked as optional.
Applied to files:
components/arxiv/actions/search-articles/search-articles.mjs
📚 Learning: 2025-07-01T17:01:46.327Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/tinypng/actions/compress-image/compress-image.mjs:18-23
Timestamp: 2025-07-01T17:01:46.327Z
Learning: In TinyPNG compress-image action (components/tinypng/actions/compress-image/compress-image.mjs), the syncDir property uses accessMode: "read" because this action only reads input files and returns API responses without writing files to /tmp, unlike other TinyPNG actions that save processed files to disk.
Applied to files:
components/arxiv/actions/search-articles/search-articles.mjs
📚 Learning: 2025-07-01T16:56:20.177Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 17375
File: components/tinypng/actions/convert-image/convert-image.mjs:33-37
Timestamp: 2025-07-01T16:56:20.177Z
Learning: 'dir' props with sync functionality do not expose a path to the sync directory directly. For files to be synced, they must be either: (1) written to `process.env.STASH_DIR` (`/tmp/__stash`), or (2) written to `/tmp` and have their file path returned from the `run` method as a string, `[filepath]`, `[_, filepath]`, `{ filePath }`, `{ filepath }`, or `{ path }`.
Applied to files:
components/arxiv/actions/search-articles/search-articles.mjs
⏰ 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)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (3)
components/arxiv/actions/search-articles/search-articles.mjs (3)
1-2: LGTM!Imports are appropriate for the action's functionality.
4-14: LGTM!Component metadata and annotations are correctly configured for a search action.
29-46: LGTM!Pagination and file configuration props are correctly defined.
Resolves #9741
Summary by CodeRabbit
New Features
Chores