Skip to content

schema improvements - #547

Merged
nikgraf merged 3 commits into
mainfrom
ng/schema-improvements
Oct 13, 2025
Merged

schema improvements#547
nikgraf merged 3 commits into
mainfrom
ng/schema-improvements

Conversation

@nikgraf

@nikgraf nikgraf commented Oct 13, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@nikgraf
nikgraf force-pushed the ng/schema-improvements branch from ffece02 to 7c31882 Compare October 13, 2025 11:01
@nikgraf
nikgraf merged commit 4bf4626 into main Oct 13, 2025
6 checks passed
@nikgraf
nikgraf deleted the ng/schema-improvements branch October 13, 2025 13:21
@nikgraf
nikgraf requested a review from Copilot October 13, 2025 13:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 schema definition API, moving from EntitySchema to Entity.Schema and consolidating schema-related functionality. The changes improve the API consistency and organization by using the Entity namespace for all entity-related operations.

Key changes:

  • Replace EntitySchema function calls with Entity.Schema throughout the codebase
  • Rename entity.ts to schema.ts and move schema functions there
  • Remove direct EntitySchema exports and update import statements
  • Update documentation to reflect the new schema API and remove mapping-related content

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/hypergraph/src/entity/schema.ts Renames EntitySchema function to Schema and updates Effect Schema imports
packages/hypergraph/src/index.ts Removes EntitySchema export in favor of Entity namespace
packages/hypergraph/test/entity/*.test.ts Updates test files to use Entity.Schema instead of EntitySchema
packages/hypergraph/src/entity/*.ts Updates imports to reference schema.ts instead of entity.ts
docs/*.md Updates documentation with new schema API and removes mapping references
apps/*/src/schema.ts Updates application schemas to use new Entity.Schema syntax

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 13 to +27
* Type utility to transform relation fields to accept string arrays or be omitted entirely.
* This specifically targets Type.Relation fields which are arrays of objects.
*/
type RelationArrayKeys<T> = {
[K in keyof T]: T[K] extends readonly (infer U)[]
? U extends object
? K
// For each property K in T, check if it's an array of objects (relation fields)
[K in keyof T]: T[K] extends readonly (infer U)[] // Check for readonly arrays
? U extends object // If array element is an object
? K // Return the property key K
: never
: T[K] extends (infer U)[]
? U extends object
? K
: T[K] extends (infer U)[] // Check for mutable arrays
? U extends object // If array element is an object
? K // Return the property key K
: never
: never;
}[keyof T];
: never; // Not an array of objects, so exclude
}[keyof T]; // Extract the union of all matching property keys

Copilot AI Oct 13, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] These comments are overly verbose and explain basic TypeScript type operations that are self-evident from the code. Consider simplifying to focus on the business logic rather than TypeScript mechanics.

Copilot uses AI. Check for mistakes.
* name: "grc-20-name",
* age: "grc-20-age"
* name: "grc-20-id",
* age: "grc-20-id"

Copilot AI Oct 13, 2025

Copy link

Choose a reason for hiding this comment

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

Both properties are mapped to the same 'grc-20-id' which is incorrect. The second property should have a unique ID like 'grc-20-age-id'.

Suggested change
* age: "grc-20-id"
* age: "grc-20-age-id"

Copilot uses AI. Check for mistakes.
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.

2 participants