-
Notifications
You must be signed in to change notification settings - Fork 95
H-4204: Remove @local/hash-subgraph
package
#6812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6812 +/- ##
==========================================
+ Coverage 30.73% 31.06% +0.32%
==========================================
Files 606 607 +1
Lines 23272 23408 +136
Branches 2965 2968 +3
==========================================
+ Hits 7152 7271 +119
- Misses 16063 16080 +17
Partials 57 57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 405 out of 406 changed files in this pull request and generated no comments.
Files not reviewed (1)
- apps/hash-ai-worker-ts/package.json: Language not supported
Comments suppressed due to low confidence (2)
apps/hash-ai-worker-ts/src/activities/graph.ts:257
- The addition of the 'false' parameter to the getEntities call changes its behavior. Please add a comment explaining its purpose to ensure future maintainers understand that this behavior is intentional.
return getEntities(deserializeSubgraph(params.subgraph), false).map((entity) => entity.toJSON()),
apps/hash-ai-worker-ts/src/activities/flow-activities/write-google-sheet-action/get-subgraph-from-filter.ts:43
- Parameterizing EntityRootType with HashEntity suggests a change in type expectations. Verify that the generic usage aligns with the intended design and update related documentation if necessary.
return mapGraphApiSubgraphToSubgraph<EntityRootType<HashEntity>>(
Benchmark results
|
Function | Value | Mean | Flame graphs |
---|---|---|---|
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph |
scaling_read_entity_complete_zero_depth
Function | Value | Mean | Flame graphs |
---|---|---|---|
entity_by_id | 10 entities | Flame Graph | |
entity_by_id | 5 entities | Flame Graph | |
entity_by_id | 50 entities | Flame Graph | |
entity_by_id | 25 entities | Flame Graph | |
entity_by_id | 1 entities | Flame Graph |
representative_read_entity_type
Function | Value | Mean | Flame graphs |
---|---|---|---|
get_entity_type_by_id | Account ID: d4e16033-c281-4cde-aa35-9085bf2e7579
|
Flame Graph |
scaling_read_entity_linkless
Function | Value | Mean | Flame graphs |
---|---|---|---|
entity_by_id | 10000 entities | Flame Graph | |
entity_by_id | 100 entities | Flame Graph | |
entity_by_id | 10 entities | Flame Graph | |
entity_by_id | 1000 entities | Flame Graph | |
entity_by_id | 1 entities | Flame Graph |
representative_read_multiple_entities
Function | Value | Mean | Flame graphs |
---|---|---|---|
entity_by_property | depths: DT=0, PT=2, ET=2, E=2 | Flame Graph | |
entity_by_property | depths: DT=0, PT=0, ET=0, E=0 | Flame Graph | |
entity_by_property | depths: DT=0, PT=0, ET=0, E=2 | Flame Graph | |
entity_by_property | depths: DT=2, PT=2, ET=2, E=2 | Flame Graph | |
entity_by_property | depths: DT=255, PT=255, ET=255, E=255 | Flame Graph | |
entity_by_property | depths: DT=0, PT=0, ET=2, E=2 | Flame Graph | |
link_by_source_by_property | depths: DT=0, PT=2, ET=2, E=2 | Flame Graph | |
link_by_source_by_property | depths: DT=0, PT=0, ET=0, E=0 | Flame Graph | |
link_by_source_by_property | depths: DT=0, PT=0, ET=0, E=2 | Flame Graph | |
link_by_source_by_property | depths: DT=2, PT=2, ET=2, E=2 | Flame Graph | |
link_by_source_by_property | depths: DT=255, PT=255, ET=255, E=255 | Flame Graph | |
link_by_source_by_property | depths: DT=0, PT=0, ET=2, E=2 | Flame Graph |
scaling_read_entity_complete_one_depth
Function | Value | Mean | Flame graphs |
---|---|---|---|
entity_by_id | 10 entities | Flame Graph | |
entity_by_id | 5 entities | Flame Graph | |
entity_by_id | 50 entities | Flame Graph | |
entity_by_id | 25 entities | Flame Graph | |
entity_by_id | 1 entities | Flame Graph |
@local/hash-subgraph
package@local/hash-subgraph
package
🌟 What is the purpose of this PR?
The
@local/hash-subgraph
package was used to redefine certain things in@blockprotocol/graph
, when the latter was in a different repository and for ease of development.Now that they're both in the same repo, it makes sense to consolidate them, so that there aren't multiple places exporting types with the same name, which removes the chance of importing the 'wrong' one. This PR does that.
As
@blockprotocol/graph
(and@blockprotocol/type-system
) are intended to be usable in non-HASH applications, the general principle of changes is that anything which is very HASH-specific goes into the@local/hash-graph-sdk
("the SDK"), and anything which is generalizable into one of the@blockprotocol
packages (graph
for working with a sub-graph,type-system
for defining types and entities).Notably:
@blockprotocol/type-system
defines a genericinterface
for anEntity
(e.g. for property and metadata access), which is then implemented byHashEntity
in the SDK (adding instance methods such as.patch
).Subgraph
in the GraphQL typedefs is renamed toGqlSubgraph
.@local/hash-subgraph
but not to@blockprotocol/graph
are ported over.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🐾 Next steps
There is scope for further package consolidation / rationalisation work (H-4205).
For example:
@local/hash-graph-types
contains very few types and could probably be consolidated into@local/hash-graph-sdk
on the basis that it contains types which are specific to working with the HASH Graph API, related to e.g. embeddings and entity validation. The equivalent Rust crate could stay where it is for now, and@local/hash-graph-types
import it.@local/hash-isomorphic-tools
. They could all be moved into@local/hash-graph-sdk
@local/hash-graph-client
still exports🛡 What tests cover this?
❓ How to test this?