Uh oh!
There was an error while loading. Please reload this page.
feat(athena): add AWS Athena integration - #4034
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Implements eight new Athena tool endpoints under Registers the new tool configs ( Reviewed by Cursor Bugbot for commit 9cae283. Configure here. |
Greptile SummaryThis PR adds a full AWS Athena integration to Sim, exposing 8 operations (start query, get query execution, get query results, stop query, list query executions, create named query, get named query, list named queries) through a unified block UI. The implementation follows the established AWS integration pattern using Confidence Score: 5/5Safe to merge — all previously flagged P1 issues have been resolved and no new issues were found. All four P1 findings from the prior review round are confirmed fixed: the rowData variable shadowing, alphabetical ordering in tools/registry.ts, the queryString runtime guard in create_named_query, and the maxResults ceiling lowered to 999. The implementation follows established AWS integration patterns, credentials use user-only visibility, auth is checked on every route, and the Zod schemas correctly validate inputs. No remaining P0 or P1 issues detected. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/tools/athena/get-query-results/route.ts | Correctly handles header-row skipping on first page, previously flagged rowData shadowing resolved, maxResults now capped at 999 to stay within AWS hard limit of 1000. |
| apps/sim/blocks/blocks/athena.ts | Well-structured block with per-operation sub-block conditions, required guards, wandConfig for SQL generation, and explicit runtime validation for all required fields. |
| apps/sim/app/api/tools/athena/start-query/route.ts | Clean implementation; correctly builds QueryExecutionContext only when database or catalog is provided. |
| apps/sim/tools/registry.ts | Athena tools correctly inserted in alphabetical order after ashby_* entries. |
| apps/sim/tools/athena/types.ts | Complete, well-typed interfaces for all 8 operations extending AthenaConnectionConfig. |
| apps/sim/app/api/tools/athena/utils.ts | Minimal shared factory — correct pattern for reusing the Athena client across all route handlers. |
| apps/sim/tools/athena/index.ts | Barrel export correctly re-exports all 8 tools and the types module, following project conventions. |
Reviews (3): Last reviewed commit: "fix(athena): cap maxResults at 999 to pr..." | Re-trigger Greptile
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Fix variable shadowing: rename inner `data` to `rowData` in row mapper - Fix first-page maxResults off-by-one: request maxResults+1 to compensate for header row - Add missing runtime guard for queryString in create_named_query - Move athena registry entries to correct alphabetical position
- Reorder athena_* registry keys to strict alphabetical order - Add type re-exports from index.ts barrel
waleedlatif1
commented
Apr 8, 2026
waleedlatif1
commented
Apr 8, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…w adjustment The +1 adjustment for the header row on first-page requests could produce MaxResults=1001 when user requests 1000, exceeding the AWS API hard cap of 1000.
waleedlatif1
commented
Apr 8, 2026
waleedlatif1
commented
Apr 8, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9cae283. Configure here.
Summary
@aws-sdk/client-athenawith standard AWS credential auth pattern (access key + secret key + region)Type of Change
Testing
Tested manually
Checklist