feat(alphavantage): add Alpha Vantage integration - #682
Conversation
|
@Agam00 is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a complete Alpha Vantage provider integration with typed schemas, 56 read-only endpoints, JSON and CSV transport, API-key authentication, error handling, symbol caching, audit logging, and test coverage. ChangesAlpha Vantage integration
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟡 Moderate · up to Technical-indicator requests can currently omit provider-required parameters, causing validation failures instead of valid results. The input contract and associated tests should be corrected before merge; the future-dated provider verification note also needs owner awareness. Sequence Diagram(s)sequenceDiagram
participant Caller
participant AlphaVantageEndpoint
participant AlphaVantageClient
participant AlphaVantageAPI
participant ErrorHandlers
Caller->>AlphaVantageEndpoint: invoke typed operation
AlphaVantageEndpoint->>AlphaVantageClient: build provider request
AlphaVantageClient->>AlphaVantageAPI: send authenticated JSON or CSV request
AlphaVantageAPI-->>AlphaVantageClient: return provider response
AlphaVantageClient->>ErrorHandlers: classify provider or transport error
AlphaVantageClient-->>AlphaVantageEndpoint: return parsed result or typed error
AlphaVantageEndpoint-->>Caller: return validated endpoint output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe Alpha Vantage plugin adds 56 read-only market-data operations across nine resource groups, API-key authentication, typed schemas, provider-specific error handling, CSV decoding, selective persistence, and test coverage.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Caller[Bound Alpha Vantage endpoint] --> Kind{Response format}
Kind -->|JSON| Shared[Shared Corsair HTTP transport]
Kind -->|CSV| Csv[Single-fetch CSV transport]
Shared --> Envelope{Provider error envelope?}
Csv --> Status{HTTP response successful?}
Status -->|No| ApiError[Sanitized ApiError]
Status -->|Yes| CsvEnvelope{JSON error envelope?}
CsvEnvelope -->|No| Parse[Parse CSV rows]
CsvEnvelope -->|Yes| ProviderError[Typed Alpha Vantage error]
Envelope -->|No| Data[Validate and return data]
Envelope -->|Yes| ProviderError
ApiError --> Handlers[Plugin error handlers]
ProviderError --> Handlers
Handlers --> Retry{Retryable rate limit?}
Retry -->|Yes| Caller
Retry -->|No| Surface[Surface classified failure]
Reviews (6): Last reviewed commit: "fix(alphavantage): cap Retry-After; requ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ✅ | |
| R2 — Tests with assertions | ✅ | |
| R3 — Description complete | ✅ | |
| R3 — Linked issue / claim | ✅ | |
| R4 — Demo video / recording | ✅ |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
|
Hey @Agam00, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
Rule Used: Every endpoint must validate inputs and outputs wi... (source) Knowledge Base Used: The provider-plugin package pattern If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/alphavantage/jest.config.cjs (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: drop the scaffold entries that this package does not use.
This package has no
tests/,plugins/, orsetup/directories, and no YAML fixtures. The extratestMatchpatterns and the YAML transform therefore never apply. Removing them shortens the config. Keep them if the plugin scaffold template requires an identical config across packages.Also applies to: 20-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/alphavantage/jest.config.cjs` around lines 5 - 10, Optionally simplify the Jest configuration by removing the unused tests/, plugins/, and setup/ testMatch patterns, along with the YAML transform entries referenced in the same config. Preserve them only if the plugin scaffold requires identical configuration across packages.
🔇 Additional comments (38)
packages/alphavantage/endpoints/shared.ts (1)
13-88: LGTM!packages/alphavantage/endpoints/logging.ts (1)
12-30: LGTM!packages/alphavantage/endpoints/persist.ts (1)
44-54: LGTM!packages/alphavantage/endpoints/indicator-series.ts (1)
18-40: LGTM!packages/alphavantage/endpoints/time-series.ts (1)
24-280: LGTM!packages/alphavantage/endpoints/market.ts (1)
13-145: LGTM!packages/alphavantage/endpoints/forex.ts (1)
8-169: LGTM!packages/alphavantage/endpoints/crypto.ts (1)
8-128: LGTM!packages/alphavantage/endpoints/economic.ts (1)
17-67: LGTM!packages/alphavantage/endpoints/commodities.ts (1)
4-42: LGTM!packages/alphavantage/endpoints/types.ts (2)
306-331: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
.refine()-wrapped input schemas are accepted by the plugin contract.
intelligenceNewsSentimentandtechnicalIndicatorare the only two input schemas that are not plainZodObjectinstances.RequiredPluginEndpointSchemas, the permissions type, and any consumer that reads.shape(tool/MCP generation, form rendering) can reject or silently mishandle a refined schema.Run the following script to check the contract and the consumers:
Also applies to: 356-398
60-94: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm the declared Zod major version supports these APIs.
.loose(), the two-argumentz.record(keySchema, valueSchema)form, andz.enum()on a readonly tuple are Zod 4 APIs. Zod 3 uses.passthrough()and a differentz.recordarity.Run the following script to confirm the declared version:
Also applies to: 100-106
packages/alphavantage/schema/database.ts (1)
20-33: LGTM!packages/alphavantage/schema/index.ts (1)
3-8: LGTM!packages/alphavantage/index.ts (2)
134-209: LGTM!Also applies to: 456-688
749-760: 🩺 Stability & Availability
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the empty-string fallback in
keyBuilder.If no key resolves,
keyBuilderreturns''. The request then goes out withapikey=, and Alpha Vantage answers HTTP 200 with anError Messagebody. The caller sees a validation error rather than a missing-credential error, and one request of the 25-per-day allowance is spent.Run the following script to check how other plugins handle an unresolved key:
packages/alphavantage/webhooks/index.ts (1)
1-1: LGTM!packages/alphavantage/webhooks/types.ts (1)
9-10: LGTM!packages/alphavantage/endpoints/index.ts (1)
1-21: LGTM!packages/corsair/core/constants.ts (1)
28-28: LGTM!Also applies to: 149-149, 277-277
packages/alphavantage/client.ts (1)
78-115: LGTM!packages/alphavantage/error-handlers.ts (2)
99-109: 🎯 Functional Correctness
⚠️ Unverified finding
Sandbox verification was unavailable.Verify handler precedence for an invalid API key.
The client throws
invalid_requestfor anyError Messagebody. An invalid key produces the messageAlpha Vantage rejected the request: the parameter apikey is invalid. That message matches AUTH_ERROR by substring, and the same error matches VALIDATION_ERROR bykind. The resulting classification depends on the order in which the runtime evaluates the handlers.Run the following script to confirm the evaluation order:
Also applies to: 126-132
32-62: LGTM!Also applies to: 68-90, 165-186
packages/alphavantage/client.test.ts (1)
56-96: LGTM!Also applies to: 98-163
packages/alphavantage/endpoints/fundamentals.ts (2)
15-38: LGTM!Also applies to: 41-56, 59-74, 77-94, 97-114, 200-217, 220-237
121-136: 🗄️ Data Integrity & Integration
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm that empty CSV results are intentional successes.
earningsCalendarandipoCalendarskip theassertNotEmptycheck that the JSON operations apply. An empty calendar is a valid provider answer, so returning[]looks correct. A CSV body that the provider rejects (for example a plain-text notice) must not decode into zero rows and then be reported as a successful empty result. Verify thatmakeAlphaVantageCsvRequestclassifies notice and error bodies before parsing.Also applies to: 170-192
packages/alphavantage/endpoints/intelligence.ts (1)
19-44: LGTM!Also applies to: 55-83, 91-108
packages/alphavantage/endpoints/technical.ts (1)
30-43: 🔒 Security & Privacy
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
extra_paramscannot setfunctionorapikey.The core parameters are spread last, so
extra_paramscannot overridesymbol,interval,time_period,series_type, ormonth.functionandapikeyare not core parameters here. The client adds them. If the client merges this query after settingapikey, a caller-suppliedextra_params.apikeycan replace the account key or produce a duplicate parameter. Confirm the merge order inmakeAlphaVantageRequest, or reject reserved keys in thetechnicalIndicatorinput schema.packages/alphavantage/endpoints.test.ts (1)
136-560: LGTM!Also applies to: 564-641, 643-700, 702-765, 767-899
packages/alphavantage/schema.test.ts (1)
13-323: LGTM!Also applies to: 325-440
packages/alphavantage/integration.test.ts (2)
1-27: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the claimed CI exclusion.
The header states that this filename matches an exclusion in
.github/workflows/pr-checks.yml. ThedescribeLiveguard already prevents live calls without a key, so CI stays safe either way. Confirm that the workflow pattern really excludesintegration.test.ts, otherwise the comment misleads later maintainers.
29-131: LGTM!packages/alphavantage/package.json (2)
16-20: 📐 Maintainability & Code Quality
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm the build script matches the other plugin packages.
rm -rf distdoes not run in a Windows shell such as PowerShell. The PR screenshot shows development on Windows. If sibling plugins use the same script, keep it for consistency. If they use a portable cleaner, align with them.
1-15: LGTM!Also applies to: 21-44
packages/alphavantage/jest.config.cjs (1)
1-4: LGTM!Also applies to: 11-19, 23-55
packages/alphavantage/tsconfig.json (2)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Exclude test and config files from the declaration build.
include: ["./**/*"]withemitDeclarationOnlyemits declarations forendpoints.test.ts,schema.test.ts,integration.test.ts, andtsup.config.tsintodist.package.jsonpublishesdist, so those declarations ship to consumers. Exclude them, unless every plugin package intentionally uses the same include list.♻️ Proposed exclude list
"include": ["./**/*"], - "exclude": ["dist", "node_modules"] + "exclude": [ + "dist", + "node_modules", + "**/*.test.ts", + "tsup.config.ts" + ],
1-16: LGTM!Also applies to: 19-20
packages/alphavantage/tsup.config.ts (1)
1-15: LGTM!
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/alphavantage/client.ts`:
- Around line 272-292: Update makeAlphaVantageCsvRequest in
packages/alphavantage/client.ts (lines 272-292) to pass a timeout-backed abort
signal to fetch and reject non-2xx responses before parseCsv, while preserving
the existing CSV error-envelope handling. Add a test in
packages/alphavantage/client.test.ts (lines 200-223) mocking a 503 text response
and assert that makeAlphaVantageCsvRequest rejects rather than returning rows.
In `@packages/alphavantage/endpoints.test.ts`:
- Around line 901-910: Update the “does not record the free-text search term”
test to mock corsair/core’s logEventFromContext near the existing imports,
capture its payload, and assert the keywords value is absent from that payload.
Remove the ineffective lastUrl assertion while preserving the existing
Market.symbolSearch invocation.
In `@packages/alphavantage/endpoints/persist.ts`:
- Around line 57-65: Update cacheSymbols to avoid sequentially awaiting every
cacheSymbol write for large symbol lists. Use bounded concurrency or an existing
bulk-upsert capability while preserving the current early return and best-effort
caching behavior; anchor the change in cacheSymbols and cacheSymbol.
In `@packages/alphavantage/endpoints/types.ts`:
- Around line 363-366: Update the indicator validation in the technical endpoint
schema to allow digits in Alpha Vantage function names such as T3, while
retaining the existing uppercase-letter and underscore constraints and error
message context.
---
Nitpick comments:
In `@packages/alphavantage/jest.config.cjs`:
- Around line 5-10: Optionally simplify the Jest configuration by removing the
unused tests/, plugins/, and setup/ testMatch patterns, along with the YAML
transform entries referenced in the same config. Preserve them only if the
plugin scaffold requires identical configuration across packages.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c77086f-879c-4ba7-9012-090ab608865b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
packages/alphavantage/client.test.tspackages/alphavantage/client.tspackages/alphavantage/endpoints.test.tspackages/alphavantage/endpoints/commodities.tspackages/alphavantage/endpoints/crypto.tspackages/alphavantage/endpoints/economic.tspackages/alphavantage/endpoints/forex.tspackages/alphavantage/endpoints/fundamentals.tspackages/alphavantage/endpoints/index.tspackages/alphavantage/endpoints/indicator-series.tspackages/alphavantage/endpoints/intelligence.tspackages/alphavantage/endpoints/logging.tspackages/alphavantage/endpoints/market.tspackages/alphavantage/endpoints/persist.tspackages/alphavantage/endpoints/shared.tspackages/alphavantage/endpoints/technical.tspackages/alphavantage/endpoints/time-series.tspackages/alphavantage/endpoints/types.tspackages/alphavantage/error-handlers.tspackages/alphavantage/index.tspackages/alphavantage/integration.test.tspackages/alphavantage/jest.config.cjspackages/alphavantage/package.jsonpackages/alphavantage/schema.test.tspackages/alphavantage/schema/database.tspackages/alphavantage/schema/index.tspackages/alphavantage/tsconfig.jsonpackages/alphavantage/tsup.config.tspackages/alphavantage/webhooks/index.tspackages/alphavantage/webhooks/types.tspackages/corsair/core/constants.ts
|
@greptile review |
|
Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/alphavantage/client.ts (1)
333-346: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe CSV path parses
Retry-Afterbut never retries.
makeAlphaVantageRequestandmakeAlphaVantageAnalyticsRequestpassALPHA_VANTAGE_RATE_LIMIT_CONFIGto the shared transport, so a 429 is retried there.makeAlphaVantageCsvRequestcallsfetchdirectly, so a 429 rejects on the first attempt and the computedretryAfteronly reaches the caller as metadata. This makes CSV operations less resilient than the JSON operations for the same rate limit.Consider a small retry loop around the
fetchcall that honorsparseRetryAfter, or document that CSV callers must handle 429 themselves.Also applies to: 411-418
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/alphavantage/client.ts` around lines 333 - 346, Update makeAlphaVantageCsvRequest to retry 429 responses using the existing ALPHA_VANTAGE_RATE_LIMIT_CONFIG and parseRetryAfter behavior, rather than returning immediately from the direct fetch path. Add a bounded retry loop around fetch that honors the server’s retry delay and preserves the existing response parsing and error behavior for non-retryable responses.packages/alphavantage/endpoints/persist.ts (1)
67-80: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the total number of cached rows, not only the concurrency.
The batch loop fixes the unbounded fan-out, but it does not bound total work.
LISTING_STATUSreturns tens of thousands of rows, socacheSymbolsstill performs aboutsymbols.length / 16sequential batch round trips inside a read-only request. The caller still waits for all of them.Since the cache is best-effort, cap the number of rows written per call, or move the write off the request path.
♻️ Proposed cap
+/** Upper bound on rows mirrored per call; the cache is best-effort. */ +const CACHE_WRITE_LIMIT = 2_000; + /** Mirrors many securities, skipping rows with no ticker. */ export async function cacheSymbols( store: EntityStore<AlphaVantageSymbolEntity> | undefined, symbols: readonly (SymbolCandidate | undefined | null)[], ) { if (!store) return; - for (let i = 0; i < symbols.length; i += CACHE_WRITE_CONCURRENCY) { - const batch = symbols.slice(i, i + CACHE_WRITE_CONCURRENCY); + const capped = symbols.slice(0, CACHE_WRITE_LIMIT); + for (let i = 0; i < capped.length; i += CACHE_WRITE_CONCURRENCY) { + const batch = capped.slice(i, i + CACHE_WRITE_CONCURRENCY); // `cacheSymbol` swallows its own failures, so no write in a batch can // reject and abandon the rest. await Promise.all(batch.map((symbol) => cacheSymbol(store, symbol))); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/alphavantage/endpoints/persist.ts` around lines 67 - 80, Update cacheSymbols to cap the total number of rows processed per invocation, in addition to the existing CACHE_WRITE_CONCURRENCY batching. Limit the input or loop to the established maximum cache-row count, preserve skipping invalid symbols and best-effort cacheSymbol behavior, and ensure callers no longer await writes for the full symbols collection.packages/alphavantage/endpoints.test.ts (1)
938-960: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winStrengthen the news audit assertion.
The current assertion checks only string absence and the presence of the
limitkey. It does not verify the event type, status, or complete payload. A future change could addtime_fromor another caller-authored field and still pass.Assert the latest mocked call directly and expect
{ limit: 5 }.Proposed test assertion
- const serialized = JSON.stringify(lastLoggedPayload()); - expect(serialized).not.toContain('AAPL'); - expect(serialized).not.toContain('TSLA'); - expect(serialized).not.toContain('earnings'); - expect(serialized).toContain('limit'); + expect(mockLogEvent).toHaveBeenLastCalledWith( + expect.anything(), + 'alphavantage.intelligence.newsSentiment', + { limit: 5 }, + 'completed', + );The expected allowlist is defined in
packages/alphavantage/endpoints/intelligence.tsLines 19-44.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/alphavantage/endpoints.test.ts` around lines 938 - 960, Strengthen the test in the “does not record the tickers or topics a news query asked for” case by asserting the latest mocked audit call directly, including its expected event type and status, with the payload exactly equal to `{ limit: 5 }`. Replace the serialized string checks with a complete-object assertion so any unapproved caller-authored fields are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/alphavantage/client.ts`:
- Around line 377-393: Redact API keys from error bodies in both production and
test coverage: in packages/alphavantage/client.ts lines 377-393, pass the
truncated CSV response text through redactApiKeyInUrl before assigning body, and
update sanitizeApiError to apply the same redaction to string body values; in
packages/alphavantage/client.test.ts lines 254-265, make the mocked body echo
the request URI and assert that body does not contain TEST_KEY.
---
Nitpick comments:
In `@packages/alphavantage/client.ts`:
- Around line 333-346: Update makeAlphaVantageCsvRequest to retry 429 responses
using the existing ALPHA_VANTAGE_RATE_LIMIT_CONFIG and parseRetryAfter behavior,
rather than returning immediately from the direct fetch path. Add a bounded
retry loop around fetch that honors the server’s retry delay and preserves the
existing response parsing and error behavior for non-retryable responses.
In `@packages/alphavantage/endpoints.test.ts`:
- Around line 938-960: Strengthen the test in the “does not record the tickers
or topics a news query asked for” case by asserting the latest mocked audit call
directly, including its expected event type and status, with the payload exactly
equal to `{ limit: 5 }`. Replace the serialized string checks with a
complete-object assertion so any unapproved caller-authored fields are rejected.
In `@packages/alphavantage/endpoints/persist.ts`:
- Around line 67-80: Update cacheSymbols to cap the total number of rows
processed per invocation, in addition to the existing CACHE_WRITE_CONCURRENCY
batching. Limit the input or loop to the established maximum cache-row count,
preserve skipping invalid symbols and best-effort cacheSymbol behavior, and
ensure callers no longer await writes for the full symbols collection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 91dc4569-6337-4b1e-99fe-d9cae499417e
📒 Files selected for processing (6)
packages/alphavantage/client.test.tspackages/alphavantage/client.tspackages/alphavantage/endpoints.test.tspackages/alphavantage/endpoints/persist.tspackages/alphavantage/endpoints/types.tspackages/alphavantage/schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/alphavantage/schema.test.ts
- packages/alphavantage/endpoints/types.ts
Lock the company cache to the 55 live OVERVIEW keys and stop extra_params from setting function, apikey, or datatype.
|
@greptile review |
Maintainer review neededAutomated rounds are exhausted. Remaining findings:
Knowledge Base Used: The provider-plugin package pattern |
github-actions
Bot
added
the
needs-maintainer
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/alphavantage/schema.test.ts (1)
453-460: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject incomplete technical-indicator requests.
The passing cases omit provider-required parameters. Alpha Vantage requires
series_typefor RSI and MACD. It requirestime_periodfor STOCHRSI.packages/alphavantage/endpoints/technical.tsforwards these missing values, so the provider receives an invalid request. (alphavantage.co)Update
AlphaVantageEndpointInputSchemas.technicalIndicatorto require parameters by indicator. Then change these cases to reject incomplete input.
packages/alphavantage/schema.test.ts#L453-L460: requireseries_typefor RSI.packages/alphavantage/schema.test.ts#L462-L468: requireseries_typefor MACD.packages/alphavantage/schema.test.ts#L482-L489: require bothtime_periodandseries_typefor STOCHRSI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/alphavantage/schema.test.ts` around lines 453 - 460, Update AlphaVantageEndpointInputSchemas.technicalIndicator to enforce indicator-specific required fields: series_type for RSI and MACD, and both time_period and series_type for STOCHRSI. In packages/alphavantage/schema.test.ts at lines 453-460, 462-468, and 482-489, change the incomplete-input cases to assert rejection rather than successful parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/alphavantage/schema/database.ts`:
- Around line 20-26: Update the provider-contract comments in database.ts for
both LISTING_STATUS and SYMBOL_SEARCH by replacing the future “live 2026-08-13”
verification claims with the actual verification date, or remove the date
annotations entirely.
---
Outside diff comments:
In `@packages/alphavantage/schema.test.ts`:
- Around line 453-460: Update
AlphaVantageEndpointInputSchemas.technicalIndicator to enforce
indicator-specific required fields: series_type for RSI and MACD, and both
time_period and series_type for STOCHRSI. In
packages/alphavantage/schema.test.ts at lines 453-460, 462-468, and 482-489,
change the incomplete-input cases to assert rejection rather than successful
parsing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b3061d0-b6ff-4f37-af47-198823f97d4e
📒 Files selected for processing (10)
packages/alphavantage/endpoints.test.tspackages/alphavantage/endpoints/fundamentals.tspackages/alphavantage/endpoints/market.tspackages/alphavantage/endpoints/persist.tspackages/alphavantage/endpoints/technical.tspackages/alphavantage/endpoints/types.tspackages/alphavantage/integration.test.tspackages/alphavantage/schema.test.tspackages/alphavantage/schema/database.tspackages/alphavantage/schema/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/alphavantage/endpoints.test.ts
- packages/alphavantage/integration.test.ts
- packages/alphavantage/endpoints/market.ts
- packages/alphavantage/schema/index.ts
- packages/alphavantage/endpoints/types.ts
- packages/alphavantage/endpoints/fundamentals.ts
|
@greptile review |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/alphavantage/endpoints/types.ts`:
- Around line 396-422: Update the indicator schema refinements in the visible
validation chain to include T3 in both the time_period and series_type
requirement lists, so validation rejects either missing parameter before
dispatch. Add schema tests covering T3 without time_period and T3 without
series_type.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fa2022e6-9bd4-49af-bea7-a0e44aded229
📒 Files selected for processing (4)
packages/alphavantage/client.test.tspackages/alphavantage/client.tspackages/alphavantage/endpoints/types.tspackages/alphavantage/schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/alphavantage/schema.test.ts
- packages/alphavantage/client.ts
|
@greptile review |
|
FIxed and LGTM Checked this against the live API. Company cache didn’t match real OVERVIEW, extra_params could override the key, and errors were leaking the apikey. CSV was retrying twice and Retry-After could hang forever one layer now, 5s cap. Also required time_period / series_type for RSI, MACD, STOCHRSI, and T3 so we don’t send broken calls. |
3 participants
Description
Adds an Alpha Vantage integration implemented against the Alpha Vantage market
data API.
Fixes #681
56 operations across 9 resource groups, each with zod input and output
schemas, a declared risk level and a description. This is the full 56-op surface
listed on corsair.dev/oss/alpha_vantage, with no additions:
timeSeriesmarketfundamentalsforexcryptocommoditieseconomicintelligencetechnicalEvery operation is a read. Alpha Vantage has no write surface at all — no
creates, no updates, no deletes — so no operation carries a
writeordestructiverisk level, and there are no triggers because the API has nowebhook, callback or streaming mechanism.
Auth
A single per-account API key passed as the
apikeyquery parameter. There is noOAuth flow and no refresh or expiry lifecycle, so this maps onto Corsair's
api_keyauth type directly andoauth_2is not offered.The thing worth reviewing: errors arrive as HTTP 200
Alpha Vantage answers every request with HTTP 200, including failures, and
signals the failure with a key in the JSON body. Status codes are never
informative.
assertNoAlphaVantageErrorinclient.tsclassifies the threebody shapes the provider actually uses and raises a typed error, so the handlers
in
error-handlers.tsmatch on an explicitkindrather than on substringguesswork:
Error MessageVALIDATION_ERRORNoteRATE_LIMIT_ERROR(retried)Informationcontaining "premium endpoint"PERMISSION_ERRORInformationotherwiseRATE_LIMIT_ERROR(barely retried)Two follow-on consequences, both verified against the live API rather than
inferred from the docs:
GLOBAL_QUOTE&symbol=ZZZZ_NOPEreturns{"Global Quote": {}}— a well-formed envelope with nothing in it. Emptinessis therefore detected per response shape in
endpoints/shared.tsand raised asan explicit not-found, rather than silently returning an empty object to the
caller.
THIS_KEY_IS_NOT_VALID_AT_ALLreturned full live data, HTTP 200. There is inpractice no auth-failure path on the query endpoint.
AUTH_ERRORis kept anddocumented as defensive so that a transport-level 401, or a future change on
the provider's side, is still classified rather than falling through to
DEFAULT— but reviewers should know it is currently unreachable rather thanassume it was tested.
Three operations return CSV, not JSON
LISTING_STATUS,EARNINGS_CALENDARandIPO_CALENDARare served asContent-Type: application/x-download. They cannot use the shared JSONtransport, so
makeAlphaVantageCsvRequestfetches them as text and decodes theminto rows. The CSV splitter handles quoted fields containing commas — Alpha
Vantage quotes company names such as
"Alphabet, Inc.", and a naivesplit(',')corrupts every row after the first one.This is the same class of problem as the
getResponseBodynote in #672: theshared request path assumes a JSON body and logs a caught
SyntaxErrorwhen itdoes not get one.
Six operations are premium-gated — including all intraday data
Verified against the live API on 2026-08-13 with a free-tier key. Each of these
answers with
{"Information": "... This is a premium endpoint ..."}andHTTP 200:
timeSeries.intradayTIME_SERIES_INTRADAYtimeSeries.intradayExtendedTIME_SERIES_INTRADAYforex.intradayFX_INTRADAYcrypto.intradayCRYPTO_INTRADAYtimeSeries.realtimeBulkQuotesREALTIME_BULK_QUOTESintelligence.historicalOptionsHISTORICAL_OPTIONSIn short: everything intraday, plus bulk quotes and the options chain. The
daily, weekly and monthly variants are all free, in every asset class. This is
not documented prominently and is easy to discover only after wiring an
operation up, so each of the six carries
[PREMIUM PLAN]in its registrydescription and a note at its handler.
They are implemented and covered by mocked tests, including a test asserting
that the premium notice is classified as
PERMISSION_ERRORand not as arate limit — both arrive as an
Informationbody and only the wording separatesthem, so confusing the two would make the client retry something that can never
succeed.
The four intraday operations return the ordinary series envelope, confirmed from
their daily and weekly siblings, so their schemas are not guesswork. The other
two are different: their shapes could not be observed at all, and for bulk quotes
the provider explicitly warns that the sample payload accompanying the notice is
artificial. Those two are the only schemas in this PR modelled from the
documentation rather than a real response, and that is stated in
types.tsatthe definitions. I did not want an invented sample payload silently becoming the
declared contract.
Other provider quirks encoded here
SECTORis deprecated upstream and now answers with an empty object. Itis implemented because the catalog lists it; the empty body is returned with a
warning rather than being reported as an error, because that is the endpoint's
actual behaviour and not a failure of the call.
TIME_SERIES_INTRADAY_EXTENDEDhas been folded intoTIME_SERIES_INTRADAYvia its
monthparameter. The operation is kept and the legacysliceargument (
year1month1…year2month12) is translated to the equivalentmonth.
COMPANY_OVERVIEW→OVERVIEW,GET_DIVIDENDS→DIVIDENDS,GET_HISTORICAL_OPTIONS→HISTORICAL_OPTIONS.GET_SLIDING_WINDOW_ANALYTICSis on a different host —alphavantageapi.co, addressed by path rather than afunctionparameter,with upper-case query parameters.
Meta Datakey punctuation is inconsistent: price series use"1. Information"(period), technical indicators use"1: Symbol"(colon),and indicator meta mixes strings with numbers. The schemas do not assume one
convention.
"185.9200"). They are kept as stringsrather than coerced, so a price is never silently altered by a float
conversion and the caller decides how to parse.
publishes both. This matches the catalog rather than adding them.
Schema design
Two shared envelopes carry most of the surface:
IndicatorSeriesSchema—{name, interval, unit, data[{date, value}]}. Allnine commodities and all ten economic indicators return exactly this, so those
19 operations are built from a single factory in
endpoints/indicator-series.tsrather than nineteen near-identical blocks.SeriesEnvelopeSchema— aMeta Datablock plus one series object whose keynames the series. Because that key varies with the request, the shape uses
catchallrather than enumerating every possible name, which still validatesthe series contents.
Only security reference data is persisted (
symbols: ticker, name, exchange,asset type, status). Everything else Alpha Vantage returns is a price or an
indicator that is stale the moment it is stored, so caching it would be actively
harmful. The symbol mapping is the identifier every other operation needs, it
changes only when a security lists or delists, and the free tier allows just 25
requests per day — so resolving a ticker from cache rather than spending a
request on
SYMBOL_SEARCHor the ~1 MBLISTING_STATUSdownload is a realsaving. Nothing is ever evicted: a delisted security is reported as
Delistedrather than disappearing, so the row stays with its status updated.
datatypeis deliberately not exposed as an input. Alpha Vantage uses it toswitch a response between JSON and CSV, and letting a caller ask for CSV on a
JSON operation would return something the declared output schema cannot
describe.
Checklist
pnpm lintand all checks passpnpm typecheckand there are no TypeScript errorspnpm buildand all packages build successfullypnpm testand all tests passScreenshots / Demos
131 tests total — 124 CI-safe + 7 live.
The 124 CI-safe tests (
client.test.ts,schema.test.ts,endpoints.test.ts)cover query construction, all three error classifications, CSV decoding
including quoted fields, all 56 endpoint wrappers and the provider function each
one calls, the premium-notice handling for the six gated operations, the
emptiness checks, and the symbol cache writes — with the network mocked.
schema.test.tsvalidates the declared schemas against payloads captured fromthe live API, trimmed for length but otherwise unedited, rather than against
payloads transcribed from the documentation. Given how inconsistent Alpha
Vantage's key naming is, that distinction is the difference between a schema
that works and one that only looks right.
integration.test.tsruns against the real API and performs genuine roundtrips: a quote, a daily series, a company overview (asserting the cache write),
a currency exchange rate, a commodity series, a symbol search, and the
unknown-ticker case that proves the empty envelope is turned into a not-found.
Seven requests, each chosen to exercise a response shape no other request
covers, paced at 1.2s — the free tier allows only 25 requests per day.
Additional Notes
integration.test.tsis named to match the CI exclusion inpr-checks.yml, soit never runs without credentials. It also self-skips when
ALPHAVANTAGE_API_KEYis absent. Run it locally with:ALPHAVANTAGE_API_KEY=<key> pnpm exec jest integrationcleanup — unlike a CRUD provider, there is no teardown to get wrong.
demo/testing/. CONTRIBUTING.md asks forthat, but R1 in PLUGIN_PR_RULES.md restricts a plugin PR to
packages/<plugin>/**, theconstants.tsregistration andpnpm-lock.yaml,so committing it would fail the scope gate. Live verification lives in
integration.test.tsinstead. Flagging again in case the two documents shouldbe reconciled.
per minute — it is easy to exhaust it and then read the resulting
Informationbody as a bug. Alpha Vantage grants unlimited access to verifiedopen-source projects on request.
Summary by CodeRabbit