2003 add new app with endpoint for submitting labels#2015
Draft
2003 add new app with endpoint for submitting labels#2015
Conversation
…ation - Introduced `apps/ens-labels-collector`, a Hono server that accepts label submissions via `POST /api/submissions`. - Implemented classification of labels against ENSNode's Omnigraph index, emitting structured JSON logs for each submission. - Added a new `Query.labels` field in ENSApi for batch lookup of labels by their hashes. - Included necessary configurations, Dockerfile, and example environment variables for local development. - Comprehensive tests for submission handling and classification logic. This feature addresses issue [#2003](#2003).
- Updated the maximum number of raw labels accepted per `POST /api/submissions` from 50 to 100. - Adjusted the `LABELS_BY_HASHES_MAX` limit in the ENSApi from 100 to 200 to accommodate larger batch lookups. - Modified tests to reflect the new limits for both submissions and label hash queries. - Enhanced documentation to clarify the relationship between submission limits and hash expansions.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: d01acae The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 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:
📝 WalkthroughWalkthroughNew ens-labels-collector Hono service accepts POST /api/submissions, hashes and classifies submitted ENS labels via Omnigraph lookups, and emits one structured JSON line per submission to stdout. ENSApi adds Query.labels(by: LabelsByHashesInput!) for batch label lookup by hashes (deduped, capped at 200, omits missing). Changes
Sequence DiagramsequenceDiagram
participant Client
participant Collector as "ens-labels-collector\n(Hono)"
participant Omnigraph as "ENSApi / Omnigraph\n(GraphQL)"
participant DB as "Label Index DB"
Client->>Collector: POST /api/submissions { labels, callerAddress }
activate Collector
Collector->>Collector: Validate input\nnormalize address, hash labels
Collector->>Collector: Collect & dedupe lookup hashes
Collector->>Omnigraph: Query labels(by: { hashes })
activate Omnigraph
Omnigraph->>DB: SELECT labels WHERE labelHash IN (...)
DB-->>Omnigraph: [{ hash, interpreted }]
Omnigraph-->>Collector: [{ hash, interpreted }]
deactivate Omnigraph
Collector->>Collector: Classify labels\n(healed/unknown/absent)
Collector->>Collector: Emit single JSON log line to stdout
Collector-->>Client: 200 { submittedAt, callerAddress, results }
deactivate Collector
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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
There was a problem hiding this comment.
Pull request overview
Adds a new “labels collector” service and the supporting Omnigraph query for batched labelhash lookups, enabling external callers to submit raw labels and have them classified against ENSNode’s indexed label table.
Changes:
- Add
Query.labels(by: { hashes })to ENSApi Omnigraph schema with a hard cap (200) and integration tests. - Add new app
apps/ens-labels-collector(Hono) exposingPOST /api/submissions+/health, with label hashing/normalization, Omnigraph lookups, and JSONL stdout logging. - Regenerate
enssdk/omnigraphschema + introspection for the new query/input; update lockfile and add changeset.
Reviewed changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds new app importer + lockfile refresh (including patched dependency hashes). |
| packages/enssdk/src/omnigraph/generated/schema.graphql | Adds LabelsByHashesInput and Query.labels to generated schema. |
| packages/enssdk/src/omnigraph/generated/introspection.ts | Updates generated introspection for typed Omnigraph client. |
| apps/ensapi/src/omnigraph-api/schema/query.ts | Implements Query.labels resolver using Drizzle inArray with max-size enforcement. |
| apps/ensapi/src/omnigraph-api/schema/label.ts | Adds LabelsByHashesInput and LABELS_BY_HASHES_MAX. |
| apps/ensapi/src/omnigraph-api/schema/label.integration.test.ts | Integration coverage for Query.labels. |
| apps/ens-labels-collector/vitest.config.ts | Vitest project config for the new app. |
| apps/ens-labels-collector/tsconfig.json | TS config for the new app. |
| apps/ens-labels-collector/src/lib/omnigraph-client.ts | Typed Omnigraph query wrapper + memoized client. |
| apps/ens-labels-collector/src/lib/labels.ts | Hashing + lookup-hash collection + classification logic. |
| apps/ens-labels-collector/src/lib/labels.test.ts | Unit tests for hashing/classification helpers. |
| apps/ens-labels-collector/src/lib/error-response.ts | Standardized { message, details? } error response helper. |
| apps/ens-labels-collector/src/index.ts | Server startup + graceful shutdown wiring. |
| apps/ens-labels-collector/src/handlers/submissions.ts | POST /api/submissions handler: validation, lookup, classification, logging. |
| apps/ens-labels-collector/src/handlers/submissions.test.ts | Handler-level tests with mocked Omnigraph client. |
| apps/ens-labels-collector/src/handlers/health.ts | /health handler. |
| apps/ens-labels-collector/src/config.ts | Env config parsing + memoization. |
| apps/ens-labels-collector/src/app.ts | Route wiring + notFound/onError handling. |
| apps/ens-labels-collector/package.json | New app package manifest and scripts. |
| apps/ens-labels-collector/README.md | App documentation + usage/config. |
| apps/ens-labels-collector/Dockerfile | Containerization for the new service. |
| apps/ens-labels-collector/.env.local.example | Local env template. |
| .changeset/ens-labels-collector-app.md | Changeset for ens-labels-collector, ensapi, and enssdk. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+19
to
+35
| const closeServer = () => | ||
| new Promise<void>((resolve, reject) => | ||
| server.close((err) => { | ||
| if (err) reject(err); | ||
| else resolve(); | ||
| }), | ||
| ); | ||
|
|
||
| const gracefulShutdown = async () => { | ||
| try { | ||
| await closeServer(); | ||
| process.exit(0); | ||
| } catch (error) { | ||
| console.error("[ens-labels-collector] shutdown error", error); | ||
| process.exit(1); | ||
| } | ||
| }; |
Comment on lines
+43
to
+58
| export async function lookupLabels(hashes: readonly string[]): Promise<LabelHit[]> { | ||
| if (hashes.length === 0) return []; | ||
|
|
||
| const result = await getClient().omnigraph.query({ | ||
| query: LabelsByHashes, | ||
| variables: { hashes: hashes as `0x${string}`[] }, | ||
| }); | ||
|
|
||
| if (result.errors && result.errors.length > 0) { | ||
| throw new Error( | ||
| `Omnigraph labels query returned errors: ${result.errors.map((e) => e.message).join("; ")}`, | ||
| ); | ||
| } | ||
|
|
||
| return (result.data?.labels ?? []) as LabelHit[]; | ||
| } |
Comment on lines
+86
to
+92
| it("rejects requests over the maximum allowed hash count", async () => { | ||
| // generate (LABELS_BY_HASHES_MAX + 1) distinct labelhashes deterministically | ||
| const hashes: LabelHash[] = []; | ||
| for (let i = 0; i <= 200; i++) { | ||
| const hex = i.toString(16).padStart(64, "0"); | ||
| hashes.push(`0x${hex}` as LabelHash); | ||
| } |
![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: 11
🤖 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/ens-labels-collector/Dockerfile`:
- Around line 11-16: The runner stage in the Dockerfile runs as root; update the
runner stage to switch to a non-root user by creating or using an existing
unprivileged user (e.g., "node" or a custom uid/gid), ensure
ownership/permissions of WORKDIR (/app/apps/ens-labels-collector) are set to
that user, and add a USER instruction before the CMD so the container process
launched by CMD ["pnpm","start"] does not run as root; reference the runner
stage, WORKDIR, and CMD when making these changes.
- Around line 14-16: The Dockerfile lacks a HEALTHCHECK so orchestrators can't
detect runtime liveness/readiness; add a HEALTHCHECK instruction after the
EXPOSE 4444 (and before CMD ["pnpm","start"]) that probes the app's health
endpoint (e.g., HTTP GET to localhost:4444/health or the appropriate route) and
configure sensible options (--interval, --timeout, --start-period, --retries) so
failed probes mark the container unhealthy; ensure the probe command exits
non‑zero on failure so Docker/k8s receive correct health signals.
In `@apps/ens-labels-collector/src/app.ts`:
- Around line 15-18: The onError handler currently returns the raw error to
clients via errorResponse in app.onError; change it to log the full error
internally (using console.error or processLogger) but return a generic 500
response body (e.g., { error: "Internal Server Error" } or a minimal error id)
from errorResponse instead of the raw error object; keep the logging call for
app.onError((error, c) => ...) but remove/error-sanitize any exposure of error
when calling errorResponse so implementation details are not leaked to clients.
In `@apps/ens-labels-collector/src/config.ts`:
- Around line 4-8: The PORT schema currently uses Number.parseInt which allows
partial numeric strings like "4444abc"; update the transform for PORT in
config.ts to validate the raw string strictly (e.g. ensure it matches /^\d+$/)
before parsing and return the default 4444 when undefined, and otherwise parse
with Number.parseInt or Number(value); if the string fails the digits-only
check, throw or return a z.ZodError/invalid value so the .pipe(z.number()...)
never receives a truncated number—target the PORT zod schema's .transform
callback to implement this validation.
In `@apps/ens-labels-collector/src/handlers/submissions.test.ts`:
- Around line 33-38: The test suite creates a global console.log spy
(consoleSpy) but never restores it, which can leak a mocked console into other
tests; add an afterAll (or afterEach) teardown that calls
consoleSpy.mockRestore() to restore the original console.log; locate the
consoleSpy declaration and the existing beforeEach block in submissions.test.ts
and add afterAll(() => consoleSpy.mockRestore()) so the spy is removed when the
suite finishes.
- Around line 40-213: Tests in submissions.test.ts use an await-then-expect
pattern for async responses (e.g., calling await res.json() then asserting), and
the reviewer suggests optionally switching to Vitest's await
expect(promise).resolves.* pattern; to apply this refactor, locate the tests
that call app.request and then await res.json() (examples: the "classifies a
healed label correctly..." test, "classifies an unhealed label...", "normalizes
the callerAddress..." and others) and replace the manual await-and-then
assertions with direct promise assertions (e.g., use await
expect(res.json()).resolves.toMatchObject(...) or resolves.toEqual(...) where
appropriate), keeping status assertions either as expect(res.status).toBe(...)
or using await expect(Promise.resolve(res.status)).resolves.toBe(...); this is
optional—apply consistently across the file if you refactor.
In `@apps/ens-labels-collector/src/handlers/submissions.ts`:
- Line 110: The call to lookupLabels() is unbounded and can hang; wrap the
external Omnigraph lookup in a bounded timeout so the handler doesn't block
indefinitely—use a Promise.race (or an AbortController if lookupLabels accepts a
signal) to enforce a maximum wait (e.g., configurable constant) and handle
timeout by returning a clear error or fallback before assigning hits; update the
code around the lookupLabels call (where hits is awaited) to cancel/ignore the
lookup on timeout and log/propagate a timeout-specific error.
In `@apps/ens-labels-collector/src/index.ts`:
- Around line 27-35: The current gracefulShutdown function always exits with 0;
update it to accept an optional exitCode parameter (default 0) and pass that to
process.exit so callers can signal failure; then change the uncaughtException
handler (process.on('uncaughtException', ...)) to call gracefulShutdown(1) (or
invoke a dedicated fatalShutdown that logs the error and calls
gracefulShutdown(1)) so uncaught exceptions exit non‑zero while SIGINT/SIGTERM
continue to call gracefulShutdown() with the default 0. Ensure references to
gracefulShutdown and the uncaughtException handler are updated accordingly.
In `@apps/ens-labels-collector/src/lib/omnigraph-client.ts`:
- Around line 43-49: Change lookupLabels to accept readonly LabelHash[] instead
of readonly string[] and remove the unsafe cast to `0x${string}` in the query
variables; update the function signature for lookupLabels, remove the cast
currently applied to `hashes` when calling getClient().omnigraph.query with
LabelsByHashes, and pass the hashes array directly (ensuring LabelHash is
imported/available). Confirm callers (e.g., collectLookupHashes) already provide
LabelHash[] so no runtime changes are needed.
In `@apps/ensapi/src/omnigraph-api/schema/label.integration.test.ts`:
- Around line 89-91: The test currently hardcodes 200 when building hashes to
trigger the overflow; import the exported constant LABELS_BY_HASHES_MAX and use
it to generate one more than the cap (e.g. loop to <= LABELS_BY_HASHES_MAX) so
the test reliably creates LABELS_BY_HASHES_MAX+1 hashes; add the import for
LABELS_BY_HASHES_MAX and replace the literal 200 in the loop in
label.integration.test.ts (the block that builds hashes) with that constant.
- Around line 34-84: Refactor each test to assert the async request using the
"await expect(...).resolves" pattern instead of awaiting the result then calling
expect: call request<LabelsByHashesResult>(LabelsByHashes, {...}) inside await
expect(...).resolves and use resolves.toHaveLength(), resolves.toEqual(),
resolves.toMatchObject() (for the healed label object) and
resolves.toMatchObject() or resolves.toHaveProperty() for specific fields; for
the dedupe/presence tests use
resolves.toHaveLength()/resolves.toHaveProperty('labels.0.hash',
ETH_LABEL_HASH), and for the "interpreted !== encodeLabelHash" case express the
inequality via a resolves matcher (e.g.,
resolves.toHaveProperty('labels.0.interpreted', expect.not...)) so all
assertions use the resolves chain against the promise returned by request
(references: LabelsByHashes, request, LabelsByHashesResult, ETH_LABEL_HASH,
ABSENT_LABEL_HASH, encodeLabelHash).
🪄 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: f8a4ec9f-6c70-4113-8141-54936f4f333c
⛔ Files ignored due to path filters (3)
packages/enssdk/src/omnigraph/generated/introspection.tsis excluded by!**/generated/**packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.changeset/ens-labels-collector-app.mdapps/ens-labels-collector/.env.local.exampleapps/ens-labels-collector/Dockerfileapps/ens-labels-collector/README.mdapps/ens-labels-collector/package.jsonapps/ens-labels-collector/src/app.tsapps/ens-labels-collector/src/config.tsapps/ens-labels-collector/src/handlers/health.tsapps/ens-labels-collector/src/handlers/submissions.test.tsapps/ens-labels-collector/src/handlers/submissions.tsapps/ens-labels-collector/src/index.tsapps/ens-labels-collector/src/lib/error-response.tsapps/ens-labels-collector/src/lib/labels.test.tsapps/ens-labels-collector/src/lib/labels.tsapps/ens-labels-collector/src/lib/omnigraph-client.tsapps/ens-labels-collector/tsconfig.jsonapps/ens-labels-collector/vitest.config.tsapps/ensapi/src/omnigraph-api/schema/label.integration.test.tsapps/ensapi/src/omnigraph-api/schema/label.tsapps/ensapi/src/omnigraph-api/schema/query.ts
Comment on lines
+11
to
+16
| FROM deps AS runner | ||
| WORKDIR /app/apps/ens-labels-collector | ||
| ENV NODE_ENV=production | ||
| EXPOSE 4444 | ||
|
|
||
| CMD ["pnpm", "start"] |
Contributor
There was a problem hiding this comment.
Run the container as a non-root user.
The runner stage currently executes as root, which is an avoidable security risk.
🔧 Proposed fix
FROM deps AS runner
WORKDIR /app/apps/ens-labels-collector
ENV NODE_ENV=production
+USER node
EXPOSE 4444
CMD ["pnpm", "start"]📝 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
| FROM deps AS runner | |
| WORKDIR /app/apps/ens-labels-collector | |
| ENV NODE_ENV=production | |
| EXPOSE 4444 | |
| CMD ["pnpm", "start"] | |
| FROM deps AS runner | |
| WORKDIR /app/apps/ens-labels-collector | |
| ENV NODE_ENV=production | |
| USER node | |
| EXPOSE 4444 | |
| CMD ["pnpm", "start"] |
🧰 Tools
🪛 Checkov (3.2.525)
[low] 1-16: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-16: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ens-labels-collector/Dockerfile` around lines 11 - 16, The runner stage
in the Dockerfile runs as root; update the runner stage to switch to a non-root
user by creating or using an existing unprivileged user (e.g., "node" or a
custom uid/gid), ensure ownership/permissions of WORKDIR
(/app/apps/ens-labels-collector) are set to that user, and add a USER
instruction before the CMD so the container process launched by CMD
["pnpm","start"] does not run as root; reference the runner stage, WORKDIR, and
CMD when making these changes.
Comment on lines
+14
to
+16
| EXPOSE 4444 | ||
|
|
||
| CMD ["pnpm", "start"] |
Contributor
There was a problem hiding this comment.
Add a HEALTHCHECK for runtime readiness/liveness.
Without it, orchestrators have weaker signal for unhealthy instances.
🔧 Proposed fix
ENV NODE_ENV=production
EXPOSE 4444
+HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \
+ CMD node -e 'fetch("http://127.0.0.1:4444/health").then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))'
CMD ["pnpm", "start"]📝 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
| EXPOSE 4444 | |
| CMD ["pnpm", "start"] | |
| EXPOSE 4444 | |
| HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ | |
| CMD node -e 'fetch("http://127.0.0.1:4444/health").then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))' | |
| CMD ["pnpm", "start"] |
🧰 Tools
🪛 Checkov (3.2.525)
[low] 1-16: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-16: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ens-labels-collector/Dockerfile` around lines 14 - 16, The Dockerfile
lacks a HEALTHCHECK so orchestrators can't detect runtime liveness/readiness;
add a HEALTHCHECK instruction after the EXPOSE 4444 (and before CMD
["pnpm","start"]) that probes the app's health endpoint (e.g., HTTP GET to
localhost:4444/health or the appropriate route) and configure sensible options
(--interval, --timeout, --start-period, --retries) so failed probes mark the
container unhealthy; ensure the probe command exits non‑zero on failure so
Docker/k8s receive correct health signals.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…r submissions - Updated error handling in the app to prevent leaking underlying error messages to clients, responding with a generic 500 status instead. - Introduced a timeout mechanism for the `POST /api/submissions` endpoint to prevent stalled requests from holding resources indefinitely. - Added a new utility function to handle promise timeouts, ensuring graceful degradation in case of delays during label lookups. - Updated tests to include new error handling and timeout scenarios.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new labels-submission collector app and extends ENSApi/Omnigraph to support batch label lookups by labelhash, enabling callers to classify submitted labels against the ENSNode index.
Changes:
- Add
apps/ens-labels-collector(Hono server) withPOST /api/submissions+ stdout JSONL sink and label classification logic. - Add Omnigraph
Query.labels(by: LabelsByHashesInput!): [Label!]!with a 200-hash cap, plus integration tests. - Regenerate
enssdk/omnigraphgenerated schema + introspection to expose the new query to typed clients.
Reviewed changes
Copilot reviewed 21 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds new app importer deps + bumps esbuild snapshot used by toolchain. |
| packages/enssdk/src/omnigraph/generated/schema.graphql | Adds LabelsByHashesInput + Query.labels to generated schema. |
| packages/enssdk/src/omnigraph/generated/introspection.ts | Updates generated introspection to include new input + query field. |
| apps/ensapi/src/omnigraph-api/yoga.ts | Configures masked GraphQL errors (prod) with dev passthrough behavior. |
| apps/ensapi/src/omnigraph-api/schema/query.ts | Implements Query.labels resolver with dedupe + max hash cap. |
| apps/ensapi/src/omnigraph-api/schema/label.ts | Adds LabelsByHashesInput + LABELS_BY_HASHES_MAX. |
| apps/ensapi/src/omnigraph-api/schema/label.integration.test.ts | Adds integration coverage for Query.labels behavior and limits. |
| apps/ens-labels-collector/vitest.config.ts | Sets up unit-test project config for the new app. |
| apps/ens-labels-collector/tsconfig.json | Adds TS config for the new app with @/* path alias. |
| apps/ens-labels-collector/src/lib/omnigraph-client.ts | Adds typed Omnigraph client + cached client for label lookups. |
| apps/ens-labels-collector/src/lib/labels.ts | Adds hashing + normalization + classification utilities. |
| apps/ens-labels-collector/src/lib/labels.test.ts | Unit tests for label hashing/collection/classification utilities. |
| apps/ens-labels-collector/src/lib/error-response.ts | Standardizes JSON error responses for the new service. |
| apps/ens-labels-collector/src/index.ts | Adds server bootstrap + graceful shutdown handling. |
| apps/ens-labels-collector/src/handlers/submissions.ts | Implements submissions endpoint schema validation, lookup, classification, timeout, and logging. |
| apps/ens-labels-collector/src/handlers/submissions.test.ts | Unit tests for submissions handler validation, behavior, and logging. |
| apps/ens-labels-collector/src/handlers/health.ts | Adds liveness endpoint handler. |
| apps/ens-labels-collector/src/config.ts | Adds env config parsing/memoization for PORT + ENSNODE_URL. |
| apps/ens-labels-collector/src/app.ts | Wires routes + notFound + error handling for the new app. |
| apps/ens-labels-collector/package.json | Declares new workspace app package, scripts, and dependencies. |
| apps/ens-labels-collector/README.md | Documents endpoints, classification logic, and configuration. |
| apps/ens-labels-collector/Dockerfile | Provides container build/run recipe for the new app. |
| apps/ens-labels-collector/.env.local.example | Adds local dev env template. |
| .changeset/ens-labels-collector-app.md | Adds changeset for new app + Omnigraph API + regenerated SDK artifacts. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+34
to
+42
| * Races `promise` against a `setTimeout`-backed timeout, rejecting with `Error(message)` on | ||
| * expiry. The underlying promise is NOT cancelled (the Omnigraph SDK does not currently expose | ||
| * an `AbortSignal`); we simply stop waiting on it. The unhandled resolution is harmless. | ||
| */ | ||
| function withTimeout<T>(promise: Promise<T>, ms: number, message: string): Promise<T> { | ||
| let timer: ReturnType<typeof setTimeout> | undefined; | ||
| const timeout = new Promise<never>((_, reject) => { | ||
| timer = setTimeout(() => reject(new Error(message)), ms); | ||
| }); |
Comment on lines
+1
to
+5
| --- | ||
| "ens-labels-collector": minor | ||
| "ensapi": minor | ||
| "enssdk": minor | ||
| --- |
Comment on lines
+35
to
+41
| ## Configuration | ||
|
|
||
| | Env var | Required | Description | | ||
| |---------|----------|-------------| | ||
| | `PORT` | no (default `4444`) | HTTP listen port. | | ||
| | `ENSNODE_URL` | yes | Base URL of an ENSNode (ENSApi) instance with Omnigraph at `/api/omnigraph`. | | ||
|
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/ens-labels-collector/package.json`:
- Line 23: You added the workspace dependency "@ensnode/ensnode-sdk" to
package.json but did not update the lockfile; run the package manager to
regenerate and commit the lockfile (e.g., run pnpm install to update
pnpm-lock.yaml) so the new workspace dependency is reflected, then include that
updated lockfile commit alongside the package.json change.
In `@apps/ens-labels-collector/src/handlers/submissions.ts`:
- Around line 34-37: The lookupLabels function currently races the Omnigraph SDK
call with withTimeout but does not pass an AbortSignal, so timed-out requests
keep running; update lookupLabels to accept an optional AbortSignal (or
QueryOptions) and pass it into the Omnigraph query call (the SDK supports
AbortSignal via QueryOptions), and change withTimeout usage to create an
AbortController whose signal is passed to the SDK and that is aborted when the
timeout fires; if lookupLabels is called with an existing signal, wire it to
abort the controller (or merge signals) so caller cancellations propagate;
finally update callers of lookupLabels (the HTTP request handler paths around
the earlier call sites) to supply the request's AbortSignal or forward
cancellation so in-flight requests are actually aborted on timeout or client
disconnect.
In `@apps/ensapi/src/omnigraph-api/yoga.ts`:
- Around line 19-28: Replace the custom masking block inside maskedErrors so it
uses Yoga's exported maskError function and the app's Pino logger: instead of
calling console.error and returning raw Error instances, call the Yoga maskError
fallback (maskError) with the error and log the error via the existing Pino
logger (logger) before returning maskError's result; ensure maskError is
imported from Yoga and the logger variable used is the same Pino instance the
app uses.
🪄 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: a9de560f-e3b8-4c43-9c9b-f3068d0f74f5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/ens-labels-collector/package.jsonapps/ens-labels-collector/src/app.tsapps/ens-labels-collector/src/config.tsapps/ens-labels-collector/src/handlers/submissions.test.tsapps/ens-labels-collector/src/handlers/submissions.tsapps/ens-labels-collector/src/index.tsapps/ens-labels-collector/src/lib/omnigraph-client.tsapps/ensapi/src/omnigraph-api/schema/label.integration.test.tsapps/ensapi/src/omnigraph-api/yoga.ts
…issions, enhancing label classification and introducing `LabelHash` scalar. Update Docker configurations and workflows to include the new app.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 39 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ema` to transform labels into `LiteralLabel` and adjust `SubmissionResultItem` to use typed labels. Enhance tests to reflect these changes and ensure proper handling of raw and normalized labels.
…or distinct `LabelHash` inputs, update error messages for clarity, and allow duplicate `LabelHashes` within the maximum distinct count. Adjust related tests and documentation to reflect these changes.
…dling for invalid inputs, ensuring clearer error messages. Modify tests to validate mixed-case hex digits and reject uppercase `0X` prefixes for `LabelHash` variables.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 39 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…` and `discover()` methods, introduce `EnsRainbowBeamHttpError` for error handling, and ensure client-side validation aligns with server expectations. Update exports and configuration for new client integration.
…plement comprehensive unit tests covering validation logic, health checks, and discover functionality, ensuring proper error handling and response validation. Introduce `omnigraph-consts.ts` for server-side limits on label requests.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 47 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ent workflows for `ensrainbowbeam`, and enhance configuration with CORS origins. Update Terraform modules for deployment and add new environment variables. Enhance UI components for label submission and results display.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 58 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+17
| env_vars = { | ||
| PORT = { value = tostring(var.port) } | ||
| ENSNODE_URL = { value = var.ensnode_url } | ||
| } |
Comment on lines
+9
to
+12
| - "4444:4444" | ||
| environment: | ||
| ENSNODE_URL: http://ensapi:4334 | ||
| depends_on: |
Comment on lines
+168
to
+172
| {results.map((r) => ( | ||
| <li | ||
| key={`${r.rawLabel}-${r.status}`} | ||
| className="py-3 flex items-start justify-between gap-3" | ||
| > |
Comment on lines
+197
to
+207
| module "ensrainbowbeam" { | ||
| source = "./modules/ensrainbowbeam" | ||
|
|
||
| render_region = local.render_region | ||
| render_environment_id = render_project.ensnode.environments["default"].id | ||
| ensnode_version = var.ensnode_version | ||
|
|
||
| # Beam uses the Omnigraph-capable ENSApi endpoint for label classification. | ||
| # Default to the Alpha API hostname for this Render environment. | ||
| ensnode_url = "https://api.alpha.${var.render_environment}.${local.hosted_zone_name}" | ||
| } |
| type = number | ||
| default = 4444 | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Terraform module lacks cors_origins variable, preventing CORS configuration in production deployments which causes all CORS requests to be rejected
| type = number | ||
| default = 4444 | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Missing cors_origins variable in Terraform module prevents CORS configuration in production deployments
Comment on lines
+168
to
+170
| {results.map((r) => ( | ||
| <li | ||
| key={`${r.rawLabel}-${r.status}`} |
Contributor
There was a problem hiding this comment.
Suggested change
| {results.map((r) => ( | |
| <li | |
| key={`${r.rawLabel}-${r.status}`} | |
| {results.map((r, index) => ( | |
| <li | |
| key={`${index}-${r.rawLabel}-${r.status}`} |
React list key collision: key={ } is not unique, causing DOM reuse and UI bugs when duplicate labels are submitted
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
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.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)