Conversation
🦋 Changeset detectedLatest commit: 332848b The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant ENSApi as ENSApi Server
participant ConfigSchema as Config Schema
participant EnsDbReader as EnsDbReader
participant PostgreSQL as PostgreSQL Database
Client->>ENSApi: GET /api/config
ENSApi->>ConfigSchema: buildEnsApiPublicConfig()
ConfigSchema->>EnsDbReader: getEnsDbPublicConfig()
EnsDbReader->>PostgreSQL: SELECT current_setting('server_version')
PostgreSQL-->>EnsDbReader: server_version string
EnsDbReader->>EnsDbReader: validateEnsDbPublicConfig()<br/>(postgresVersion + ENSDB_ROOT_SCHEMA_VERSION)
EnsDbReader-->>ConfigSchema: EnsDbPublicConfig
ConfigSchema->>ConfigSchema: merge into EnsApiPublicConfig
ConfigSchema-->>ENSApi: EnsApiPublicConfig (with ensDbPublicConfig)
ENSApi-->>Client: JSON response with ensDbPublicConfig
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
@greptile review |
Greptile SummaryThis PR introduces Confidence Score: 5/5Safe to merge — all layers are consistent, tests cover the happy and error paths, and previously raised concerns are resolved. The implementation is clean, follows established patterns, is well-tested, and has no logic bugs. All three packages have changesets. The only prior concerns (missing ensdb-sdk changeset, literal version string in test, dev-script env override) are addressed in this revision. The one acknowledged gap (no pRetry on getEnsDbPublicConfig) is deliberately deferred with a TODO and is low-risk because the DB connection is already proven by the prior retry loop. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant ENSApi as ENSApi
participant ConfigSchema as Config Builder
participant EnsDbReader as EnsDbReader
participant PG as PostgreSQL
ENSApi->>ConfigSchema: buildConfigFromEnvironment
ConfigSchema->>EnsDbReader: getEnsIndexerPublicConfig with pRetry
EnsDbReader->>PG: query metadata table
PG-->>EnsDbReader: config row
EnsDbReader-->>ConfigSchema: EnsIndexerPublicConfig
ConfigSchema->>EnsDbReader: getEnsDbPublicConfig
EnsDbReader->>PG: SELECT current_setting server version
PG-->>EnsDbReader: version string
EnsDbReader->>EnsDbReader: parse and validate
EnsDbReader-->>ConfigSchema: EnsDbPublicConfig
ConfigSchema-->>ENSApi: EnsApiConfig
Note over ENSApi: GET /api/config request
ENSApi->>ConfigSchema: buildEnsApiPublicConfig
ConfigSchema-->>ENSApi: EnsApiPublicConfig with ensDbPublicConfig field
Reviews (3): Last reviewed commit: "Update OpenAPI spec for `/api/config` ro..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds an ENSDb “public config” surface and wires it into ENSApi’s public config so downstream consumers (e.g., ENSAdmin) can display Postgres + ENSDb root schema version information.
Changes:
- Introduced
EnsDbPublicConfigmodel + validation in@ensnode/ensnode-sdk, and embedded it intoEnsApiPublicConfig(including serialization + tests). - Added
ENSDB_ROOT_SCHEMA_VERSIONto@ensnode/ensdb-sdkand implementedEnsDbReader.getEnsDbPublicConfig()(with unit tests). - Updated ENSApi config building to fetch and include
ensDbPublicConfig.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/internal.ts | Re-exports ENSDb public config zod schema via the internal entrypoint. |
| packages/ensnode-sdk/src/index.ts | Exposes the new ensdb module from the SDK top-level. |
| packages/ensnode-sdk/src/ensdb/zod-schemas/ensdb-public-config.ts | Adds zod schema for ENSDb public config. |
| packages/ensnode-sdk/src/ensdb/validate/ensdb-public-config.ts | Adds runtime validation helper for ENSDb public config. |
| packages/ensnode-sdk/src/ensdb/index.ts | Barrel export for the new ENSDb public config domain model + validator. |
| packages/ensnode-sdk/src/ensdb/ensdb-public-config.ts | Defines the EnsDbPublicConfig interface. |
| packages/ensnode-sdk/src/ensapi/config/zod-schemas.ts | Extends ENSApi public config schemas to include ensDbPublicConfig. |
| packages/ensnode-sdk/src/ensapi/config/types.ts | Extends EnsApiPublicConfig type with ensDbPublicConfig. |
| packages/ensnode-sdk/src/ensapi/config/serialize.ts | Includes ensDbPublicConfig in serialization output. |
| packages/ensnode-sdk/src/ensapi/config/conversions.test.ts | Updates config serialization/deserialization tests for new field. |
| packages/ensnode-sdk/src/ensapi/client.test.ts | Updates client config-response fixture for new field. |
| packages/ensdb-sdk/src/lib/drizzle.ts | Renames internal schema builder to “root schema” for clarity. |
| packages/ensdb-sdk/src/index.ts | Exports the new config module from the package entrypoint. |
| packages/ensdb-sdk/src/config/index.ts | Introduces ENSDB_ROOT_SCHEMA_VERSION constant. |
| packages/ensdb-sdk/src/client/ensdb-reader.ts | Adds getEnsDbPublicConfig() and a Postgres version query helper. |
| packages/ensdb-sdk/src/client/ensdb-reader.test.ts | Adds unit tests for getEnsDbPublicConfig() (mocking execute). |
| apps/ensapi/src/config/config.schema.ts | Fetches ensDbPublicConfig during startup and includes it in public config. |
| apps/ensapi/src/config/config.schema.test.ts | Updates config-building tests to include ensDbPublicConfig. |
| apps/ensapi/package.json | Changes dev script to set ENSINDEXER_SCHEMA_NAME. |
| .changeset/heavy-laws-sing.md | Adds a changeset for the new SDK data model. |
Comments suppressed due to low confidence (1)
packages/ensnode-sdk/src/ensapi/config/zod-schemas.ts:48
- Same backwards-compatibility concern as above: the serialized public config schema also requires
ensDbPublicConfig, so older ENSApi responses will fail validation. Consider making it optional or version-gating the schema.
return z.object({
version: z.string().min(1, `${label}.version must be a non-empty string`),
theGraphFallback: TheGraphFallbackSchema,
ensDbPublicConfig: schemaEnsDbPublicConfig,
ensIndexerPublicConfig: makeSerializedEnsIndexerPublicConfigSchema(
`${label}.ensIndexerPublicConfig`,
),
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return z.object({ | ||
| version: z.string().min(1, `${label}.version must be a non-empty string`), | ||
| theGraphFallback: TheGraphFallbackSchema, | ||
| ensDbPublicConfig: schemaEnsDbPublicConfig, | ||
| ensIndexerPublicConfig: makeEnsIndexerPublicConfigSchema(`${label}.ensIndexerPublicConfig`), | ||
| }); |
There was a problem hiding this comment.
ensDbPublicConfig is added as a required field in the public config schema. This will cause deserializeEnsApiPublicConfig() to reject responses from older ENSApi servers that don't include this field, which can break SDK users talking to older deployments. Consider making this field optional (and/or providing a default) for backwards compatibility, or bump the SDK major version if strictness is intended.
Set `ENSINDEXER_SCHEMA_NAME` to `public` as that is the most common schema name to use while working on ENSNode development.
tk-o
force-pushed
the
feat/1833-ensdb-sdk-enhancements
branch
from
9cb4deb to
96ebaca
Compare
…to include `ensDbPublicConfig` field.
tk-o
force-pushed
the
feat/1833-ensdb-sdk-enhancements
branch
from
8ad6501 to
5f34c2c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@packages/ensdb-sdk/src/lib/drizzle.ts`:
- Around line 122-131: The JSDoc for buildEnsDbRootSchema contains a redundant
`@returns` line that repeats the summary; remove the `@returns` tag (or its text) so
the docblock only documents anything new beyond the one-line summary. Locate the
JSDoc immediately above the buildEnsDbRootSchema function and delete the
redundant "@returns The ENSDb Root Schema definition for use in building a
Drizzle client for ENSDb." entry, leaving any necessary param/type tags intact.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c456a169-af79-450e-ba0b-06b5b94cc3c9
📒 Files selected for processing (21)
.changeset/heavy-laws-sing.md.changeset/social-dryers-find.mdapps/ensapi/package.jsonapps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.schema.tspackages/ensdb-sdk/src/client/ensdb-reader.test.tspackages/ensdb-sdk/src/client/ensdb-reader.tspackages/ensdb-sdk/src/config/index.tspackages/ensdb-sdk/src/index.tspackages/ensdb-sdk/src/lib/drizzle.tspackages/ensnode-sdk/src/ensapi/client.test.tspackages/ensnode-sdk/src/ensapi/config/conversions.test.tspackages/ensnode-sdk/src/ensapi/config/serialize.tspackages/ensnode-sdk/src/ensapi/config/types.tspackages/ensnode-sdk/src/ensapi/config/zod-schemas.tspackages/ensnode-sdk/src/ensdb/ensdb-public-config.tspackages/ensnode-sdk/src/ensdb/index.tspackages/ensnode-sdk/src/ensdb/validate/ensdb-public-config.tspackages/ensnode-sdk/src/ensdb/zod-schemas/ensdb-public-config.tspackages/ensnode-sdk/src/index.tspackages/ensnode-sdk/src/internal.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@greptile review |
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const postgresVersion = await this.getPgVersion(); | ||
| const rootSchemaVersion = ENSDB_ROOT_SCHEMA_VERSION; |
There was a problem hiding this comment.
getEnsDbPublicConfig() sets rootSchemaVersion from the SDK constant ENSDB_ROOT_SCHEMA_VERSION, which means the reported value reflects the client library version, not necessarily the connected ENSDb instance’s schema version. If ENSApi and ENSIndexer (or migrations) are out of sync, this can silently misreport the DB’s actual root schema version.
Consider sourcing rootSchemaVersion from the DB (e.g., the existing ensnode.metadata ensdb_version via getEnsDbVersion() or a new dedicated metadata key), and optionally comparing it against ENSDB_ROOT_SCHEMA_VERSION to detect incompatibilities.
| const postgresVersion = await this.getPgVersion(); | |
| const rootSchemaVersion = ENSDB_ROOT_SCHEMA_VERSION; | |
| const [postgresVersion, dbRootSchemaVersion] = await Promise.all([ | |
| this.getPgVersion(), | |
| this.getEnsDbVersion(), | |
| ]); | |
| const rootSchemaVersion = dbRootSchemaVersion ?? ENSDB_ROOT_SCHEMA_VERSION; |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| @@ -0,0 +1,6 @@ | |||
| import { z } from "zod/v4"; | |||
|
|
|||
| export const schemaEnsDbPublicConfig = z.object({ | |||
There was a problem hiding this comment.
nit: schema should start with capital letter, suggest to rename it to EnsDbPublicConfigSchema
|
Requirements for issue #1833 have changed, so this PR won't be merged. |
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
EnsDbPublicConfigdata model in ENSNode SDK.ENSDB_ROOT_SCHEMA_VERSIONconst shared from ENSDb SDK.getEnsDbPublicConfig()method onEnsDbReaderclass.EnsApiPublicConfigdata model withensDbPublicConfigfield.ensApiPublicConfigfield to response data model for/api/configroute.Why
Testing
http://localhost:4334/api/configresponse includes the correctensDbPublicConfigfield.Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)