Skip to content

hotfix: correctly embed new embedded data entities#2163

Merged
shrugs merged 1 commit into
mainfrom
hotfix/embedded-data
May 20, 2026
Merged

hotfix: correctly embed new embedded data entities#2163
shrugs merged 1 commit into
mainfrom
hotfix/embedded-data

Conversation

@shrugs
Copy link
Copy Markdown
Member

@shrugs shrugs commented May 20, 2026

see title

@shrugs shrugs requested a review from a team as a code owner May 20, 2026 21:05
Copilot AI review requested due to automatic review settings May 20, 2026 21:05
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 20, 2026

⚠️ No Changeset found

Latest commit: a0fe23f

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

This PR includes no changesets

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

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

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

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 20, 2026

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

Project Deployment Actions Updated (UTC)
enskit-react-example.ensnode.io Ready Ready Preview, Comment May 20, 2026 9:06pm
3 Skipped Deployments
Project Deployment Actions Updated (UTC)
admin.ensnode.io Skipped Skipped May 20, 2026 9:06pm
ensnode.io Skipped Skipped May 20, 2026 9:06pm
ensrainbow.io Skipped Skipped May 20, 2026 9:06pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ac869acb-2e94-4724-b4fc-1383cfa2ef35

📥 Commits

Reviewing files that changed from the base of the PR and between 53c4573 and a0fe23f.

📒 Files selected for processing (2)
  • apps/ensapi/src/omnigraph-api/schema/scalars.ts
  • packages/enskit/src/react/omnigraph/_lib/cache-exchange.ts

📝 Walkthrough

Walkthrough

Interpreted scalar parsing logic is simplified by removing helper function calls and using direct type casts, while graphcache is configured to treat three additional entity types as non-keyable embedded data to suppress cache warnings.

Changes

Omnigraph Schema and Cache Updates

Layer / File(s) Summary
Interpreted scalar parsing refactoring
apps/ensapi/src/omnigraph-api/schema/scalars.ts
asInterpretedName and asInterpretedLabel helper imports are replaced with direct type imports. The parseValue transform logic for both InterpretedName and InterpretedLabel scalars now casts the validated string directly to the target type instead of calling the removed helpers.
Graphcache key configuration for embedded entities
packages/enskit/src/react/omnigraph/_lib/cache-exchange.ts
Graphcache keys configuration extends the embedded-data mapping to include CanonicalName, DomainCanonical, and DomainResolver, marking them as non-keyable entities.

🎯 2 (Simple) | ⏱️ ~8 minutes

🐰 The scalars dance with fewer friends,
Yet cast their truth without depends,
While cache marks entities at rest,
No warnings now to fail the test! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is almost entirely empty ('see title'), failing to provide required sections like Summary, Why, Testing, or Pre-Review Checklist as specified in the template. Complete all required sections of the description template: Summary (1-3 bullets), Why (explain motivation/linked issues), Testing (describe testing approach), and Pre-Review Checklist.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'hotfix: correctly embed new embedded data entities' clearly summarizes the main change: properly configuring embedded data entities in the cache.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/embedded-data

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This hotfix addresses two gaps introduced when new schema entity types were added: missing cache-exchange registrations in enskit and a broken import in the ensapi scalar parser.

  • cache-exchange.ts: Registers CanonicalName, DomainCanonical, and DomainResolver as EMBEDDED_DATA in urql's graphcache keys map. Without this, graphcache would emit warnings for every response containing these types because it could not find an id field to normalize them by. A schema audit confirms all non-keyable object types are now covered.
  • scalars.ts: Swaps asInterpretedName/asInterpretedLabel (not part of the public enssdk API surface) for direct TypeScript casts in the zod .transform() step. The casts are safe because the preceding .check() block already calls isInterpretedName/isInterpretedLabel and rejects invalid input before .transform() is reached.

Confidence Score: 5/5

Both changes are targeted and correct — safe to merge.

The cache-exchange addition covers exactly the three new entity types that lack an id field in the schema, and a schema audit confirms no other non-keyable types are missing. The scalar change removes a redundant validation call that was already performed by the zod .check() guard immediately before it, and the functions being removed were never part of the public enssdk API.

No files require special attention.

Important Files Changed

Filename Overview
packages/enskit/src/react/omnigraph/_lib/cache-exchange.ts Adds CanonicalName, DomainCanonical, and DomainResolver to the EMBEDDED_DATA keys map so urql's graphcache does not warn about un-normalizable entities. Schema audit confirms all non-keyable types are now covered.
apps/ensapi/src/omnigraph-api/schema/scalars.ts Replaces asInterpretedName/asInterpretedLabel (not exported from the enssdk public API) with direct TypeScript casts in the zod transform step. Safe because the preceding .check() call already performs the same isInterpretedName/isInterpretedLabel guard.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GraphQL Response] --> B{urql graphcache}
    B --> C{Has id field?}
    C -- Yes --> D[Normalize by id]
    C -- No --> E{In keys map?}
    E -- EMBEDDED_DATA --> F[Store inline, no normalization]
    E -- Not registered --> G[⚠️ Warning: cannot normalize]

    subgraph Previously Missing
        H[CanonicalName]
        I[DomainCanonical]
        J[DomainResolver]
    end

    subgraph Already Registered
        K[Label]
        L[WrappedBaseRegistrarRegistration]
        M[AccountId - custom key fn]
    end

    H & I & J --> F
    K & L --> F
Loading

Reviews (1): Last reviewed commit: "hotfix: correctly embed new embedded dat..." | Re-trigger Greptile

@shrugs shrugs merged commit 8220c38 into main May 20, 2026
21 checks passed
@shrugs shrugs deleted the hotfix/embedded-data branch May 20, 2026 21:14
@coderabbitai coderabbitai Bot mentioned this pull request May 21, 2026
2 tasks
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.

1 participant