BE-590: hgraph: Refactor entity query handlers into entity/query submodule#8840
BE-590: hgraph: Refactor entity query handlers into entity/query submodule#8840indietyp wants to merge 17 commits into
entity/query submodule#8840Conversation
chore: add new dependency chore: format feat: error module feat: introduce hashql_eval interner chore: checkpoint feat: checkpoint feat: checkpoint chore: remove old value module feat: checkpoint feat: checkpoint feat: checkpoint feat: checkpoint feat: checkpoint chore: checkpoint feat: move entity query into its own modul fix: query request feat: checkpoint (it compiles!) feat: checkpoint feat: checkpoint feat: checkpoint fix: issue around cached thunking feat: covariance for opaque inners fix: cfgattr serde chore: remove graph module fix: merge fuckup
PR SummaryHigh Risk Overview API contract: Dependencies: Other: Graph query benchmarks import the new request types and call Reviewed by Cursor Bugbot for commit 6daec9b. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8840 +/- ##
==========================================
+ Coverage 59.61% 59.74% +0.13%
==========================================
Files 1366 1368 +2
Lines 132649 132846 +197
Branches 6044 6052 +8
==========================================
+ Hits 79078 79370 +292
+ Misses 52637 52532 -105
- Partials 934 944 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Refactors the entity query REST API by moving query request types and handlers into a dedicated rest/entity/query submodule, while removing the legacy entity_query_request.rs module (including dead/unused HashQL-related code paths). This simplifies the request shapes (direct filter struct field; two-variant subgraph request) and updates benchmarks accordingly.
Changes:
- Extracts entity query handlers to
libs/@local/graph/api/src/rest/entity/query/mod.rsand request types (plus deserialization tests) to.../entity/query/request.rs. - Removes
libs/@local/graph/api/src/rest/entity_query_request.rsand theInteractiveheader from entity query OpenAPI definitions/handlers. - Updates manual benchmark scaffolding to use the new request types and APIs directly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/graph/benches/manual_queries/entity_queries/mod.rs | Updates benchmarks to use the refactored request types and the new into_*params APIs. |
| libs/@local/graph/api/src/rest/mod.rs | Drops the old entity_query_request module inclusion. |
| libs/@local/graph/api/src/rest/entity/query/request.rs | Introduces simplified request types and adds unit tests for deserialization/variant disambiguation. |
| libs/@local/graph/api/src/rest/entity/query/mod.rs | Adds extracted handlers and updated OpenAPI path definitions without the Interactive header. |
| libs/@local/graph/api/src/rest/entity/mod.rs | Wires the new submodule into routing and OpenAPI, removing the inlined legacy handlers/types. |
| libs/@local/graph/api/src/rest/entity_query_request.rs | Removes the legacy request module (dead HashQL/proxy-related code). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
044a4ff to
c1e8a90
Compare
8af8844 to
82bb954
Compare
233e8eb to
0f62210
Compare
0f62210 to
e9a1230
Compare
5ac6afb to
7083934
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
libs/@local/graph/api/src/rest/entity/mod.rs:4
- This refactor moves
QueryEntitiesRequest/QueryEntitySubgraphRequestunderentity::query, but the previous public re-exports fromhash_graph_api::rest::entity::*are gone. Sincehash-graph-apiis a published crate, this is a breaking Rust API change for downstream users even though HTTP behavior is unchanged.
Consider re-exporting the request/error types from entity to preserve the old import path (or add a deprecation cycle if you want to force the new path).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
libs/@local/graph/api/src/rest/entity/mod.rs:226
- This router change removes previously-exposed endpoints (
/entities/searchand/entities/query/summarize) and introduces/entities/query/count, but the PR description frames this as a refactor of entity query handlers and says the endpoints should behave as before. Also, the checked-in OpenAPI snapshot still contains/entities/searchand/entities/query/summarizeand does not contain/entities/query/count(seelibs/@local/graph/api/openapi/openapi.jsonaround the paths section). Please confirm whether these endpoints were intended to be removed/added; if so, update/regenerate the OpenAPI accordingly, and if not, restore the routes/handlers.
libs/@local/graph/api/openapi/openapi.json:1655 - The OpenAPI snapshot appears out of sync with the current router/handler set: it still documents
/entities/query/summarizeand/entities/search(operationIdssummarize_entities/search_entities), and it does not document the newly-added/entities/query/countendpoint. This will break client generation and makes it unclear what the supported API surface is. Regenerate/updateopenapi.jsonfrom the current utoipa definitions (or restore the removed endpoints if they are still intended to exist).
{
"name": "after",
"in": "query",
"description": "The cursor to start reading from",
"required": false,
Merging this PR will degrade performance by 15.38%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c65837f. Configure here.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
libs/@local/graph/api/src/rest/entity/mod.rs:440
- This block isn’t rustfmt-formatted (the
.change_context(...)call indentation is off). The repo’s Lint workflow runsjust format --checkfor Rust packages (.github/workflows/lint.yml:132-136), so this is likely to fail CI.
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/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/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/person/v/1
|
Flame Graph | |
| 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/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |


🌟 What is the purpose of this PR?
The entity query request types and their associated HTTP handler functions have been refactored out of a single flat module into a dedicated
entity/querysubmodule hierarchy. As part of this, the oldentity_query_request.rsfile — which contained a large amount of dead code related to a planned HashQL integration, proxy deserialization structs, and a four-variant subgraph enum — has been removed entirely.The new
QueryEntitiesRequestandQueryEntitySubgraphRequesttypes are simpler:QueryEntitiesRequestis now a plain struct with afilterfield directly (noEntityQueryenum wrapping a filter or raw HashQL value), andQueryEntitySubgraphRequestis a two-variant untagged enum (Paths/ResolveDepths) rather than four variants. Theinto_paramsandinto_traversal_paramsmethods are now on the request types themselves rather than on a separateEntityQueryOptionsstruct, removing an intermediate layer.The
Interactiveheader parameter has also been removed from thequery_entitiesandquery_entity_subgraphOpenAPI path definitions, as it was only relevant to the HashQL compilation path that no longer exists in these handlers.🔗 Related links
🔍 What does this change?
entity.rsintoentity/mod.rsand extracts query handlers intoentity/query/mod.rsand request types intoentity/query/request.rs.entity_query_request.rsand all dead code it contained:EntityQuery,EntityQueryOptions,FlatQueryEntitiesRequestData,CompilationOptions, HashQL diagnostic rendering, and the four-variantQueryEntitySubgraphRequest.EntityQuery::Filter/EntityQuery::Querysplit with a directfilter: Filter<'q, Entity>field onQueryEntitiesRequest.into_paramsandinto_traversal_paramsontoQueryEntitiesRequestandQueryEntitySubgraphRequestrespectively, eliminatingEntityQueryOptionsas a separate type.Interactiveheader fromquery_entitiesandquery_entity_subgraphhandler signatures and OpenAPI docs.tests/graph/benchesto use the simplified request types directly, removing theinto_parts/from_partsround-trips that previously extracted anEntityQueryandEntityQueryOptions.entity/query/request.rscovering deserialization of both request types, including required-field validation and correct disambiguation of thePathsvsResolveDepthssubgraph variants.🛡 What tests cover this?
libs/@local/graph/api/src/rest/entity/query/request.rscovering minimal and fully-populatedQueryEntitiesRequestdeserialization, rejection of missing required fields, and correct parsing of bothQueryEntitySubgraphRequestvariants including edge cases around ontology vs entity traversal edges.❓ How to test this?
cargo testinlibs/@local/graph/api./entities/query,/entities/query/subgraph, and/entities/query/countendpoints and confirm they behave as before.