feat(ensapi): add forward and reverse resolution to Omnigraph#1974
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 514e12b The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 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 |
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR adds two new ENS resolution GraphQL fields to the Omnigraph API: ChangesOmnigraph Resolution API Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR spans multiple layers of the Omnigraph API with new GraphQL types, selection parsing logic, middleware wiring, and two new resolver implementations. The selection parsing and resolver logic are dense, and the changes require understanding the acceleration control flow and context propagation across Hono→Yoga→resolver boundaries. Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
Contributor
Greptile SummaryThis PR adds live ENS forward resolution (
Confidence Score: 5/5Safe to merge; all previously-flagged bugs are addressed and the new resolution layer handles edge cases (non-canonical names, empty selections, unsupported coin types) correctly. The three architectural correctness concerns from earlier rounds — encoded-label names throwing in the forward resolver, empty apps/ensapi/src/omnigraph-api/schema/reverse-resolve.ts — the defensive throws in Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as GraphQL Client
participant DR as Domain.resolve resolver
participant BSI as buildRecordsSelection<br/>FromResolveContainerInfo
participant RF as resolveForward
participant AR as Account.resolve resolver
participant BAP as buildAccountPrimaryNames<br/>Selection
participant RPNR as resolvePrimaryNameRecords
participant RR as resolveReverse (per coinType)
Client->>DR: "query { domain { resolve { records { texts addresses } } } }"
DR->>BSI: info (GraphQL AST)
BSI-->>DR: ResolverRecordsSelection
DR->>RF: resolveForward(name, selection, opts)
RF-->>DR: "{trace, result}"
DR-->>Client: "ForwardResolve {records, acceleration, trace}"
Client->>AR: "query { account { resolve { primaryNames(where:{...}) { name } } } }"
AR->>BAP: info (GraphQL AST)
BAP-->>AR: coinTypes[] (merged across all primaryName/primaryNames args)
AR->>RPNR: resolvePrimaryNameRecords(address, coinTypes, opts)
RPNR->>RR: resolveReverse(address, coinType1, opts)
RPNR->>RR: resolveReverse(address, coinType2, opts)
RR-->>RPNR: ReverseResolutionResult (parallel)
RPNR-->>AR: "{trace, records: PrimaryNameRecordModel[]}"
AR-->>Client: "ReverseResolve {primaryNames, acceleration, trace}"
Reviews (23): Last reviewed commit: "fix tests" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/omnigraph-api/schema/domain.ts`:
- Around line 224-249: Domain.records currently lacks an accelerate arg and
always calls resolveForward with accelerate: false; add a GraphQL arg
"accelerate" to the Domain.records field (the field defined at records: t.field)
and forward its value along with context.canAccelerate into resolveForward
(i.e., call resolveForward(name, {...}, { accelerate: args.accelerate,
canAccelerate: context.canAccelerate })). Also wire up the acceleration
middleware chain to the omnigraph-api router by applying
makeIsRealtimeMiddleware, indexingStatusMiddleware and canAccelerateMiddleware
(same order used in resolution-api) and update the omnigraph-api context factory
to accept the initial context param that contains canAccelerate from the
middleware so context.canAccelerate is available in the resolver; keep
getDomainInterpretedName usage unchanged.
In `@apps/ensapi/src/omnigraph-api/schema/resolution.ts`:
- Around line 16-18: Update the description string for the texts field in
resolution.ts to fix the malformed example; locate the texts: t.stringList({ ...
}) declaration and replace the example snippet "Text record keys to resolve
(e.g. `avatar`, `description`, `com.)." with a correctly punctuated example such
as "Text record keys to resolve (e.g. `avatar`, `description`, `com`)." ensuring
matching backticks and proper parentheses/period placement.
🪄 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: 909769d3-67ea-4cf4-873d-e0d220e4bdd9
⛔ Files ignored due to path filters (1)
packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**
📒 Files selected for processing (4)
apps/ensapi/src/omnigraph-api/schema.tsapps/ensapi/src/omnigraph-api/schema/domain.integration.test.tsapps/ensapi/src/omnigraph-api/schema/domain.tsapps/ensapi/src/omnigraph-api/schema/resolution.ts
shrugs
reviewed
Apr 21, 2026
Member
shrugs
left a comment
shrugs
left a comment
There was a problem hiding this comment.
So i mentioned this in the original issue, which is that i'm not sure we want to allow people to query the trace and other meta information, like whether it was accelerated or not via the graphql api because i'm not convinced the ergonomics of it makes sense. but if you find a format that feels good, we can totally do so. For example, we could make the signature
query GetRecords {
domain(by: { name: "eth" }) {
resolve([accelerate: True]) {
records { # equivalent to a ResolverRecordsSelection
name
texts(keys: ["description"]) { key value }
addresses(coinType: [60]) { coinType address }
}
acceleration { requested attempted }
trace
}
}
}
query GetPrimaryNames {
account(by: { address: "0xabcd" }) {
resolve(accelerate: False) {
primaryNames(coinTypes: [60]) { coinType name }
acceleration { requested attempted }
trace
}
}
}then that feels good; let's do that! then the pattern is clear; Domain.resolve.records and Account.resolve.primaryNames — feels good to me!
Note that within enskit, we need to update the cache client to mark these fields as EMBEDDED_DATA because they don't have a global key. Also, feel free to update the example app to render these records. So maybe add a new component that fetches a name's records by Domain.id or (name from the params, whatever) and then plug it into the name page.
Overall, this is definitely the correct direction and the data loader usage is correct. Let's move that helper into its own library.
to access the pothos context type, let's
- define a
type Context = ReturnType<typeof createContext>inapps/ensapi/src/omnigraph-api/context.ts - use that everywhere we currently use
context: ReturnType<typeof createContext> - make sure the helper you extracted here uses said type as well.
no need to alias it to graphqlContext; within the omnigraph graphql server module, "context" is always the graphql context
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/ensapi/src/omnigraph-api/schema/domain.ts (1)
185-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Domain.recordsstill ignores the acceleration contract.Line 197 hardcodes
accelerate: false, canAccelerate: false, so clients cannot use the documented default accelerated path (acceleratedefault true) and context capability is bypassed.🤖 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 `@apps/ensapi/src/omnigraph-api/schema/domain.ts` around lines 185 - 201, The Domain.records resolver currently hardcodes acceleration by calling resolveForward(name, selection, { accelerate: false, canAccelerate: false }), which prevents using the default accelerated path and ignores context capabilities; update the resolve function in the records field to stop forcing those flags — either omit the options so resolveForward uses its defaults (allowing accelerate default true) or explicitly pass accelerate from args and canAccelerate from the GraphQL context (e.g., use _args.accelerate ?? true and _context.canAccelerate) when invoking resolveForward so acceleration behavior respects client requests and context capability checks.
🤖 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 `@apps/ensapi/src/omnigraph-api/lib/build-records-selection.ts`:
- Around line 45-48: The code currently uses only the first
GraphQLResolveInfo.fieldNodes element (fieldNode) when building the selection
and thus drops merged selections; update the build logic in
build-records-selection (where fieldNode is referenced) to merge all
info.fieldNodes by iterating over info.fieldNodes, collecting each
node.selectionSet?.selections and concatenating them into a single combined
selection array (or combined selectionSet), then use that combined selectionSet
for further processing; if no selection sets are present across all nodes, still
throw the existing GraphQLError with EMPTY_RECORDS_SELECTION_MESSAGE.
In `@apps/ensapi/src/omnigraph-api/schema/account.ts`:
- Around line 75-95: The Account.primaryNames field currently accepts chainIds
and forces acceleration off; change its args to match the contract (accept
optional accelerate?: boolean and coinTypes?: ["CoinType"] or appropriate type)
and stop hard-disabling acceleration: call resolvePrimaryNames(account.id,
/*pass coinTypes or equivalent*/ coinTypes ?? undefined, { accelerate:
accelerate ?? context.canAccelerate, canAccelerate: context.canAccelerate })
instead of {accelerate:false, canAccelerate:false}; keep input validation
(replace or adapt validatePrimaryNamesChainIds to validate the new
coinTypes/args if necessary) and ensure runWithTrace still wraps the
resolvePrimaryNames call.
---
Duplicate comments:
In `@apps/ensapi/src/omnigraph-api/schema/domain.ts`:
- Around line 185-201: The Domain.records resolver currently hardcodes
acceleration by calling resolveForward(name, selection, { accelerate: false,
canAccelerate: false }), which prevents using the default accelerated path and
ignores context capabilities; update the resolve function in the records field
to stop forcing those flags — either omit the options so resolveForward uses its
defaults (allowing accelerate default true) or explicitly pass accelerate from
args and canAccelerate from the GraphQL context (e.g., use _args.accelerate ??
true and _context.canAccelerate) when invoking resolveForward so acceleration
behavior respects client requests and context capability checks.
🪄 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: df655b88-c1d7-4207-b684-04d8bbcb87a2
📒 Files selected for processing (13)
apps/ensapi/src/omnigraph-api/builder.tsapps/ensapi/src/omnigraph-api/lib/build-records-selection.test.tsapps/ensapi/src/omnigraph-api/lib/build-records-selection.tsapps/ensapi/src/omnigraph-api/lib/records-selection-config.tsapps/ensapi/src/omnigraph-api/lib/validate-primary-names-chain-ids.test.tsapps/ensapi/src/omnigraph-api/lib/validate-primary-names-chain-ids.tsapps/ensapi/src/omnigraph-api/schema.tsapps/ensapi/src/omnigraph-api/schema/account.integration.test.tsapps/ensapi/src/omnigraph-api/schema/account.tsapps/ensapi/src/omnigraph-api/schema/domain.integration.test.tsapps/ensapi/src/omnigraph-api/schema/domain.tsapps/ensapi/src/omnigraph-api/schema/resolution.tsapps/ensapi/src/omnigraph-api/schema/scalars.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/ensapi/src/omnigraph-api/schema/domain.integration.test.ts (1)
551-601: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winPrefer
await expect(request(...)).resolves.toMatchObject(...)in these async assertions.Please switch the two updated test cases to the preferred resolves style for consistency.
As per coding guidelines, Prefer the
await expect(...).resolves.*format over await-then-expect for async assertions.🤖 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 `@apps/ensapi/src/omnigraph-api/schema/domain.integration.test.ts` around lines 551 - 601, Update the two async tests ("resolves address and text records for example.eth" and "resolves every supported record type for test.eth") to use the await expect(...).resolves.toMatchObject(...) pattern instead of awaiting the request and then calling expect on the result; specifically, call request<DomainRecordsResult>(DomainRecords, {...}) and request<DomainAllRecordsResult>(DomainRecordsAll, {...}) inside expect(...) and append .resolves.toMatchObject(...) so the assertions are written as await expect(request(...)).resolves.toMatchObject(...).apps/ensapi/src/omnigraph-api/schema/resolution.ts (1)
196-246:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParametric record resolvers should enforce argument-scoped output.
interfaces(ids),texts(keys), andaddresses(coinTypes)ignore their args and return all entries inr.interfaces/r.texts/r.addresses. That breaks field-level scoping for these parametric fields and can leak supersets when selections are merged.Proposed fix
interfaces: t.field({ @@ - resolve: (r) => - r.interfaces - ? Object.entries(r.interfaces).map(([interfaceId, implementer]) => ({ - interfaceId: interfaceId as InterfaceId, - implementer, - })) - : [], + resolve: (r, { ids }) => + ids.map((interfaceId) => ({ + interfaceId, + implementer: r.interfaces?.[interfaceId] ?? null, + })), }), @@ texts: t.field({ @@ - resolve: (r) => - r.texts ? Object.entries(r.texts).map(([key, value]) => ({ key, value })) : [], + resolve: (r, { keys }) => + keys.map((key) => ({ + key, + value: r.texts?.[key] ?? null, + })), }), @@ addresses: t.field({ @@ - resolve: (r) => - r.addresses - ? Object.entries(r.addresses).map(([coinType, address]) => ({ - coinType: Number(coinType) as CoinType, - address, - })) - : [], + resolve: (r, { coinTypes }) => + coinTypes.map((coinType) => ({ + coinType, + address: r.addresses?.[coinType] ?? null, + })), }),🤖 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 `@apps/ensapi/src/omnigraph-api/schema/resolution.ts` around lines 196 - 246, The resolvers for interfaces, texts, and addresses return all records from r.interfaces/r.texts/r.addresses instead of restricting results to the requested args; update the resolve functions for interfaces (field interfaces with arg ids), texts (field texts with arg keys), and addresses (field addresses with arg coinTypes) to iterate over the provided args (ids, keys, coinTypes) and for each arg lookup the corresponding value in r.interfaces/r.texts/r.addresses, returning only entries found (or an explicit null/omit if missing) and preserving the declared return shape (InterfaceId, key/value, CoinType/address) so the field output is scoped to the arguments.apps/ensapi/src/omnigraph-api/lib/records-selection-config.ts (1)
45-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParametric selection assignment is currently lossy (last-write-wins).
applyToRecordsSelectionoverwritestexts/addresses/interfaces/abieach time. If the same parametric field appears multiple times (aliases/fragments), earlier requested values are dropped.Proposed fix
{ @@ applyToRecordsSelection: (recordsSelection, args) => { - recordsSelection.texts = args.keys as string[]; + const next = args.keys as string[]; + recordsSelection.texts = [...new Set([...(recordsSelection.texts ?? []), ...next])]; }, }, @@ applyToRecordsSelection: (recordsSelection, args) => { - recordsSelection.addresses = args.coinTypes as CoinType[]; + const next = args.coinTypes as CoinType[]; + recordsSelection.addresses = [ + ...new Set([...(recordsSelection.addresses ?? []), ...next]), + ]; }, }, @@ applyToRecordsSelection: (recordsSelection, args) => { - recordsSelection.abi = args.contentTypeMask as ContentType; + const next = args.contentTypeMask as ContentType; + recordsSelection.abi = ((recordsSelection.abi ?? 0n) | next) as ContentType; }, }, @@ applyToRecordsSelection: (recordsSelection, args) => { - recordsSelection.interfaces = args.ids as InterfaceId[]; + const next = args.ids as InterfaceId[]; + recordsSelection.interfaces = [ + ...new Set([...(recordsSelection.interfaces ?? []), ...next]), + ]; }, },🤖 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 `@apps/ensapi/src/omnigraph-api/lib/records-selection-config.ts` around lines 45 - 78, The current RECORDS_SELECTION_PARAMETRIC_FIELDS entries' applyToRecordsSelection handlers overwrite recordsSelection.texts / .addresses / .interfaces / .abi on each call, losing earlier values; change each handler to merge into existing values instead of replacing: for texts/addresses/interfaces, if recordsSelection.<field> is unset assign args value, otherwise append new items and deduplicate (preserve existing entries) so repeated param occurrences accumulate; for abi (contentTypeMask) combine masks using bitwise OR rather than assignment; update the applyToRecordsSelection lambdas inside RECORDS_SELECTION_PARAMETRIC_FIELDS to perform these merge/dedupe steps when reading args.keys, args.coinTypes, args.ids and args.contentTypeMask.
♻️ Duplicate comments (1)
apps/ensapi/src/omnigraph-api/schema/domain.ts (1)
190-207:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
accelerate(defaulttrue) instead of inversedisableAccelerationfor this GraphQL field.This inverts the contract described for
Domain.recordsand makes the API less ergonomic/consistent with the issue objectives.Proposed fix
args: { - disableAcceleration: t.arg.boolean({ + accelerate: t.arg.boolean({ required: false, - defaultValue: false, - description: "When true, disables protocol acceleration feature.", + defaultValue: true, + description: "Whether to use protocol acceleration when available.", }), }, - resolve: async (domain, { disableAcceleration }, context, info) => { + resolve: async (domain, { accelerate }, context, info) => { @@ const { result } = await runWithTrace(() => resolveForward(name, recordsSelection, { - accelerate: !disableAcceleration, + accelerate, canAccelerate: context.canAccelerate, }), );🤖 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 `@apps/ensapi/src/omnigraph-api/schema/domain.ts` around lines 190 - 207, The GraphQL field currently accepts disableAcceleration and inverts it when calling resolveForward, which is confusing; change the argument to accelerate: t.arg.boolean({ required: false, defaultValue: true, description: "When true, enables protocol acceleration feature." }) (replace the disableAcceleration arg and its description) and in the resolve function use the incoming accelerate value directly when calling resolveForward (i.e., pass accelerate instead of !disableAcceleration), keeping context.canAccelerate unchanged and leaving buildRecordsSelectionFromResolveInfo(info) and resolveForward intact.
🤖 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 `@apps/ensapi/src/handlers/api/omnigraph/omnigraph-api.ts`:
- Line 15: Add a short explanatory comment above the
MAX_REALTIME_DISTANCE_TO_ACCELERATE constant describing that the 60-second
threshold defines the realtime window used to decide whether to accelerate
processing (e.g., to use realtime data vs. batched processing), and include the
rationale or source for the value (user-experience/latency target, backend
polling interval, or a linked issue/config) so future maintainers understand why
60 seconds was chosen; update the comment near the constant declaration
MAX_REALTIME_DISTANCE_TO_ACCELERATE: Duration = 60 with this brief
justification.
In `@apps/ensapi/src/omnigraph-api/lib/validate-primary-names-chain-ids.ts`:
- Around line 6-18: Update the JSDoc for validatePrimaryNamesChainIds to state
that this function only enforces non-empty arrays and that individual ChainId
values are validated by the GraphQL scalar (so invalid chain IDs like 0 are
rejected upstream), referencing the function name validatePrimaryNamesChainIds,
the ChainId scalar, and the EMPTY_CHAIN_IDS_MESSAGE/GraphQLError behavior to
make the division of responsibility explicit for future maintainers.
---
Outside diff comments:
In `@apps/ensapi/src/omnigraph-api/lib/records-selection-config.ts`:
- Around line 45-78: The current RECORDS_SELECTION_PARAMETRIC_FIELDS entries'
applyToRecordsSelection handlers overwrite recordsSelection.texts / .addresses /
.interfaces / .abi on each call, losing earlier values; change each handler to
merge into existing values instead of replacing: for texts/addresses/interfaces,
if recordsSelection.<field> is unset assign args value, otherwise append new
items and deduplicate (preserve existing entries) so repeated param occurrences
accumulate; for abi (contentTypeMask) combine masks using bitwise OR rather than
assignment; update the applyToRecordsSelection lambdas inside
RECORDS_SELECTION_PARAMETRIC_FIELDS to perform these merge/dedupe steps when
reading args.keys, args.coinTypes, args.ids and args.contentTypeMask.
In `@apps/ensapi/src/omnigraph-api/schema/domain.integration.test.ts`:
- Around line 551-601: Update the two async tests ("resolves address and text
records for example.eth" and "resolves every supported record type for
test.eth") to use the await expect(...).resolves.toMatchObject(...) pattern
instead of awaiting the request and then calling expect on the result;
specifically, call request<DomainRecordsResult>(DomainRecords, {...}) and
request<DomainAllRecordsResult>(DomainRecordsAll, {...}) inside expect(...) and
append .resolves.toMatchObject(...) so the assertions are written as await
expect(request(...)).resolves.toMatchObject(...).
In `@apps/ensapi/src/omnigraph-api/schema/resolution.ts`:
- Around line 196-246: The resolvers for interfaces, texts, and addresses return
all records from r.interfaces/r.texts/r.addresses instead of restricting results
to the requested args; update the resolve functions for interfaces (field
interfaces with arg ids), texts (field texts with arg keys), and addresses
(field addresses with arg coinTypes) to iterate over the provided args (ids,
keys, coinTypes) and for each arg lookup the corresponding value in
r.interfaces/r.texts/r.addresses, returning only entries found (or an explicit
null/omit if missing) and preserving the declared return shape (InterfaceId,
key/value, CoinType/address) so the field output is scoped to the arguments.
---
Duplicate comments:
In `@apps/ensapi/src/omnigraph-api/schema/domain.ts`:
- Around line 190-207: The GraphQL field currently accepts disableAcceleration
and inverts it when calling resolveForward, which is confusing; change the
argument to accelerate: t.arg.boolean({ required: false, defaultValue: true,
description: "When true, enables protocol acceleration feature." }) (replace the
disableAcceleration arg and its description) and in the resolve function use the
incoming accelerate value directly when calling resolveForward (i.e., pass
accelerate instead of !disableAcceleration), keeping context.canAccelerate
unchanged and leaving buildRecordsSelectionFromResolveInfo(info) and
resolveForward 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: e0c05acb-eec2-4985-9af4-2595aba12a18
⛔ Files ignored due to path filters (2)
packages/enssdk/src/omnigraph/generated/introspection.tsis excluded by!**/generated/**packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**
📒 Files selected for processing (17)
apps/ensapi/src/handlers/api/omnigraph/omnigraph-api.tsapps/ensapi/src/omnigraph-api/builder.tsapps/ensapi/src/omnigraph-api/context.tsapps/ensapi/src/omnigraph-api/lib/build-records-selection.test.tsapps/ensapi/src/omnigraph-api/lib/build-records-selection.tsapps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver.tsapps/ensapi/src/omnigraph-api/lib/records-selection-config.tsapps/ensapi/src/omnigraph-api/lib/validate-primary-names-chain-ids.test.tsapps/ensapi/src/omnigraph-api/lib/validate-primary-names-chain-ids.tsapps/ensapi/src/omnigraph-api/schema/account.integration.test.tsapps/ensapi/src/omnigraph-api/schema/account.tsapps/ensapi/src/omnigraph-api/schema/domain.integration.test.tsapps/ensapi/src/omnigraph-api/schema/domain.tsapps/ensapi/src/omnigraph-api/schema/resolution.tsapps/ensapi/src/omnigraph-api/schema/scalars.tsapps/ensapi/src/omnigraph-api/yoga.tspackages/enssdk/src/omnigraph/graphql.ts
💤 Files with no reviewable changes (3)
- packages/enssdk/src/omnigraph/graphql.ts
- apps/ensapi/src/omnigraph-api/lib/validate-primary-names-chain-ids.test.ts
- apps/ensapi/src/omnigraph-api/schema/scalars.ts
Member
Author
|
@greptile review |
Member
Author
|
@greptile |
Member
Author
|
@greptile review |
shrugs
reviewed
May 28, 2026
Member
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Super exciting to see your updates here. Reviewed and shared suggestions for how to get this PR ready to merge 👍
Member
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Super exciting to see your updates here. Reviewed and shared suggestions for how to get this PR ready to merge 👍
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Domain.recordsto Omnigraph for live ENS forward resolution (texts, addresses, contenthash, pubkey, ABI, interfaces, etc.), driven by the GraphQL field selection.Account.primaryNamesfor ENSIP-19 multichain primary name resolution, with optionalchainIdsfiltering.Why
Omnigraph previously exposed only indexed ENS state. Resolution is important thing for ENS developers so this PR adds resolution alongside that data so GraphQL clients can fetch live records and primary names in the same query shape as the rest of the API! It's cool, is it?
Testing
build-records-selection.test.ts(GraphQL selection →ResolverRecordsSelection),validate-primary-names-chain-ids.test.ts.domain.integration.test.ts(Domain.recordsfor address/text and all record types),account.integration.test.ts(Account.primaryNamesper-chain, all chains, and invalid chain id rejection).domain-recordsandaccount-primary-names.devnetand tested queries in ensadmin locally 👍Notes for Reviewer (Optional)
Domain.records(e.g.records { texts(keys: [...]) addresses(coinTypes: [...]) })disableAcceleration; acceleration is gated by request context (canAccelerate). Since we want to enable acceleration by default decided to rename oldaccelecateparam todisableAccelerationapps/ensapi/src/omnigraph-api/lib/build-records-selection.ts. This is my first dive into graphql query world, still not an expert in this.Pre-Review Checklist (Blocking)