Skip to content

refactor route definitions#1631

Draft
notrab wants to merge 1 commit intomainfrom
ensapi-openapi-poc
Draft

refactor route definitions#1631
notrab wants to merge 1 commit intomainfrom
ensapi-openapi-poc

Conversation

@notrab
Copy link
Member

@notrab notrab commented Feb 15, 2026

Lite PR

Tip: Review docs on the ENSNode PR process

Summary

  • Extracted inline describeRoute definitions from handler files into sibling .routes.ts files with zero config/env dependencies.
  • Added shared openapi.ts (documentation config) and routes.ts (lightweight route composition for spec generation).
  • Added scripts/generate-openapi.ts and pnpm generate:openapi script to generate the OpenAPI spec without booting the server.

Why

The OpenAPI spec was only available at runtime via GET /openapi.json, which requires a fully configured and running server. We need to generate it in CI and for docs/SDK generation without env vars or network access.


Testing

  • pnpm typecheck passes with noerrors.
  • pnpm test passes (63/63 tests)
  • pnpm generate:openapi produces valid OpenAPI 3.1.0 JSON with all 14 paths. (not committed file, will in another PR)

Notes for Reviewer (Optional)

The runtime server (index.ts) still mounts the real handler files directly. routes.ts and createRoutesForSpec() are only used by the generate script. Handler files now import their route descriptions from the .routes.ts siblings.

No changes to runtime behaviour.


Pre-Review Checklist (Blocking)

  • This PR does not introduce significant changes and is low-risk to review quickly.
  • Relevant changesets are included (or are not required)

Copilot AI review requested due to automatic review settings February 15, 2026 17:53
@changeset-bot
Copy link

changeset-bot bot commented Feb 15, 2026

⚠️ No Changeset found

Latest commit: 50c7d6f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Contributor

vercel bot commented Feb 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
admin.ensnode.io Skipped Skipped Feb 15, 2026 5:53pm
ensnode.io Skipped Skipped Feb 15, 2026 5:53pm
ensrainbow.io Skipped Skipped Feb 15, 2026 5:53pm

@coderabbitai
Copy link

coderabbitai bot commented Feb 15, 2026

📝 Walkthrough

Walkthrough

This PR refactors OpenAPI route metadata across multiple API handlers by extracting inline route descriptions into separate, centralized .routes.ts files. Each handler's route metadata is now defined as DescribeRouteOptions constants imported and used by handler files. A new openapi.ts file centralizes OpenAPI documentation configuration, and a routes.ts module is added for spec generation.

Changes

Cohort / File(s) Summary
AmI RealTime API
apps/ensapi/src/handlers/amirealtime-api.routes.ts, amirealtime-api.ts
Extracted route metadata into a new .routes.ts file with getAmIRealtimeRoute constant; handler now imports and uses this descriptor via describeRoute().
ENS Analytics v1 API
apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts, ensanalytics-api-v1.ts
Created .routes.ts file defining three route descriptors; handler updated to use them. Error handling refactored in /editions endpoint with consolidated try/catch flow.
ENS Analytics API
apps/ensapi/src/handlers/ensanalytics-api.routes.ts, ensanalytics-api.ts
Extracted two route descriptors (getReferrerLeaderboardRoute, getReferrerDetailRoute); handler replaced inline metadata with imports.
ENS Node API
apps/ensapi/src/handlers/ensnode-api.routes.ts, ensnode-api.ts
Created .routes.ts file with getConfigRoute and getIndexingStatusRoute including response schema validation; handler delegates to these descriptors.
Name Tokens API
apps/ensapi/src/handlers/name-tokens-api.routes.ts, name-tokens-api.ts
New .routes.ts file exports getNameTokensRoute with comprehensive response schemas; handler simplified by delegating OpenAPI metadata.
Registrar Actions API
apps/ensapi/src/handlers/registrar-actions-api.routes.ts, registrar-actions-api.ts
Added two route descriptors for standard and parent-node-filtered endpoints; handler uses these instead of inline metadata.
Resolution API
apps/ensapi/src/handlers/resolution-api.routes.ts, resolution-api.ts
Extracted three route descriptors with response schemas; handler simplified by removing inline schema construction.
OpenAPI Configuration
apps/ensapi/src/openapi.ts, routes.ts, index.ts
New openapi.ts centralizes OpenAPI documentation (info, servers, tags). New routes.ts module builds minimal Hono app for spec generation. index.ts updated to use centralized openapiDocumentation instead of hard-coded values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

ensanalytics, ensnode-sdk

Poem

🐰 Routes now gather in ordered files,
No scattered metadata for miles,
Descriptors dance in harmony,
OpenAPI specs flow so free,
Refactored clean, a rabbit's delight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is a template placeholder with no actual content filled in—Summary, Why, Testing, and Notes sections are all empty, and the blocking checklist items are unchecked. Complete all required sections: add a 1-3 bullet summary of changes, explain the rationale with relevant issue links, describe testing approach, and check off the blocking checklist items (or explain why they cannot be satisfied).
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refactor route definitions' accurately describes the main objective of the PR, which centralizes and externalizes route metadata across multiple handler files.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ensapi-openapi-poc

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors ENSApi’s OpenAPI route metadata so it can be shared between the runtime server and spec-generation contexts without pulling in config/env-dependent modules.

Changes:

  • Extracted describeRoute metadata into dedicated *.routes.ts modules per handler.
  • Centralized OpenAPI documentation (info/servers/tags) into src/openapi.ts and reused it in the runtime /openapi.json endpoint.
  • Added a lightweight createRoutesForSpec() route composer (src/routes.ts) that mounts only metadata + stub handlers for spec generation.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
apps/ensapi/src/routes.ts Adds spec-only Hono app composition using stub handlers and imported route metadata.
apps/ensapi/src/openapi.ts Centralizes OpenAPI documentation metadata (info/servers/tags).
apps/ensapi/src/index.ts Uses openapiDocumentation for /openapi.json, overriding version and adding localhost server.
apps/ensapi/src/handlers/resolution-api.ts Replaces inline describeRoute objects with imports from resolution-api.routes.ts.
apps/ensapi/src/handlers/resolution-api.routes.ts New module containing OpenAPI metadata + response schemas for resolution endpoints.
apps/ensapi/src/handlers/registrar-actions-api.ts Replaces inline describeRoute objects with imports from registrar-actions-api.routes.ts.
apps/ensapi/src/handlers/registrar-actions-api.routes.ts New module containing OpenAPI metadata for registrar-actions endpoints.
apps/ensapi/src/handlers/name-tokens-api.ts Replaces inline describeRoute objects with import from name-tokens-api.routes.ts.
apps/ensapi/src/handlers/name-tokens-api.routes.ts New module containing OpenAPI metadata + response schemas for name-tokens endpoint.
apps/ensapi/src/handlers/ensnode-api.ts Replaces inline describeRoute objects with imports from ensnode-api.routes.ts.
apps/ensapi/src/handlers/ensnode-api.routes.ts New module containing OpenAPI metadata + response schemas for config/indexing-status endpoints.
apps/ensapi/src/handlers/ensanalytics-api.ts Replaces inline describeRoute objects with imports from ensanalytics-api.routes.ts.
apps/ensapi/src/handlers/ensanalytics-api.routes.ts New module containing OpenAPI metadata for ENSAnalytics v0 endpoints.
apps/ensapi/src/handlers/ensanalytics-api-v1.ts Replaces inline describeRoute objects with imports from ensanalytics-api-v1.routes.ts.
apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts New module containing OpenAPI metadata for ENSAnalytics v1 endpoints.
apps/ensapi/src/handlers/amirealtime-api.ts Replaces inline describeRoute object with import from amirealtime-api.routes.ts.
apps/ensapi/src/handlers/amirealtime-api.routes.ts New module containing OpenAPI metadata for /amirealtime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Around line 4-63: The route descriptors getReferralLeaderboardV1Route,
getReferrerDetailV1Route, and getEditionConfigSetRoute currently only list
response descriptions—update each responses object to include content entries
that attach JSON schemas via validationResolver (same pattern used in
name-tokens-api.routes.ts) so the OpenAPI spec exposes response body schemas;
include the appropriate status codes (e.g., 200 success schemas and
400/404/500/503 error schemas where applicable) and reference the existing
response schema validators for leaderboard pages, referrer detail, edition
config set, and shared error shapes.

In `@apps/ensapi/src/handlers/ensnode-api.routes.ts`:
- Line 9: Replace the explicit type annotation on your route exports with a
"satisfies DescribeRouteOptions" clause to preserve literal type inference
(e.g., tag strings); specifically update getConfigRoute (and the other route
export at the second occurrence) to use "satisfies DescribeRouteOptions" instead
of ": DescribeRouteOptions" so the routes match the style used in
amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and
ensanalytics-api-v1.routes.ts.

In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Around line 10-56: Add documented error responses to each route metadata
(getResolveRecordsRoute, getResolvePrimaryNameRoute,
getResolvePrimaryNamesRoute): include common error status entries such as 400
(invalid input), 404 (not found), and 500 (internal error) under the responses
object and reference the appropriate error response schema (e.g., a shared
validationErrorSchema or genericErrorSchema via validationResolver) so the
OpenAPI spec describes possible failures as well as the 200 success case.

In `@apps/ensapi/src/handlers/resolution-api.ts`:
- Around line 21-25: The route descriptors returned by
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute, and
getResolveRecordsRoute only document a 200 response; update each descriptor to
include standard error responses (e.g., 400, 404, 500, 503) matching the style
used in name-tokens-api.routes.ts so the OpenAPI spec covers middleware
invariant failures and thrown errors; modify the response objects in those
functions to add descriptive schemas/messages for each error status and ensure
any shared error schema is reused if present.

In `@apps/ensapi/src/routes.ts`:
- Line 40: The stub handler currently uses an untyped parameter (const stub =
(c: any) => c.text("")); replace the any with a minimal Hono type: import {
Context } from "hono" and change the signature to use Context (e.g., const stub
= (c: Context) => c.text("") or declare the return as Response/Promise<Response>
if async); update the parameter name if desired and ensure the import for
Context is added so the handler is properly typed.
- Around line 42-85: The route strings in createRoutesForSpec are hard-coded and
can drift from the handler definitions; refactor by exporting path constants
from each handler module (e.g., export const CONFIG_ROUTE = "/api/config" in the
module that defines getConfigRoute, REGISTRAR_ACTIONS_ROUTE for
getRegistrarActionsRoute, AM_I_REALTIME_ROUTE for getAmIRealtimeRoute, etc.) and
import those constants into routes.ts to use in createRoutesForSpec instead of
literal strings, ensuring each handler (getConfigRoute, getIndexingStatusRoute,
getNameTokensRoute, getRegistrarActionsRoute,
getRegistrarActionsByParentNodeRoute, getResolveRecordsRoute,
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
getReferrerLeaderboardRoute, getReferrerDetailRoute,
getReferralLeaderboardV1Route, getReferrerDetailV1Route,
getEditionConfigSetRoute, getAmIRealtimeRoute) exports its path constant so both
the handler and the router share the single source of truth.

Comment on lines +4 to +63
export const getReferralLeaderboardV1Route = {
tags: ["ENSAwards"],
summary: "Get Referrer Leaderboard (v1)",
description: "Returns a paginated page from the referrer leaderboard for a specific edition",
responses: {
200: {
description: "Successfully retrieved referrer leaderboard page",
},
404: {
description: "Unknown edition slug",
},
500: {
description: "Internal server error",
},
503: {
description: "Service unavailable",
},
},
} satisfies DescribeRouteOptions;

export const getReferrerDetailV1Route = {
tags: ["ENSAwards"],
summary: "Get Referrer Detail for Editions (v1)",
description: `Returns detailed information for a specific referrer for the requested editions. Requires 1-${MAX_EDITIONS_PER_REQUEST} distinct edition slugs. All requested editions must be recognized and have cached data, or the request fails.`,
responses: {
200: {
description: "Successfully retrieved referrer detail for requested editions",
},
400: {
description: "Invalid request",
},
404: {
description: "Unknown edition slug",
},
500: {
description: "Internal server error",
},
503: {
description: "Service unavailable",
},
},
} satisfies DescribeRouteOptions;

export const getEditionConfigSetRoute = {
tags: ["ENSAwards"],
summary: "Get Edition Config Set (v1)",
description:
"Returns the currently configured referral program edition config set. Editions are sorted in descending order by start timestamp (most recent first).",
responses: {
200: {
description: "Successfully retrieved edition config set",
},
500: {
description: "Internal server error",
},
503: {
description: "Service unavailable",
},
},
} satisfies DescribeRouteOptions;
Copy link

@coderabbitai coderabbitai bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Route descriptors lack response content/schema definitions.

These three descriptors define only description for each response status, while name-tokens-api.routes.ts uses validationResolver to attach full JSON schemas. This means the generated OpenAPI spec will show response codes with descriptions but no body schema for these ENSAwards endpoints. Consider adding content entries with validationResolver for consistency and a more useful spec.

🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts` around lines 4 - 63,
The route descriptors getReferralLeaderboardV1Route, getReferrerDetailV1Route,
and getEditionConfigSetRoute currently only list response descriptions—update
each responses object to include content entries that attach JSON schemas via
validationResolver (same pattern used in name-tokens-api.routes.ts) so the
OpenAPI spec exposes response body schemas; include the appropriate status codes
(e.g., 200 success schemas and 400/404/500/503 error schemas where applicable)
and reference the existing response schema validators for leaderboard pages,
referrer detail, edition config set, and shared error shapes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notrab This is an important feedback from coderabbit. Appreciate your advice. Is this a goal for a future PR?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

makeIndexingStatusResponseSchema,
} from "@ensnode/ensnode-sdk/internal";

export const getConfigRoute: DescribeRouteOptions = {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Inconsistent typing: explicit annotation vs. satisfies.

This file uses : DescribeRouteOptions (Lines 9, 25) while amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and ensanalytics-api-v1.routes.ts use satisfies DescribeRouteOptions. The explicit annotation widens the type and loses literal inference (e.g., tag strings), whereas satisfies preserves it. Consider using one approach consistently across all route files.

Also applies to: 25-25

🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/ensnode-api.routes.ts` at line 9, Replace the
explicit type annotation on your route exports with a "satisfies
DescribeRouteOptions" clause to preserve literal type inference (e.g., tag
strings); specifically update getConfigRoute (and the other route export at the
second occurrence) to use "satisfies DescribeRouteOptions" instead of ":
DescribeRouteOptions" so the routes match the style used in
amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and
ensanalytics-api-v1.routes.ts.

Comment on lines +10 to +56
export const getResolveRecordsRoute: DescribeRouteOptions = {
tags: ["Resolution"],
summary: "Resolve ENS Records",
description: "Resolves ENS records for a given name",
responses: {
200: {
description: "Successfully resolved records",
content: {
"application/json": {
schema: validationResolver(makeResolveRecordsResponseSchema()),
},
},
},
},
};

export const getResolvePrimaryNameRoute: DescribeRouteOptions = {
tags: ["Resolution"],
summary: "Resolve Primary Name",
description: "Resolves a primary name for a given `address` and `chainId`",
responses: {
200: {
description: "Successfully resolved name",
content: {
"application/json": {
schema: validationResolver(makeResolvePrimaryNameResponseSchema()),
},
},
},
},
};

export const getResolvePrimaryNamesRoute: DescribeRouteOptions = {
tags: ["Resolution"],
summary: "Resolve Primary Names",
description: "Resolves all primary names for a given address across multiple chains",
responses: {
200: {
description: "Successfully resolved records",
content: {
"application/json": {
schema: validationResolver(makeResolvePrimaryNamesResponseSchema()),
},
},
},
},
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Only 200 responses are documented — error responses are missing from the spec.

All three resolution routes only define a 200 response. If these endpoints can return error status codes (e.g., 400 for invalid input, 404, 500), they should be documented here for a complete OpenAPI spec. This may be pre-existing, but since you're centralizing route metadata, now is a good opportunity to add them.

🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/resolution-api.routes.ts` around lines 10 - 56, Add
documented error responses to each route metadata (getResolveRecordsRoute,
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute): include common error
status entries such as 400 (invalid input), 404 (not found), and 500 (internal
error) under the responses object and reference the appropriate error response
schema (e.g., a shared validationErrorSchema or genericErrorSchema via
validationResolver) so the OpenAPI spec describes possible failures as well as
the 200 success case.

Comment on lines +21 to +25
import {
getResolvePrimaryNameRoute,
getResolvePrimaryNamesRoute,
getResolveRecordsRoute,
} from "./resolution-api.routes";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Resolution route descriptors only document the 200 response.

The extracted route descriptors for /records, /primary-name, and /primary-names (in resolution-api.routes.ts) only define a 200 response, unlike name-tokens-api.routes.ts which documents 400/404/500/503 cases. Since these handlers can throw or return errors (e.g., middleware invariant failures), consider adding error response entries to the descriptors for a more complete OpenAPI spec — even if this was pre-existing, this refactor is a good opportunity to address it.

🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/resolution-api.ts` around lines 21 - 25, The route
descriptors returned by getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
and getResolveRecordsRoute only document a 200 response; update each descriptor
to include standard error responses (e.g., 400, 404, 500, 503) matching the
style used in name-tokens-api.routes.ts so the OpenAPI spec covers middleware
invariant failures and thrown errors; modify the response objects in those
functions to add descriptive schemas/messages for each error status and ensure
any shared error schema is reused if present.

} from "./handlers/resolution-api.routes";

// Stub handler used for all routes — generateSpecs only reads describeRoute metadata
const stub = (c: any) => c.text("");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Avoid any — use a minimal typed signature for the stub handler.

Proposed fix
-const stub = (c: any) => c.text("");
+const stub = (c: { text: (s: string) => Response }) => c.text("");

Alternatively, import Context from hono and type it as (c: Context) => Response.

📝 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.

Suggested change
const stub = (c: any) => c.text("");
const stub = (c: { text: (s: string) => Response }) => c.text("");
🤖 Prompt for AI Agents
In `@apps/ensapi/src/routes.ts` at line 40, The stub handler currently uses an
untyped parameter (const stub = (c: any) => c.text("")); replace the any with a
minimal Hono type: import { Context } from "hono" and change the signature to
use Context (e.g., const stub = (c: Context) => c.text("") or declare the return
as Response/Promise<Response> if async); update the parameter name if desired
and ensure the import for Context is added so the handler is properly typed.

Comment on lines +42 to +85
export function createRoutesForSpec() {
const app = new Hono();

// /api routes (ensnode-api)
app.get("/api/config", describeRoute(getConfigRoute), stub);
app.get("/api/indexing-status", describeRoute(getIndexingStatusRoute), stub);

// /api/name-tokens routes
app.get("/api/name-tokens", describeRoute(getNameTokensRoute), stub);

// /api/registrar-actions routes
app.get("/api/registrar-actions", describeRoute(getRegistrarActionsRoute), stub);
app.get(
"/api/registrar-actions/:parentNode",
describeRoute(getRegistrarActionsByParentNodeRoute),
stub,
);

// /api/resolve routes (resolution-api)
app.get("/api/resolve/records/:name", describeRoute(getResolveRecordsRoute), stub);
app.get(
"/api/resolve/primary-name/:address/:chainId",
describeRoute(getResolvePrimaryNameRoute),
stub,
);
app.get("/api/resolve/primary-names/:address", describeRoute(getResolvePrimaryNamesRoute), stub);

// /ensanalytics routes
app.get("/ensanalytics/referrers", describeRoute(getReferrerLeaderboardRoute), stub);
app.get("/ensanalytics/referrers/:referrer", describeRoute(getReferrerDetailRoute), stub);

// /v1/ensanalytics routes
app.get(
"/v1/ensanalytics/referral-leaderboard",
describeRoute(getReferralLeaderboardV1Route),
stub,
);
app.get("/v1/ensanalytics/referrer/:referrer", describeRoute(getReferrerDetailV1Route), stub);
app.get("/v1/ensanalytics/editions", describeRoute(getEditionConfigSetRoute), stub);

// /amirealtime routes
app.get("/amirealtime", describeRoute(getAmIRealtimeRoute), stub);

return app;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the index.ts file to see how routes are mounted
cat -n apps/ensapi/src/index.ts

Repository: namehash/ensnode

Length of output: 6466


🏁 Script executed:

# Let's also check the handler files to understand the relative paths
find apps/ensapi/src -type f -name "*.ts" | grep -E "(handler|route)" | head -20

Repository: namehash/ensnode

Length of output: 1017


🏁 Script executed:

# Check the structure of the ensapi/src directory
tree -L 2 apps/ensapi/src/ 2>/dev/null || find apps/ensapi/src -type f -name "*.ts" | sed 's|[^/]*/| |g' | sort

Repository: namehash/ensnode

Length of output: 2053


🏁 Script executed:

# Check ensnode-api.routes.ts to see the relative paths
cat -n apps/ensapi/src/handlers/ensnode-api.routes.ts

Repository: namehash/ensnode

Length of output: 1728


🏁 Script executed:

# Check ensanalytics-api.routes.ts
cat -n apps/ensapi/src/handlers/ensanalytics-api.routes.ts

Repository: namehash/ensnode

Length of output: 1205


🏁 Script executed:

# Check amirealtime-api.ts (main file, not routes)
cat -n apps/ensapi/src/handlers/amirealtime-api.ts | head -100

Repository: namehash/ensnode

Length of output: 3252


🏁 Script executed:

# Check routes.ts to see the full file
cat -n apps/ensapi/src/routes.ts

Repository: namehash/ensnode

Length of output: 3809


🏁 Script executed:

# Check the actual route definitions in ensnode-api.ts
cat -n apps/ensapi/src/handlers/ensnode-api.ts | head -80

Repository: namehash/ensnode

Length of output: 2525


🏁 Script executed:

# Check ensanalytics-api.ts
cat -n apps/ensapi/src/handlers/ensanalytics-api.ts | head -100

Repository: namehash/ensnode

Length of output: 4356


🏁 Script executed:

# Check registrar-actions-api.ts
cat -n apps/ensapi/src/handlers/registrar-actions-api.ts | head -80

Repository: namehash/ensnode

Length of output: 3136


🏁 Script executed:

# Check if there are any tests that verify route synchronization
find apps/ensapi -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "route" 2>/dev/null | head -5

Repository: namehash/ensnode

Length of output: 108


🏁 Script executed:

# Let me check ensanalytics-api.ts for the referrer detail route
cat -n apps/ensapi/src/handlers/ensanalytics-api.ts | sed -n '100,150p'

Repository: namehash/ensnode

Length of output: 2127


🏁 Script executed:

# Check the openapi.ts file to understand how routes.ts is used
cat -n apps/ensapi/src/openapi.ts

Repository: namehash/ensnode

Length of output: 1541


🏁 Script executed:

# Check name-tokens-api.ts for actual routes
cat -n apps/ensapi/src/handlers/name-tokens-api.ts

Repository: namehash/ensnode

Length of output: 7330


🏁 Script executed:

# Check resolution-api.ts for actual routes
cat -n apps/ensapi/src/handlers/resolution-api.ts

Repository: namehash/ensnode

Length of output: 7354


🏁 Script executed:

# Check ensanalytics-api-v1.ts
cat -n apps/ensapi/src/handlers/ensanalytics-api-v1.ts

Repository: namehash/ensnode

Length of output: 16290


🏁 Script executed:

# Check registrar-actions-api.ts for the parent node route
cat -n apps/ensapi/src/handlers/registrar-actions-api.ts | sed -n '80,150p'

Repository: namehash/ensnode

Length of output: 2448


🏁 Script executed:

# Check registrar-actions-api.ts for the parent node route definition
cat -n apps/ensapi/src/handlers/registrar-actions-api.ts | grep -A 20 "parentNode"

Repository: namehash/ensnode

Length of output: 3883


🏁 Script executed:

# Final check: verify if there are any integration or end-to-end tests that validate route synchronization
find apps/ensapi -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "route\|path" 2>/dev/null

Repository: namehash/ensnode

Length of output: 108


🏁 Script executed:

# Check the test file to see if it validates routes
cat -n apps/ensapi/src/handlers/amirealtime-api.test.ts

Repository: namehash/ensnode

Length of output: 9718


Route paths are duplicated across three locations—consider exporting path constants from handlers.

Every route path in routes.ts (e.g., /api/config, /amirealtime) is manually constructed by combining the mount points from index.ts with the relative routes defined in handler files. There's no compile-time or runtime check ensuring these stay synchronized. If a path changes in a handler or mount point in index.ts without updating routes.ts, the OpenAPI spec will silently diverge from the actual API.

Consider exporting path constants from handler modules (e.g., export const REGISTRAR_ACTIONS_ROUTE = "/registrar-actions") and consuming them in both the handler files and routes.ts, or adding a build-time or runtime validation to enforce the sync.

🤖 Prompt for AI Agents
In `@apps/ensapi/src/routes.ts` around lines 42 - 85, The route strings in
createRoutesForSpec are hard-coded and can drift from the handler definitions;
refactor by exporting path constants from each handler module (e.g., export
const CONFIG_ROUTE = "/api/config" in the module that defines getConfigRoute,
REGISTRAR_ACTIONS_ROUTE for getRegistrarActionsRoute, AM_I_REALTIME_ROUTE for
getAmIRealtimeRoute, etc.) and import those constants into routes.ts to use in
createRoutesForSpec instead of literal strings, ensuring each handler
(getConfigRoute, getIndexingStatusRoute, getNameTokensRoute,
getRegistrarActionsRoute, getRegistrarActionsByParentNodeRoute,
getResolveRecordsRoute, getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
getReferrerLeaderboardRoute, getReferrerDetailRoute,
getReferralLeaderboardV1Route, getReferrerDetailV1Route,
getEditionConfigSetRoute, getAmIRealtimeRoute) exports its path constant so both
the handler and the router share the single source of truth.

Copy link
Member

@lightwalker-eth lightwalker-eth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notrab Sharing some preliminary feedback 😄

@@ -0,0 +1,18 @@
import type { DescribeRouteOptions } from "hono-openapi";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of calling each of these files a ".routes.ts" file we call it something like ".meta.ts" since I understand each of these is metadata about the route, not the route implementation itself.

@@ -0,0 +1,18 @@
import type { DescribeRouteOptions } from "hono-openapi";

export const getAmIRealtimeRoute = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const getAmIRealtimeRoute = {
export const amIRealtimeGetMeta = {

Goals:

  1. When get... is the prefix it sounds like it's a getter function, when this is a const value rather than a function.
  2. Appreciate how there's a desire to identify which HTTP action this metadata is for. Therefore suggested moving "...Get..." to later in the naming pattern.
  3. Renamed "Route" to "Meta" because this isn't the route itself, it's just metadata about the route.

If this makes sense please apply these patterns across all the updates in this PR 👍

Comment on lines +4 to +63
export const getReferralLeaderboardV1Route = {
tags: ["ENSAwards"],
summary: "Get Referrer Leaderboard (v1)",
description: "Returns a paginated page from the referrer leaderboard for a specific edition",
responses: {
200: {
description: "Successfully retrieved referrer leaderboard page",
},
404: {
description: "Unknown edition slug",
},
500: {
description: "Internal server error",
},
503: {
description: "Service unavailable",
},
},
} satisfies DescribeRouteOptions;

export const getReferrerDetailV1Route = {
tags: ["ENSAwards"],
summary: "Get Referrer Detail for Editions (v1)",
description: `Returns detailed information for a specific referrer for the requested editions. Requires 1-${MAX_EDITIONS_PER_REQUEST} distinct edition slugs. All requested editions must be recognized and have cached data, or the request fails.`,
responses: {
200: {
description: "Successfully retrieved referrer detail for requested editions",
},
400: {
description: "Invalid request",
},
404: {
description: "Unknown edition slug",
},
500: {
description: "Internal server error",
},
503: {
description: "Service unavailable",
},
},
} satisfies DescribeRouteOptions;

export const getEditionConfigSetRoute = {
tags: ["ENSAwards"],
summary: "Get Edition Config Set (v1)",
description:
"Returns the currently configured referral program edition config set. Editions are sorted in descending order by start timestamp (most recent first).",
responses: {
200: {
description: "Successfully retrieved edition config set",
},
500: {
description: "Internal server error",
},
503: {
description: "Service unavailable",
},
},
} satisfies DescribeRouteOptions;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notrab This is an important feedback from coderabbit. Appreciate your advice. Is this a goal for a future PR?

},
}),
describeRoute(getReferralLeaderboardV1Route),
validate("query", referrerLeaderboardPageQuerySchema),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should move things like referrerLeaderboardPageQuerySchema into these new separate metadata files? Appreciate your advice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants