-
Notifications
You must be signed in to change notification settings - Fork 104
H-5134: Create standalone Petri net editor – Petrinaut
#7752
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
…angeset for @blockprotocol/type-system
19043a5 to
6da0660
Compare
| } | ||
|
|
||
| return false; | ||
| }, [arcs, nodes, tokenTypes, persistedNet, title, parentProcess]); |
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.
The persistToGraph function lacks error handling, which means if any of the database operations fail, the persistPending state will remain true forever, leaving the UI in a broken state.
View Details
📝 Patch Details
diff --git a/apps/hash-frontend/src/pages/process.page/process-editor-wrapper/use-process-save-and-load.tsx b/apps/hash-frontend/src/pages/process.page/process-editor-wrapper/use-process-save-and-load.tsx
index 68ddde592..366220c70 100644
--- a/apps/hash-frontend/src/pages/process.page/process-editor-wrapper/use-process-save-and-load.tsx
+++ b/apps/hash-frontend/src/pages/process.page/process-editor-wrapper/use-process-save-and-load.tsx
@@ -219,7 +219,8 @@ export const useProcessSaveAndLoad = ({
setPersistPending(true);
- let persistedEntityId = selectedNetId;
+ try {
+ let persistedEntityId = selectedNetId;
if (selectedNetId) {
await updateEntity({
@@ -591,11 +592,12 @@ export const useProcessSaveAndLoad = ({
}
}
- await refetchPersistedNets({ updatedEntityId: persistedEntityId });
- setSelectedNetId(persistedEntityId);
- setUserEditable(true);
-
- setPersistPending(false);
+ await refetchPersistedNets({ updatedEntityId: persistedEntityId });
+ setSelectedNetId(persistedEntityId);
+ setUserEditable(true);
+ } finally {
+ setPersistPending(false);
+ }
}, [
activeWorkspaceWebId,
archiveEntity,
Analysis
persistToGraph function lacks error handling causing UI to break on database failures
What fails: persistToGraph function in use-process-save-and-load.tsx has no try-catch around database operations (updateEntity, createEntity, archiveEntity), leaving persistPending state stuck at true when mutations fail
How to reproduce:
// Any database operation failure (network error, permission denied, etc.) in persistToGraph
// For example: network disconnection during updateEntity call at line 225Result: setPersistPending(false) at line 598 never executes, edit bar permanently hidden (isDirty && !persistPending condition fails), user cannot save again
Expected: setPersistPending(false) should execute in finally block to restore UI functionality regardless of database operation success/failure
Fix applied: Wrapped function body in try-finally block ensuring setPersistPending(false) always executes per Apollo error handling docs
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 10002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 5001 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 27604 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 13450 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 11308 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 5628 | Flame Graph |
policy_resolution_large
| 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: 1001 | 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: 1526 | 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: 51 | 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: 107 | 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: 1 | 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: 25 | 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: 26 | 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 |
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 | 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=0, PT=0, ET=2, E=2 | Flame Graph | |
| entity_by_property | depths: DT=0, PT=2, ET=2, 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 | |
| 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=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=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 |
scaling_read_entity_complete_one_depth
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 25 entities | Flame Graph | |
| entity_by_id | 5 entities | Flame Graph | |
| entity_by_id | 50 entities | Flame Graph |
scaling_read_entity_complete_zero_depth
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 25 entities | Flame Graph | |
| entity_by_id | 5 entities | Flame Graph | |
| entity_by_id | 50 entities | Flame Graph |
scaling_read_entity_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 |
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?
We want to use the Petri net editor in other projects/repos, as well as making it place nicely with change management systems that require changes by mutation rather than creating data with new identity (e.g. Automerge).
@blockprotocol/corepatch (I have published a version that includes the patch)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: