-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Components] finnhub #10909 #19105
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?
[Components] finnhub #10909 #19105
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughThe changes introduce new Finnhub integration actions and supporting infrastructure. Four new action modules are added for getting insider transactions, market news, recommendation trends, and symbols. A constants module provides data for news categories and exchanges. The core app module is enhanced with prop definitions and API methods using a new internal HTTP client pattern with authentication via headers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs(1 hunks)components/finnhub/actions/get-market-news/get-market-news.mjs(1 hunks)components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs(1 hunks)components/finnhub/actions/get-symbols/get-symbols.mjs(1 hunks)components/finnhub/common/constants.mjs(1 hunks)components/finnhub/finnhub.app.mjs(1 hunks)components/finnhub/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
components/finnhub/actions/get-symbols/get-symbols.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/finnhub/package.json
📚 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/finnhub/finnhub.app.mjs
🧬 Code graph analysis (4)
components/finnhub/actions/get-symbols/get-symbols.mjs (2)
components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs (1)
response(36-43)components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs (1)
response(24-29)
components/finnhub/actions/get-market-news/get-market-news.mjs (3)
components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs (1)
response(36-43)components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs (1)
response(24-29)components/finnhub/actions/get-symbols/get-symbols.mjs (1)
response(36-43)
components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs (3)
components/finnhub/actions/get-market-news/get-market-news.mjs (1)
response(24-29)components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs (1)
response(24-29)components/finnhub/actions/get-symbols/get-symbols.mjs (1)
response(36-43)
components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs (2)
components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs (1)
response(36-43)components/finnhub/actions/get-symbols/get-symbols.mjs (1)
response(36-43)
⏰ 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: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (3)
components/finnhub/package.json (1)
3-3: LGTM!The version bump to 0.1.0 is appropriate for the new actions being added.
components/finnhub/finnhub.app.mjs (1)
51-69: LGTM!The
_makeRequesthelper method is well-structured with proper authentication via theX-Finnhub-Tokenheader and clean axios integration.components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs (1)
44-44: Remove review comment - code is correct per Finnhub API specification.The Finnhub Insider Transactions API returns a response object with structure
{symbol: string, data: [transactions]}. The code correctly accessesresponse.data.length. This is not an inconsistency—different Finnhub endpoints have different response structures. The get-market-news, get-symbols, and get-recommendation-trends endpoints likely return different structures than insider transactions. The current implementation is correct and requires no changes.Likely an incorrect or invalid review comment.
| export default { | ||
| key: "finnhub-get-insider-transactions", | ||
| name: "Get Insider Transactions", | ||
| description: "Get a list os insider transactions from a specified time period. [See the documentation](https://finnhub.io/docs/api/insider-transactions)", |
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 typo in description.
"Get a list os insider transactions" should be "Get a list of insider transactions".
Apply this diff:
- description: "Get a list os insider transactions from a specified time period. [See the documentation](https://finnhub.io/docs/api/insider-transactions)",
+ description: "Get a list of insider transactions from a specified time period. [See the documentation](https://finnhub.io/docs/api/insider-transactions)",📝 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.
| description: "Get a list os insider transactions from a specified time period. [See the documentation](https://finnhub.io/docs/api/insider-transactions)", | |
| description: "Get a list of insider transactions from a specified time period. [See the documentation](https://finnhub.io/docs/api/insider-transactions)", |
🤖 Prompt for AI Agents
In
components/finnhub/actions/get-insider-transactions/get-insider-transactions.mjs
around line 6, the description string has a typo ("Get a list os insider
transactions..."); change "os" to "of" so the description reads "Get a list of
insider transactions from a specified time period. [See the
documentation](https://finnhub.io/docs/api/insider-transactions)" and save the
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.
Looks like a good suggestion
| const response = await this.app.getMarketNews({ | ||
| $, | ||
| data: { | ||
| category: this.category, | ||
| }, | ||
| }); |
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.
🧩 Analysis chain
Verify parameter passing pattern.
This action uses data property to pass the category parameter, while other actions (get-insider-transactions, get-symbols, get-recommentadion-trends) use params. Ensure this is intentional and matches the Finnhub API requirements for the /news endpoint.
Run this script to check the API documentation pattern:
🏁 Script executed:
#!/bin/bash
# Check how parameters are passed in other action files
echo "=== Checking parameter passing patterns ==="
rg -A5 "await this\.app\." components/finnhub/actions/Length of output: 2326
I need to verify if this parameter passing difference is intentional by checking how the app methods are implemented.
Also, let me search for the app definition or method signatures:
Additionally, let me check the Finnhub API documentation context:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check how the app methods are defined in the app/actions.mjs or similar
echo "=== Searching for app method implementations ==="
find components/finnhub -name "*.mjs" -type f | head -20 | xargs grep -l "getMarketNews\|getSymbols\|getInsiderTransactions" | grep -v actions
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Look for where these methods are defined
echo "=== Searching for method definitions ==="
rg -n "getMarketNews|getSymbols|getInsiderTransactions" --type mjs -B2 -A10 | grep -E "(^[^-]|function|async|params|data)" | head -50Length of output: 68
🌐 Web query:
Finnhub API /news endpoint parameters
💡 Result:
Summary — GET /api/v1/news parameters
- category (optional) — news category value (examples: general, forex, crypto, merger). [1]
- minId (optional) — integer ID; return only articles with id > minId (default 0). [1]
- token / api_key (required) — your Finnhub API token for authentication. [2]
Example request:
GET https://finnhub.io/api/v1/news?category=general&minId=0&token=YOUR_API_KEY. [1][2]
Sources: [1] Finnhub API docs / client library reference (market_news: category, minId). [2] Finnhub examples showing /news usage.
---
🏁 Script executed:
```shell
#!/bin/bash
# View the app method implementations
echo "=== Viewing finnhub.app.mjs methods ==="
rg -A15 "getMarketNews|getSymbols|getInsiderTransactions" components/finnhub/finnhub.app.mjs
Length of output: 618
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the _makeRequest method to understand how it handles params vs data
echo "=== Viewing _makeRequest method ==="
rg -A30 "_makeRequest" components/finnhub/finnhub.app.mjs | head -60
Length of output: 957
Change data to params in get-market-news.mjs (lines 24-29).
The /news endpoint requires query parameters, not a request body. The axios library interprets data as request body and params as query string parameters. All other Finnhub API endpoints in this codebase correctly use params, and the Finnhub documentation confirms /news accepts query parameters (category, minId).
const response = await this.app.getMarketNews({
$,
params: {
category: this.category,
},
});
🤖 Prompt for AI Agents
In components/finnhub/actions/get-market-news/get-market-news.mjs around lines
24 to 29, the call to this.app.getMarketNews incorrectly sends query values
under a request body key `data`; change the payload key from `data` to `params`
so axios will serialize those values as query string parameters (e.g., include
category and any other query fields under `params`) and ensure behavior matches
other Finnhub endpoints and the API docs.
| category: this.category, | ||
| }, | ||
| }); | ||
| $.export("$summary", "Successfully retrieved " + response.length + " market news"); |
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.
🧹 Nitpick | 🔵 Trivial
Use template literal for summary message.
For consistency with best practices, use a template literal instead of string concatenation.
Apply this diff:
- $.export("$summary", "Successfully retrieved " + response.length + " market news");
+ $.export("$summary", `Successfully retrieved ${response.length} market news`);Based on learnings.
📝 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.
| $.export("$summary", "Successfully retrieved " + response.length + " market news"); | |
| $.export("$summary", `Successfully retrieved ${response.length} market news`); |
🤖 Prompt for AI Agents
In components/finnhub/actions/get-market-news/get-market-news.mjs around line
30, the exported summary uses string concatenation ("Successfully retrieved " +
response.length + " market news"); replace this with a template literal to
follow best practices—use backticks and ${response.length} to build the string,
preserving the same wording and spacing.
| key: "finnhub-get-recommentadion-trends", | ||
| name: "Get Recommentadion Trends", |
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 typo in action key and name.
"Recommentadion" should be "Recommendation" throughout. This affects the action key (line 4), name (line 5), the method call (line 24), and ideally the filename and directory name.
Apply this diff:
- key: "finnhub-get-recommentadion-trends",
- name: "Get Recommentadion Trends",
+ key: "finnhub-get-recommendation-trends",
+ name: "Get Recommendation Trends",Also update the method call on line 24:
- const response = await this.app.getRecommentadionTrends({
+ const response = await this.app.getRecommendationTrends({And rename the directory from get-recommentadion-trends to get-recommendation-trends.
📝 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.
| key: "finnhub-get-recommentadion-trends", | |
| name: "Get Recommentadion Trends", | |
| key: "finnhub-get-recommendation-trends", | |
| name: "Get Recommendation Trends", |
🤖 Prompt for AI Agents
In
components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs
around lines 4-5 (and also affecting line 24), fix the misspelling
"Recommentadion" to "Recommendation": update the action key to
"finnhub-get-recommendation-trends", update the action name to "Get
Recommendation Trends", and change the method call on line 24 to the corrected
name; then rename the directory and file from get-recommentadion-trends to
get-recommendation-trends (and update any import/path references accordingly).
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 here
| symbol: this.symbol, | ||
| }, | ||
| }); | ||
| $.export("$summary", "Successfully retrieved " + response.length + " recommendation trends"); |
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.
🧹 Nitpick | 🔵 Trivial
Use template literal for summary message.
For consistency with best practices, use a template literal instead of string concatenation.
Apply this diff:
- $.export("$summary", "Successfully retrieved " + response.length + " recommendation trends");
+ $.export("$summary", `Successfully retrieved ${response.length} recommendation trends`);Based on learnings.
📝 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.
| $.export("$summary", "Successfully retrieved " + response.length + " recommendation trends"); | |
| $.export("$summary", `Successfully retrieved ${response.length} recommendation trends`); |
🤖 Prompt for AI Agents
In
components/finnhub/actions/get-recommentadion-trends/get-recommentadion-trends.mjs
around line 30, the summary message uses string concatenation; replace it with a
template literal to improve consistency and readability — update the $.export
call to use a backtick-delimited template like `Successfully retrieved
${response.length} recommendation trends`.
| mic: this.mic, | ||
| }, | ||
| }); | ||
| $.export("$summary", "Successfully retrieved " + response.length + " symbols"); |
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.
🧹 Nitpick | 🔵 Trivial
Use template literal for summary message.
For consistency with best practices, use a template literal instead of string concatenation.
Apply this diff:
- $.export("$summary", "Successfully retrieved " + response.length + " symbols");
+ $.export("$summary", `Successfully retrieved ${response.length} symbols`);Based on learnings.
📝 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.
| $.export("$summary", "Successfully retrieved " + response.length + " symbols"); | |
| $.export("$summary", `Successfully retrieved ${response.length} symbols`); |
🤖 Prompt for AI Agents
In components/finnhub/actions/get-symbols/get-symbols.mjs around line 44, the
summary uses string concatenation ("Successfully retrieved " + response.length +
" symbols"); replace it with a template literal that interpolates
response.length (e.g., `\`Successfully retrieved ${response.length} symbols\``)
to follow best practices and maintain consistency.
| ], | ||
| EXCHANGES: [ | ||
| { | ||
| label: "ADU DHABI SECURITIES EXCHANGE", |
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 typo in exchange name.
"ADU DHABI" should be "ABU DHABI".
Apply this diff:
- label: "ADU DHABI SECURITIES EXCHANGE",
+ label: "ABU DHABI SECURITIES EXCHANGE",📝 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.
| label: "ADU DHABI SECURITIES EXCHANGE", | |
| label: "ABU DHABI SECURITIES EXCHANGE", |
🤖 Prompt for AI Agents
In components/finnhub/common/constants.mjs around line 10, the exchange label
has a typo "ADU DHABI SECURITIES EXCHANGE"; update the string to the correct
name "ABU DHABI SECURITIES EXCHANGE" by replacing "ADU" with "ABU" so the label
reads exactly "ABU DHABI SECURITIES EXCHANGE".
| to: { | ||
| type: "string", | ||
| label: "To", | ||
| description: "Initial date to get insider transactions from, i.e.: `2020-03-15`", | ||
| optional: true, | ||
| }, |
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 incorrect description for "to" prop.
Line 22 says "Initial date to get insider transactions from" but this is the "to" field (end date), not the "from" field (initial date).
Apply this diff:
to: {
type: "string",
label: "To",
- description: "Initial date to get insider transactions from, i.e.: `2020-03-15`",
+ description: "End date to get insider transactions to, i.e.: `2020-03-15`",
optional: true,
},📝 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.
| to: { | |
| type: "string", | |
| label: "To", | |
| description: "Initial date to get insider transactions from, i.e.: `2020-03-15`", | |
| optional: true, | |
| }, | |
| to: { | |
| type: "string", | |
| label: "To", | |
| description: "End date to get insider transactions to, i.e.: `2020-03-15`", | |
| optional: true, | |
| }, |
🤖 Prompt for AI Agents
In components/finnhub/finnhub.app.mjs around lines 19 to 24, the description for
the "to" property incorrectly says "Initial date to get insider transactions
from"; update that description to indicate this is the end date (e.g., "End date
to get insider transactions to, i.e.: `2020-03-15`" or "End date for retrieving
insider transactions, i.e.: `2020-03-15`") by replacing the existing description
string accordingly.
| async getRecommentadionTrends(args = {}) { | ||
| return this._makeRequest({ | ||
| path: "/stock/recommendation", | ||
| ...args, | ||
| }); |
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 typo in method name.
"getRecommentadionTrends" should be "getRecommendationTrends". This must be fixed in sync with the action file that calls it.
Apply this diff:
- async getRecommentadionTrends(args = {}) {
+ async getRecommendationTrends(args = {}) {
return this._makeRequest({
path: "/stock/recommendation",
...args,
});
},📝 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.
| async getRecommentadionTrends(args = {}) { | |
| return this._makeRequest({ | |
| path: "/stock/recommendation", | |
| ...args, | |
| }); | |
| async getRecommendationTrends(args = {}) { | |
| return this._makeRequest({ | |
| path: "/stock/recommendation", | |
| ...args, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
In components/finnhub/finnhub.app.mjs around lines 83 to 87, the method name
getRecommentadionTrends is misspelled and should be renamed to
getRecommendationTrends; rename the method declaration to
getRecommendationTrends and update all references/call sites (including the
action file that calls it) to use the corrected name so exports/uses remain in
sync, keeping the method body and parameters unchanged.
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 here
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 @lcaresia There was a few suggestions other than that lgtm! Ready for QA!
| export default { | ||
| key: "finnhub-get-insider-transactions", | ||
| name: "Get Insider Transactions", | ||
| description: "Get a list os insider transactions from a specified time period. [See the documentation](https://finnhub.io/docs/api/insider-transactions)", |
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 like a good suggestion
| async getRecommentadionTrends(args = {}) { | ||
| return this._makeRequest({ | ||
| path: "/stock/recommendation", | ||
| ...args, | ||
| }); |
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 here
| key: "finnhub-get-recommentadion-trends", | ||
| name: "Get Recommentadion Trends", |
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 here
WHY
Summary by CodeRabbit
Version updated to 0.1.0