Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
github-actions
Bot
added
area/apps > hash*
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9891 +/- ##
==========================================
+ Coverage 66.07% 66.21% +0.13%
==========================================
Files 1911 1913 +2
Lines 211128 211993 +865
Branches 8285 8349 +64
==========================================
+ Hits 139496 140361 +865
+ Misses 70070 70067 -3
- Partials 1562 1565 +3
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:
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
TimDiekmann
reviewed
Oct 2, 2026
Member
There was a problem hiding this comment.
Nothing you could have possibly known, but the goal is to migrate away from http-tests, mainly because they're very long to write and annoying to test locally.
I would keep the changes you added, but as a general flag I think it's worth pointing out.
In particular this file is already quite large, and the embedded embeddings make this even worse.
TimDiekmann
reviewed
Oct 2, 2026
Comment on lines
+69
to
+70
| } else if (isVersionExhausted) { | ||
| label = "– this type has reached the maximum version and cannot be updated"; |
montekki
commented
Oct 2, 2026
Comment on lines
83
to
87
| closed_schema: Valid::new_unchecked(ClosedEntityType { | ||
| id: VersionedUrl { | ||
| base_url: schema.id.base_url.clone(), | ||
| version: OntologyTypeVersion { | ||
| major: 0, | ||
| pre_release: None, | ||
| }, | ||
| }, | ||
| id: schema.id.clone(), | ||
| title: String::new(), | ||
| title_plural: None, | ||
| description: String::new(), |
Member
Author
There was a problem hiding this comment.
My assumption here that the schema is filled with dummy values anyway and is properly UPDATE-ed in entity_type.rs, so just cloning the whole schema.id would work here as well as any other id value.
montekki
marked this pull request as ready for review
October 2, 2026 13:51
PR SummaryMedium Risk Overview The Graph API uses checked version increments and returns 400 when publishing would exceed the max; bulk updates stay atomic. Store/authorization paths reject zero majors (including Postgres decode and Cedar policy parsing). Snapshot restore no longer seeds placeholder closed schemas at v/0. HASH frontend stops using version 0 for drafts: Reviewed by Cursor Bugbot for commit c2d12a1. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 c2d12a1. Configure here.
| validateVersionedUrl(`${dataTypeBaseUrl}v/${requestedVersionString}`) | ||
| .type === "Err" | ||
| ) { | ||
| return <NotFound resourceLabel={{ label: "data type" }} />; |
There was a problem hiding this comment.
NotFound copy drops the article
Low Severity
The new invalid-version NotFound calls pass only label, so authenticated users see “If you believe you should be able to see here”. Other type pages supply withArticle, and NotFound interpolates that field with no fallback. The same gap is on the external entity-type route.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit c2d12a1. Configure here.
indietyp
reviewed
Oct 2, 2026
| )), | ||
| vec![], | ||
| )) | ||
| })?; |
Member
There was a problem hiding this comment.
shouldn't this be an error enough (that we can then also e.g. reference), instead of directly being a stringly status? (I guess not as important in the sense that we're about to port to the public API)
Member
There was a problem hiding this comment.
It may make sense to create a newtype over NonZero<u32> to encode the major version (and similarly for the minor version). That way, we could encode the error that way and just call report_to_response, since we currently duplicate the same error in more than half a dozen places.
Comment on lines
956
to
+961
| .major | ||
| .get() | ||
| .checked_sub(1) | ||
| .ok_or(UpdateError) | ||
| .and_then(NonZero::new) | ||
| .ok_or(OntologyVersionDoesNotExist) | ||
| .change_context(UpdateError) |
Member
There was a problem hiding this comment.
Having a dedicated type that enforces NonZero would also allow this to be: .previous().change_context() instead.
Contributor
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 |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🌟 What is the purpose of this PR?
Ontology type versions start at 1, but the Rust and TypeScript validators currently accept 0. This change rejects zero versions and updates the Graph and frontend callers to use valid versions.
🔗 Related links
🔍 What does this change?
NonZero<u32>and rejects zero during parsing and PostgreSQL decoding. Graph version increments usechecked_add, and updates without a valid predecessor retain the existing error type.makeOntologyTypeVersionaccepts only integers from 1 throughu32::MAX.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
📜 Does this require a change to the docs?
🕸️ Does this require a change to the Turbo Graph?
🛡 What tests cover this?
The broader integration run had 23 failures because Kratos, Temporal, and MinIO were unavailable locally. The TypeScript coverage run also discovered compiled AI-worker tests in
dist, which require a Graph API and include paths to uncopied fixtures; the AI-worker source tests passed with coverage whendistwas excluded.❓ How to test this?
Run
cargo check --all --all-features --all-targetsandcargo clippy --workspace --all-features --all-targets --no-deps -- -D warnings.Run
yarn workspace @blockprotocol/type-system test:unitandyarn workspace @apps/hash-frontend test:unitafter building their dependencies. With PostgreSQL running, runcargo nextest run -p hash-graph-integration --all-features.