refactor: use zod-openapi for ensanalytics-api#1685
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
📝 WalkthroughWalkthroughIntroduces a new ENS Analytics API routes module defining paginated referrer leaderboard and detail endpoints. Refactors the handler from inline route definitions to a createApp-based approach using external route descriptors. Updates tests to use direct HTTP requests. Registers the new routes in the route groups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR migrates the ensanalytics-api routes to use @hono/zod-openapi for route definitions, aligning with the pattern already established in ensnode-api, amirealtime-api, and resolution-api. This enables OpenAPI spec generation from route definitions without runtime dependencies.
Changes:
- Created new
ensanalytics-api.routes.tswith zod-openapi route definitions - Refactored
ensanalytics-api.tsto usecreateApp()andapp.openapi()pattern - Updated tests to use
app.request()instead oftestClient()for compatibility with OpenAPIHono
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
apps/ensapi/src/handlers/ensanalytics-api.routes.ts |
New file defining OpenAPI routes for referrer leaderboard and detail endpoints using createRoute and zod schemas |
apps/ensapi/src/handlers/ensanalytics-api.ts |
Migrated from factory.createApp() to createApp(), replaced route decorators with app.openapi() calls, imported route definitions from routes file |
apps/ensapi/src/handlers/ensanalytics-api.test.ts |
Updated test calls from testClient(app) pattern to app.request() for OpenAPIHono compatibility |
apps/ensapi/src/stub-routes.ts |
Added ensanalytics routes to the route groups for OpenAPI spec generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryMigrated
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 037d942 |
lightwalker-eth
left a comment
There was a problem hiding this comment.
@notrab Also looks good, thanks 👍
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/handlers/ensanalytics-api.routes.ts`:
- Around line 42-49: The OpenAPI responses for the two route descriptors with
validated inputs are missing a 400 validation response; update the responses
objects in ensanalytics-api.routes.ts (the route descriptors that define
validated inputs around the referrer leaderboard and the other validated route)
to include a 400 entry that maps validation errors to a body shape like {
message, details } and mentions ZodError/standard schema validation, so
generated docs include validation error responses for both routes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/ensapi/src/handlers/ensanalytics-api.routes.tsapps/ensapi/src/handlers/ensanalytics-api.test.tsapps/ensapi/src/handlers/ensanalytics-api.tsapps/ensapi/src/stub-routes.ts
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved referrer leaderboard page", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Document 400 validation responses in both route descriptors.
Line 40 and Line 59 define validated inputs, but the responses blocks don’t include 400. This makes generated OpenAPI incomplete for client/error handling.
🛠️ Proposed update
export const getReferrerLeaderboardRoute = createRoute({
@@
responses: {
description: "Successfully retrieved referrer leaderboard page",
},
+ 400: {
+ description: "Invalid query parameters",
+ },
description: "Internal server error",
},
},
});
@@
export const getReferrerDetailRoute = createRoute({
@@
responses: {
description: "Successfully retrieved referrer detail",
},
+ 400: {
+ description: "Invalid referrer address parameter",
+ },
description: "Internal server error",
},
description: "Service unavailable - referrer leaderboard data not yet cached",As per coding guidelines: "Map errors to response codes: validation errors (ZodError/Standard Schema) → 400 with { message, details }."
Also applies to: 61-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/ensanalytics-api.routes.ts` around lines 42 - 49,
The OpenAPI responses for the two route descriptors with validated inputs are
missing a 400 validation response; update the responses objects in
ensanalytics-api.routes.ts (the route descriptors that define validated inputs
around the referrer leaderboard and the other validated route) to include a 400
entry that maps validation errors to a body shape like { message, details } and
mentions ZodError/standard schema validation, so generated docs include
validation error responses for both routes.
There was a problem hiding this comment.
@coderabbitai this was already the behaviour in the current API. We should follow up with a feature request for this.
There was a problem hiding this comment.
@notrab, that makes sense — since this PR maintains existing behavior, we can defer the 400 response documentation improvement. Would you like me to open a GitHub issue to track adding 400 validation error responses to the OpenAPI route descriptors for ensanalytics-api (and potentially other migrated routes)?
🧠 Learnings used
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-22T20:37:05.869Z
Learning: Applies to apps/{ensapi,ensadmin,ensrainbow,fallback-ensapi}/**/*.{ts,tsx} : Use existing Hono validation middleware (Zod schemas + `validate()` from `apps/ensapi/src/lib/handlers/validate.ts`) for API request validation instead of manually calling `zod.parse`/`safeParse` in route handlers
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-22T20:37:05.869Z
Learning: Applies to apps/{ensapi,ensadmin,ensrainbow,fallback-ensapi}/**/*.{ts,tsx} : Map errors to response codes: validation errors (ZodError/Standard Schema) → 400 with `{ message, details }`; known client errors → 4xx with `{ message }`; server errors → 500 with `{ message }`
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-22T20:37:05.869Z
Learning: Applies to apps/{ensapi,ensadmin,ensrainbow,fallback-ensapi}/**/*.{ts,tsx} : Use response shape `{ message: string, details?: unknown }` for API errors; do not add a `code` field inconsistently (may be adopted project-wide later)
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-22T20:37:05.869Z
Learning: Applies to apps/{ensapi,ensadmin,ensrainbow,fallback-ensapi}/**/*.{ts,tsx} : Use the shared `errorResponse` helper from `apps/ensapi/src/lib/handlers/error-response.ts` for all error responses in ENSApi and apply equivalent patterns in other Hono apps
Learnt from: notrab
Repo: namehash/ensnode PR: 1673
File: apps/ensapi/src/handlers/registrar-actions-api.ts:102-115
Timestamp: 2026-02-25T07:04:48.803Z
Learning: The registrar-actions-api endpoint (apps/ensapi/src/handlers/registrar-actions-api.ts) uses `serializeRegistrarActionsResponse` with `RegistrarActionsResponseError` and includes a `responseCode` field. This differs from the standard ENSApi `errorResponse` helper pattern to maintain backward compatibility and avoid breaking the established API contract.
Learnt from: notrab
Repo: namehash/ensnode PR: 1661
File: apps/ensapi/src/handlers/amirealtime-api.ts:41-45
Timestamp: 2026-02-22T09:10:50.106Z
Learning: In the ensnode/ensapi project, prefer consistency with existing codebase patterns for c.json() calls; explicit status codes (e.g., c.json(payload, 200)) are not required when the default behavior (200) is already consistent across the codebase.
Learnt from: notrab
Repo: namehash/ensnode PR: 1631
File: apps/ensapi/src/handlers/ensnode-api.ts:23-27
Timestamp: 2026-02-18T16:11:00.691Z
Learning: Dynamic imports inside request handlers can be acceptable if used judiciously, but they introduce asynchronous loading and potential variability in startup latency. This pattern should be reviewed on a per-file basis: prefer top-level imports for commonly used modules and reserve dynamic import for conditional loading or rare code paths. If you continue to use dynamic imports in ensnode-api.ts, ensure you profile startup and per-request latency, verify error handling for promise rejections, and document the reasoning for caching behavior.
Learnt from: Goader
Repo: namehash/ensnode PR: 1663
File: packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts:74-96
Timestamp: 2026-02-24T15:53:06.633Z
Learning: In TypeScript code reviews, prefer placing invariants on type aliases only when the invariant is context-independent or reused across multiple fields. If an invariant depends on surrounding rules or object semantics (e.g., field-specific metrics), keep the invariant as a field JSDoc instead. This guideline applies to TS files broadly (e.g., the repo's v1/award-models and similar modules).
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
Migrate
ensanalytics-apito@hono/zod-openapiroute definitions.Why
Consistent with the pattern established across other migrated routes (
ensnode-api,amirealtime-api,resolution-api), enabling OpenAPI spec generation from route definitions without runtime dependencies.Testing
Notes for Reviewer (Optional)
testClient(app)withapp.request().OpenAPIHono'sapp.openapi()doesn't produce the chained route types thattestClientneeds for type inference. Theapp.request()pattern is already used byamirealtime-api.test.tsand by the/referrers/:referrertests in this same file.Pre-Review Checklist (Blocking)