Skip to content

Refactor Find Domains into Layers#1680

Merged
shrugs merged 11 commits intomainfrom
feat/refactor-find-domains-availability
Feb 25, 2026
Merged

Refactor Find Domains into Layers#1680
shrugs merged 11 commits intomainfrom
feat/refactor-find-domains-availability

Conversation

@shrugs
Copy link
Collaborator

@shrugs shrugs commented Feb 25, 2026

  • refactors findDomains into composable query layers
  • add Account.domains
  • keeps top-level Query.domains
  • enhances Domain.subdomains and Registry.domains

Reviewer Focus (Read This First)

  • the composable layer architecture in find-domains/layers/ — is the shape/contract between layers clear and maintainable?
  • the domainsBase() universal base set, particularly the v2 parentId derivation via canonical registry traversal (registryCanonicalDomain → reverse pointer verification)
  • breaking changes to Query.domains: where.name is now required, where.owner was removed (moved to Account.domains)

Problem & Motivation

  • findDomains was a monolithic function that handled path traversal, owner/canonical/partial filtering, and ordering metadata in one giant CTE
  • Domain.subdomains and Registry.domains used simple cursor pagination with no name filtering or ordering support
  • no Account.domains field existed — owner filtering was baked into Query.domains via where.owner
  • adding new entry points or filter combinations required duplicating or further complicating the monolith

What Changed (Concrete)

  1. decomposed findDomains into composable query layers in find-domains/layers/:
    • domainsBase() — universal v1+v2 union with v2 parentId derived via registryCanonicalDomain traversal, sortableLabel, registryId
    • filterByName(base, name?) — path traversal + partial LIKE matching
    • filterByCanonical(base) — canonical registry filtering via recursive CTE
    • filterByOwner(base, owner) — owner address filtering
    • filterByParent(base, parentId) — parent domain filtering (uniform v1/v2)
    • filterByRegistry(base, registryId) — registry ID filtering
    • withOrderingMetadata(base) — join registration timestamps/expiry for ordering
  2. added Account.domains connection field with where: { name?: String, canonical?: Boolean } and order args
  3. enhanced Domain.subdomains with where: { name?: String } and order args (was simple id-cursor pagination)
  4. enhanced Registry.domains with where: { name?: String } and order args; return type changed from ENSv2Domain to DomainInterface
  5. Query.domains: where.name is now required, where.owner removed (use Account.domains instead)
  6. added per-field input types: DomainsWhereInput, AccountDomainsWhereInput, RegistryDomainsWhereInput, SubdomainsWhereInput
  7. resolveFindDomains now accepts a pre-built DomainsCTE instead of calling findDomains internally
  8. canonical registries CTE output column renamed registryIdid to avoid drizzle column name ambiguity in LEFT JOINs

Design & Planning

  • planned iteratively via claude code plan mode; each layer was validated against generated SQL output

  • the key design decision was a single universal base set (domainsBase()) rather than specialized base sets per entry point — simpler composition at the cost of a larger base union, but postgres predicate pushdown into union branches should
    handle this

  • v2 parentId is derived in the base set via canonical registry traversal: LEFT JOIN registryCanonicalDomain to find the declared canonical domain for the child's registry, then LEFT JOIN v2ParentDomain to verify the reverse pointer
    (parent.subregistryId = child.registryId). this is a single-hop version of what getV2CanonicalPath does recursively — eliminates v1/v2 branching in filterByParent

  • sortableLabel pattern: base set provides the domain's own label; filterByName overrides it with the head domain's label when a concrete path is present (leafId ≠ headId in path traversal)

  • Planning artifacts: .claude/plans/humble-yawning-swing.md

  • Reviewed / approved by: n/a


Self-Review

  • found and fixed drizzle column name ambiguity: registryId existed in both the base set and canonical registries CTE, causing "column reference 'registryId' is ambiguous" at runtime. fixed by renaming the CTE output column to id.

  • simplified filterByCanonical from IN-subquery approach back to LEFT JOIN after identifying the column rename as the correct fix

  • Bugs caught: ambiguous column reference in filterByCanonical LEFT JOIN

  • Logic simplified: filterByParent — no v1/v2 branching needed thanks to derived parentId in base set

  • Naming / terminology improved: FindDomainsWhereArg removed, replaced by per-field input types

  • Dead or unnecessary code removed: ~200 lines removed from find-domains.ts


Cross-Codebase Alignment

  • Search terms used: findDomains, resolveFindDomains, FindDomainsWhereArg, DomainsWhereInput, subdomains, Registry.domains
  • Reviewed but unchanged: find-domains-by-labelhash-path.ts (reused by filterByName), canonical-registries-cte.ts (reused by filterByCanonical, only column rename), domain-cursor.ts, get-canonical-path.ts
  • Deferred alignment (with rationale): ENSv1Domain.children left as-is — simple id-cursor connection, doesn't need the composable layer treatment

Downstream & Consumer Impact

  • breaking: Query.domains where.name is now required (was optional). where.owner removed — use Account.domains instead.

  • breaking: Registry.domains return type changed from ENSv2Domain to DomainInterface (now includes v1 domains if registry filter somehow matches, though in practice only v2 domains have non-null registryId)

  • new: Account.domains connection field for querying domains by owner with optional name/canonical filters and ordering

  • enhanced: Domain.subdomains and Registry.domains now support name filtering and ordering (NAME, REGISTRATION_TIMESTAMP, REGISTRATION_EXPIRY)

  • Public APIs affected: Query.domains, Account.domains (new), Domain.subdomains, Registry.domains

  • Docs updated: changeset included

  • Naming decisions worth calling out: separate input types per field (DomainsWhereInput, AccountDomainsWhereInput, etc.) rather than one shared type — keeps each field's contract precise


Testing Evidence

  • typecheck, lint, and test all pass

  • manually tested all four entry points against a running instance with indexed data

  • verified canonical filtering works independently of name filtering on Account.domains

  • Testing performed: pnpm typecheck, pnpm lint, pnpm test, manual GraphQL queries

  • Known gaps: no automated integration tests for the new entry points or layer composition

  • What reviewers have to reason about manually (and why): correctness of generated SQL for each pipeline composition — drizzle's subquery aliasing behavior is non-obvious, particularly around column name qualification in JOINs


Scope Reductions

  • ENSv1Domain.children left as simple id-cursor connection (no name/ordering support)
  • no automated tests for layer composition — deferred to a follow-up

Risk Analysis

  • drizzle column name ambiguity is a known footgun — any future layer that JOINs a table with columns named domainId, ownerId, registryId, etc. could hit the same issue. mitigated by using distinct column names in CTEs.

  • the universal base set scans all v1+v2 domains before filtering. postgres predicate pushdown should optimize this, but if it doesn't, query performance could regress for large datasets.

  • Risk areas: query performance on large datasets, drizzle column aliasing edge cases

  • Mitigations or rollback options: revert to previous monolithic findDomains if needed; SQL debug logging in resolver aids diagnosis

  • Named owner if this causes problems: @shrugs


Pre-Review Checklist (Blocking)

  • I reviewed every line of this diff and understand it end-to-end
  • I'm prepared to defend this PR line-by-line in review
  • I'm comfortable being the on-call owner for this change
  • Relevant changesets are included (or explicitly not required)

shrugs and others added 3 commits February 24, 2026 17:09
decompose monolithic findDomains into composable filter layers:
- domainsBase(): universal base set with derived v2 parentId and sortableLabel
- selectBase(): helper for consistent column selection across layers
- filterByOwner/filterByParent/filterByRegistry/filterByCanonical/filterByName
- withOrderingMetadata: final CTE for cursor pagination

each GraphQL entry point composes its own pipeline:
- Query.domains: domainsBase → filterByName → filterByCanonical → withOrderingMetadata
- Account.domains: domainsBase → filterByOwner → filterByName → filterByCanonical → withOrderingMetadata
- Registry.domains: domainsBase → filterByRegistry → filterByName → withOrderingMetadata
- Domain.subdomains: domainsBase → filterByParent → filterByName → withOrderingMetadata

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rename output column from `registryId` to `id` in getCanonicalRegistriesCTE
to prevent column name clash when LEFT JOINed with base domain set's
`registryId` column. Drizzle doesn't qualify subquery column references
in JOINs, causing PostgreSQL "column reference is ambiguous" errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 25, 2026 16:03
@changeset-bot
Copy link

changeset-bot bot commented Feb 25, 2026

🦋 Changeset detected

Latest commit: 388c9bd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
ensapi Patch
ensindexer Patch
ensadmin Patch
ensrainbow Patch
fallback-ensapi Patch
@ensnode/datasources Patch
@ensnode/ensrainbow-sdk Patch
@ensnode/ponder-metadata Patch
@ensnode/ensnode-schema Patch
@ensnode/ensnode-react Patch
@ensnode/ensnode-sdk Patch
@ensnode/ponder-sdk Patch
@ensnode/ponder-subgraph Patch
@ensnode/shared-configs Patch
@docs/ensnode Patch
@docs/ensrainbow Patch
@docs/mintlify Patch
@namehash/ens-referrals Patch
@namehash/namehash-ui Patch

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

@vercel
Copy link
Contributor

vercel bot commented Feb 25, 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 25, 2026 5:52pm
ensnode.io Skipped Skipped Feb 25, 2026 5:52pm
ensrainbow.io Skipped Skipped Feb 25, 2026 5:52pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

This pull request refactors the domain-finding pipeline from a monolithic function into a composable, layered architecture. The new system builds a base domain set combining v1 and v2 sources, applies sequential filters (canonical, name, owner, registry, parent), enriches with ordering metadata, and integrates across GraphQL schema endpoints (Account, Domain, Query, Registry).

Changes

Cohort / File(s) Summary
Layered Domain Filtering Pipeline
apps/ensapi/src/graphql-api/lib/find-domains/layers/base-domain-set.ts, filter-by-canonical.ts, filter-by-name.ts, filter-by-owner.ts, filter-by-parent.ts, filter-by-registry.ts, with-ordering-metadata.ts, index.ts
New modular architecture that replaces monolithic domain-finding logic. Includes base domain set combining v1/v2 sources, individual filter functions for canonical, name, owner, parent, and registry matching, and ordering metadata enrichment for cursor-based pagination. Complex name filter handles InterpretedName parsing and path traversal.
Domain Finding Core Logic
apps/ensapi/src/graphql-api/lib/find-domains/find-domains.ts, find-domains-resolver.ts, types.ts
Refactored from monolithic findDomains function to DomainsCTE-based types and signatures. Introduces DomainsCTE type alias, removes FindDomainsWhereArg, updates resolver to accept pre-built domains CTE instead of where argument, eliminates 203 lines of removed findDomains implementation.
GraphQL Schema Integration
apps/ensapi/src/graphql-api/schema/account.ts, domain.ts, query.ts, registry.ts
Unified resolver pattern across all domain query endpoints. Account.domains connection added with where/order args; Domain.subdomains, Query.domains, and Registry.domains updated to use layered filtering pipeline. Introduces AccountDomainsWhereInput, RegistryDomainsWhereInput, SubdomainsWhereInput with name/canonical filters. Name now required in top-level domains query.
Canonical Registry Infrastructure
apps/ensapi/src/graphql-api/lib/canonical-registries-cte.ts
Changed registry_id alias from registryId to id to avoid PostgreSQL ambiguity in drizzle queries. Comment added explaining scope handling.
Documentation
.changeset/refactor-find-domains-layers.md
Release notes documenting new paginated connections and enhanced filtering/ordering capabilities across Account.domains, Domain.subdomains, Registry.domains, and Query.domains fields.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 Layer upon layer, filters align,
From base to blessed canonical design,
Domains dance through name and owner's gate,
Order metadata makes pagination great,
One function scattered, now composed so fine! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'Refactor Find Domains into Layers' accurately describes the main structural refactoring of decomposing a monolithic function into composable query layers.
Docstring Coverage ✅ Passed Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The PR description is comprehensive and well-structured, following the template with all required sections completed.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/refactor-find-domains-availability

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.

@vercel vercel bot temporarily deployed to Preview – ensnode.io February 25, 2026 16:03 Inactive
@vercel vercel bot temporarily deployed to Preview – admin.ensnode.io February 25, 2026 16:03 Inactive
@vercel vercel bot temporarily deployed to Preview – ensrainbow.io February 25, 2026 16:03 Inactive
@shrugs shrugs marked this pull request as ready for review February 25, 2026 16:03
@shrugs shrugs requested a review from a team as a code owner February 25, 2026 16:03
@vercel vercel bot temporarily deployed to Preview – ensrainbow.io February 25, 2026 16:04 Inactive
@vercel vercel bot temporarily deployed to Preview – ensnode.io February 25, 2026 16:04 Inactive
@vercel vercel bot temporarily deployed to Preview – admin.ensnode.io February 25, 2026 16:04 Inactive
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Greptile Summary

This PR successfully refactors findDomains from a monolithic function into a clean, composable layer architecture. The decomposition into domainsBase(), filter layers (filterByName, filterByOwner, filterByCanonical, etc.), and withOrderingMetadata() provides excellent separation of concerns and enables new entry points like Account.domains.

Key improvements:

  • Unified v1/v2 handling through derived parentId in base set eliminates v1/v2 branching in filter layers
  • New Account.domains field with name/canonical filtering and ordering
  • Enhanced Domain.subdomains and Registry.domains with filtering and ordering support
  • Reduced complexity: ~200 lines removed from monolithic implementation

Breaking changes:

  • Query.domains now requires where.name (was optional)
  • Query.domains removed where.owner field (use Account.domains instead)
  • Registry.domains return type changed from ENSv2Domain to DomainInterface

Notable design decisions:

  • Single universal base set instead of specialized sets per entry point - simpler composition, relies on Postgres predicate pushdown for performance
  • v2 parentId derived via LEFT JOIN in base set: v2ParentDomain.subregistryId = v2Domain.registryId
  • Column rename fix: canonical registries CTE output registryIdid to avoid drizzle ambiguity

The layer contracts are clear and maintainable - each layer accepts a BaseDomainSet and returns a new base set with the same shape.

Confidence Score: 4/5

  • Safe to merge with monitoring - well-architected refactor with clear benefits, but query performance needs validation on production datasets
  • Score reflects excellent architecture and implementation quality, but deducted 1 point for: (1) universal base set performance risk on large datasets - Postgres predicate pushdown is assumed but not verified, (2) lack of automated integration tests for layer composition, (3) drizzle column ambiguity remains a known footgun that could resurface
  • Monitor query performance for base-domain-set.ts (universal UNION ALL) and verify v2 parentId derivation handles edge cases correctly

Important Files Changed

Filename Overview
apps/ensapi/src/graphql-api/lib/find-domains/layers/base-domain-set.ts adds universal base domain set with v1+v2 union and derived v2 parentId via LEFT JOIN
apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-name.ts implements name filtering with path traversal and partial LIKE matching; contains SQL injection mitigation TODOs
apps/ensapi/src/graphql-api/lib/canonical-registries-cte.ts renames CTE output column from registryId to id to avoid drizzle column ambiguity
apps/ensapi/src/graphql-api/schema/query.ts BREAKING: Query.domains now requires where.name, removed where.owner field
apps/ensapi/src/graphql-api/schema/account.ts NEW: adds Account.domains with optional name/canonical filters and ordering support
apps/ensapi/src/graphql-api/schema/domain.ts enhances Domain.subdomains with name filtering and ordering; adds new input types per field
apps/ensapi/src/graphql-api/schema/registry.ts BREAKING: Registry.domains return type changed to DomainInterface, added filtering and ordering

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start[Entry Point: Query.domains / Account.domains / Domain.subdomains / Registry.domains]
    Base[domainsBase: Universal v1+v2 union with derived parentId]
    FilterName[filterByName: Path traversal + LIKE matching]
    FilterOwner[filterByOwner: owner address filter]
    FilterParent[filterByParent: parent domain filter]
    FilterRegistry[filterByRegistry: registry ID filter]
    FilterCanonical[filterByCanonical: LEFT JOIN canonical registries CTE]
    WithMetadata[withOrderingMetadata: JOIN registration data]
    Resolver[resolveFindDomains: cursor pagination + dataloader]
    Result[Return: Domain connection]

    Start --> Base
    Base --> FilterName
    Base --> FilterOwner
    Base --> FilterParent
    Base --> FilterRegistry
    
    FilterName --> FilterCanonical
    FilterOwner --> FilterName
    FilterOwner --> FilterCanonical
    FilterParent --> FilterName
    FilterRegistry --> FilterName
    
    FilterName --> WithMetadata
    FilterCanonical --> WithMetadata
    
    WithMetadata --> Resolver
    Resolver --> Result

    style Base fill:#e1f5ff
    style WithMetadata fill:#ffe1f5
    style Resolver fill:#f5ffe1
Loading

Last reviewed commit: 11d9564

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

17 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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

This PR refactors the monolithic findDomains query function into a composable layer architecture, enabling better code reuse and more flexible domain querying across multiple GraphQL entry points.

Changes:

  • Decomposed findDomains into composable query layers: domainsBase() (universal v1+v2 union), filter layers (canonical, name, owner, parent, registry), and withOrderingMetadata()
  • Added new Account.domains connection field for querying domains by owner with optional name/canonical filters
  • Enhanced Domain.subdomains and Registry.domains with name filtering and ordering support
  • Breaking changes to Query.domains: where.name is now required, where.owner removed (moved to Account.domains)

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/graphql-api/lib/find-domains/layers/base-domain-set.ts Universal base domain set with v1+v2 union and derived v2 parentId via canonical registry traversal
apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-canonical.ts Filters domains to only canonical (v1 domains + v2 domains in canonical registries)
apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-name.ts Applies path traversal and partial LIKE matching for name-based filtering
apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-owner.ts Filters domains by owner address
apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-parent.ts Filters domains to children of a specific parent (uniform v1/v2 handling)
apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-registry.ts Filters domains to those belonging to a specific registry
apps/ensapi/src/graphql-api/lib/find-domains/layers/with-ordering-metadata.ts Enriches domain set with registration metadata for ordering support
apps/ensapi/src/graphql-api/lib/find-domains/layers/index.ts Exports all layer functions for composable use
apps/ensapi/src/graphql-api/lib/find-domains/find-domains.ts Removed monolithic findDomains function, retained utility functions for ordering and cursor filtering
apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.ts Updated to accept pre-built domains CTE instead of where args
apps/ensapi/src/graphql-api/lib/find-domains/types.ts Removed FindDomainsWhereArg, updated comments to reference new architecture
apps/ensapi/src/graphql-api/lib/canonical-registries-cte.ts Renamed output column from registryId to id to avoid drizzle column ambiguity
apps/ensapi/src/graphql-api/schema/account.ts Added Account.domains connection field using layer composition
apps/ensapi/src/graphql-api/schema/domain.ts Enhanced Domain.subdomains, added per-field input types (DomainsWhereInput, AccountDomainsWhereInput, RegistryDomainsWhereInput, SubdomainsWhereInput)
apps/ensapi/src/graphql-api/schema/registry.ts Enhanced Registry.domains with filtering/ordering, changed return type from ENSv2Domain to DomainInterface
apps/ensapi/src/graphql-api/schema/query.ts Updated Query.domains to use layer composition, where.name now required
.changeset/refactor-find-domains-layers.md Documents new Account.domains field and enhancements to Domain.subdomains and Registry.domains
Comments suppressed due to low confidence (2)

.changeset/refactor-find-domains-layers.md:17

  • The changeset should document that the return type of Registry.domains has changed from ENSv2Domain to DomainInterface. While in practice this field will only return v2 domains (since only v2 domains have a non-null registryId), this is a breaking change for type-sensitive GraphQL clients that may need to update their queries to handle the more general interface type.
**`Registry.domains`** (enhanced) — paginated connection of domains in this registry, now with filtering and ordering.
- `where: { name?: String }` — optional partial Interpreted Name filter
- `order: { by: NAME | REGISTRATION_TIMESTAMP | REGISTRATION_EXPIRY, dir: ASC | DESC }` — ordering

.changeset/refactor-find-domains-layers.md:21

  • The changeset documents that where.name is now required for Query.domains, but does not document that where.owner has been removed. This is a breaking change that should be explicitly documented. Consider adding a note like: "Breaking: where.owner has been removed from Query.domains - use the new Account.domains field instead to query domains by owner."
**`Query.domains`** (updated) — `where.name` is now required. Added optional `where.canonical` filter (defaults to false).
- `where: { name: String!, canonical?: Boolean }` — required partial Interpreted Name, optional canonical filter
- `order: { by: NAME | REGISTRATION_TIMESTAMP | REGISTRATION_EXPIRY, dir: ASC | DESC }` — ordering

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

@vercel vercel bot temporarily deployed to Preview – admin.ensnode.io February 25, 2026 17:15 Inactive
@vercel vercel bot temporarily deployed to Preview – ensrainbow.io February 25, 2026 17:15 Inactive
@vercel vercel bot temporarily deployed to Preview – ensnode.io February 25, 2026 17:15 Inactive
Copy link
Contributor

@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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/ensapi/src/graphql-api/lib/find-domains/find-domains.ts (1)

104-109: 🧹 Nitpick | 🔵 Trivial

orderFindDomains is the only exported function without a doc comment.

Every other exported function in this file (cursorFilter, isEffectiveDesc) and the private getOrderColumn carry JSDoc blocks. A brief comment here would restore consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ensapi/src/graphql-api/lib/find-domains/find-domains.ts` around lines
104 - 109, The exported function orderFindDomains lacks a JSDoc comment like the
other exports; add a brief JSDoc block above orderFindDomains describing its
purpose, parameters and return type (e.g., that it builds SQL ordering clauses
for DomainsCTE using orderBy: DomainsOrderBy, orderDir: OrderDirection and
inverted flag) and reference any relevant helpers such as getOrderColumn,
cursorFilter and isEffectiveDesc to keep documentation consistent with the rest
of the file.
🤖 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/graphql-api/lib/find-domains/find-domains.ts`:
- Line 36: Remove the redundant JSDoc `@returns` tag that restates the TypeScript
return type (: SQL) for the function that builds the cursor filter SQL
expression; locate the JSDoc immediately above the function returning SQL (the
cursor-filter builder) and delete the line "@returns SQL expression for the
cursor filter", leaving the rest of the comment intact.

In `@apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-name.ts`:
- Around line 37-56: The schema allows empty string names, causing
filterByName(base, name) to receive "" and skip the LIKE filter; update the
GraphQL input definitions by adding minLength: 1 to the name fields on the four
WhereInput types so empty strings are rejected: add minLength: 1 to the name
field of DomainsWhereInput, AccountDomainsWhereInput, RegistryDomainsWhereInput,
and SubdomainsWhereInput (the GraphQL schema input definitions that declare the
name property) so queries always provide at least one character and filterByName
will receive a non-empty string.

---

Outside diff comments:
In `@apps/ensapi/src/graphql-api/lib/find-domains/find-domains.ts`:
- Around line 104-109: The exported function orderFindDomains lacks a JSDoc
comment like the other exports; add a brief JSDoc block above orderFindDomains
describing its purpose, parameters and return type (e.g., that it builds SQL
ordering clauses for DomainsCTE using orderBy: DomainsOrderBy, orderDir:
OrderDirection and inverted flag) and reference any relevant helpers such as
getOrderColumn, cursorFilter and isEffectiveDesc to keep documentation
consistent with the rest of the file.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e772ab and 689000c.

📒 Files selected for processing (17)
  • .changeset/refactor-find-domains-layers.md
  • apps/ensapi/src/graphql-api/lib/canonical-registries-cte.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/find-domains.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/base-domain-set.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-canonical.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-name.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-owner.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-parent.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/filter-by-registry.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/index.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/layers/with-ordering-metadata.ts
  • apps/ensapi/src/graphql-api/lib/find-domains/types.ts
  • apps/ensapi/src/graphql-api/schema/account.ts
  • apps/ensapi/src/graphql-api/schema/domain.ts
  • apps/ensapi/src/graphql-api/schema/query.ts
  • apps/ensapi/src/graphql-api/schema/registry.ts

Copilot AI review requested due to automatic review settings February 25, 2026 17:22
@vercel vercel bot temporarily deployed to Preview – ensnode.io February 25, 2026 17:23 Inactive
@vercel vercel bot temporarily deployed to Preview – admin.ensnode.io February 25, 2026 17:23 Inactive
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

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

Comments suppressed due to low confidence (1)

.changeset/refactor-find-domains-layers.md:3

  • The changeset specifies this as a patch release, but the PR introduces breaking changes to Query.domains (making where.name required and removing where.owner). This should be at least a minor version bump, or possibly major depending on the versioning policy of this package.

Additionally, the changeset doesn't explicitly document the removal of where.owner from Query.domains and the recommended migration path (use Account.domains instead). Consider adding this breaking change to the changeset description.

---
"ensapi": patch
---

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

@vercel vercel bot temporarily deployed to Preview – ensrainbow.io February 25, 2026 17:37 Inactive
@vercel vercel bot temporarily deployed to Preview – ensnode.io February 25, 2026 17:37 Inactive
@vercel vercel bot temporarily deployed to Preview – admin.ensnode.io February 25, 2026 17:37 Inactive
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

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver-helpers.test.ts:5

  • This mock references a file path that no longer exists. The v1DomainsByLabelHashPath and v2DomainsByLabelHashPath functions were moved into layers/filter-by-name.ts as part of this refactor. Since this test file only imports isEffectiveDesc from find-domains-resolver-helpers and doesn't use anything from the previously separate labelhash-path file, this mock can be safely removed.
    .changeset/refactor-find-domains-layers.md:18
  • The changeset doesn't document that Registry.domains has a breaking return type change from ENSv2Domain to DomainInterface. While in practice this may only return v2 domains (since only v2 domains have a registryId), the GraphQL schema type has changed, which could affect clients performing type checks or fragment matching. This should be documented in the changeset as a breaking change.
**`Registry.domains`** (enhanced) — paginated connection of domains in this registry, now with filtering and ordering.
- `where: { name?: String }` — optional partial Interpreted Name filter
- `order: { by: NAME | REGISTRATION_TIMESTAMP | REGISTRATION_EXPIRY, dir: ASC | DESC }` — ordering


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

@vercel vercel bot temporarily deployed to Preview – ensnode.io February 25, 2026 17:52 Inactive
@vercel vercel bot temporarily deployed to Preview – ensrainbow.io February 25, 2026 17:52 Inactive
@vercel vercel bot temporarily deployed to Preview – admin.ensnode.io February 25, 2026 17:52 Inactive
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.

@shrugs Beautiful refactoring. Definitely helps make this logic easier to wrap your head around.

@shrugs shrugs merged commit a5f9178 into main Feb 25, 2026
17 checks passed
@shrugs shrugs deleted the feat/refactor-find-domains-availability branch February 25, 2026 19:17
@github-actions github-actions bot mentioned this pull request Feb 25, 2026
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.

3 participants