Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces configuration overrides for relation and value space filters in GraphQL query generation. Users can now specify _config: { relationSpaces, valueSpaces } within relation include branches to control which spaces are queried, supporting either specific space lists or 'all' to bypass filtering.
Key changes:
- Added
RelationSpacesOverrideandRelationIncludeConfigtypes to support space filter configuration - Modified query building functions to honor space overrides when generating GraphQL fragments
- Updated type extraction logic to propagate config overrides through nested relations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/src/entity/types.ts | Defines new types for relation space overrides and adds _config property to RelationIncludeBranch |
| packages/hypergraph/src/utils/relation-query-helpers.ts | Updates filter-building functions to apply space overrides when constructing GraphQL queries |
| packages/hypergraph/src/utils/get-relation-type-ids.ts | Extracts and propagates _config space overrides from include branches to RelationTypeIdInfo objects |
| packages/hypergraph/test/utils/relation-config-overrides.test.ts | Adds tests verifying that space overrides are correctly propagated to generated query fragments |
| .changeset/warm-oranges-check.md | Documents the new feature for the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (override.length === 0) { | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
When override is an empty array, the function returns an empty string, which means no filter is applied. However, an empty array of spaces likely indicates that no results should be returned. Consider returning a filter that matches nothing, such as '(filter: {spaceId: {in: []}})' or adding documentation explaining this behavior is intentional.
| }, | ||
| } | ||
| ``` | ||
|
No newline at end of file |
There was a problem hiding this comment.
Remove trailing whitespace at the end of the file.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [key: string]: RelationIncludeBranch | boolean | undefined; | ||
| _config?: RelationIncludeConfig; | ||
| } & { | ||
| [key: string]: RelationIncludeBranch | RelationIncludeConfig | boolean | undefined; |
There was a problem hiding this comment.
The type definition allows RelationIncludeConfig to appear anywhere in the branch structure, but _config is a special reserved key. The index signature [key: string]: RelationIncludeBranch | RelationIncludeConfig | boolean | undefined could allow a user to accidentally create a property called _config at any level, which may conflict with the intended special _config property defined in the intersection. Consider restricting the index signature to exclude the _config key explicitly, or documenting this behavior clearly.
| [key: string]: RelationIncludeBranch | RelationIncludeConfig | boolean | undefined; | |
| [K in Exclude<string, '_config'>]?: RelationIncludeBranch | RelationIncludeConfig | boolean; |
| ``` | ||
|
No newline at end of file |
There was a problem hiding this comment.
Trailing whitespace at the end of the file should be removed.
| ``` | |
| }, | ||
| }, | ||
| }); | ||
| console.log({ person, personInvalidEntity, personInvalidRelationEntities }); |
There was a problem hiding this comment.
[nitpick] The console.log statement appears to be debug code that should likely be removed before merging to production.
| console.log({ person, personInvalidEntity, personInvalidRelationEntities }); |
docs/docs/query-public-data.md
Outdated
| - `relationSpaces` controls which spaces are searched for the relation edges themselves (`relations`/`backlinks`). Pass an array to whitelist specific spaces, `'all'` to drop the filter entirely, or `[]` if you intentionally want the branch to match nothing. | ||
| - `valueSpaces` applies the same override to the `valuesList` lookups for the related entities. This lets you fetch relation edges from one space while trusting the canonical values that live in another. | ||
|
|
||
| Overrides cascade to nested branches, so you can attach `_config` anywhere within the two supported include levels. Mix and match the settings per branch to stitch together data that spans multiple public spaces without issuing separate queries. |
There was a problem hiding this comment.
The phrase "Overrides cascade to nested branches" might be misleading. Based on the implementation, overrides don't actually cascade - each branch must explicitly specify its own _config if it wants to override the defaults. Consider clarifying this to avoid confusion, e.g., "Each nested branch can have its own _config settings."
| Overrides cascade to nested branches, so you can attach `_config` anywhere within the two supported include levels. Mix and match the settings per branch to stitch together data that spans multiple public spaces without issuing separate queries. | |
| Each nested branch can have its own `_config` settings, so you can attach `_config` anywhere within the two supported include levels. Mix and match the settings per branch to stitch together data that spans multiple public spaces without issuing separate queries. |
closes #568