Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Merging this PR will degrade performance by 15.93%1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## bm/fe-1862-upgrade-to-vite-5 #10011 +/- ##
================================================================
- Coverage 68.03% 68.03% -0.01%
================================================================
Files 2006 2006
Lines 206287 206286 -1
Branches 8815 8817 +2
================================================================
- Hits 140354 140348 -6
- Misses 64258 64262 +4
- Partials 1675 1676 +1
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:
|
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 |
533565d to
e69fb40
Compare
e69fb40 to
9c63f19
Compare
910e379 to
4dd74b8
Compare
4dd74b8 to
e405447
Compare
e405447 to
e0fd993
Compare
PR SummaryHigh Risk Overview The harpc client and graph SDK are updated to Effect 4 APIs: synchronous wire/binary codecs return
Reviewed by Cursor Bugbot for commit 7eaa3b6. Bugbot is set up for automated code reviews on this repo. Configure here. |
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 e0fd993. Configure here.
|
|
||
| // TODO: we might want to observe the task, for that we would need to have a partial connection that we then patch | ||
| yield* Effect.fork(task(self)); | ||
| yield* Effect.forkChild(task(self)); |
There was a problem hiding this comment.
Forked connection fibers die immediately
High Severity
Effect.fork was renamed to Effect.forkChild, but Effect 4 child fibers are interrupted when the parent succeeds. makeUnchecked and send return right after forking, so the response reader and request writer are cancelled immediately and RPC traffic never completes.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit e0fd993. Configure here.
lunelson
left a comment
There was a problem hiding this comment.
Not sure why this pinged Design System, it doesn't seem to overlap?
lunelson
left a comment
There was a problem hiding this comment.
I took a run at an agent review on this one, and came up with a handful of comments that may or may not be of value to you. They are posted as me but they are Claude. Main thing seems to be the "lag-drop question".
It also noted one follow-up outside this diff: the shared Renovate config (hashintel/actions) excludes @effect/vitest from the effect group. Now that @effect/vitest@4 requires effect ^4.0.3, they need to update together, so you could remove the !/@effect/vitest/ exclusion.
|
|
||
| MutableHashMap.remove(connection.transactions, id); | ||
| yield* transaction.value.queue.shutdown; | ||
| yield* Queue.end(transaction.value.queue); |
There was a problem hiding this comment.
When a transaction lags and gets dropped, the reader now gets a cut-off response with no error. The lag timeout (the catchTag("TimeoutError") above) calls drop, which ends the queue normally. Response.decode then treats a body with no end-of-response packet as complete. Before this PR the single-packet read (see the Transaction.ts comment) hid this. Now it can actually happen.
Suggestion: since v4 queues carry a typed error, fail the queue on the lag path (e.g. Queue.fail(queue, new TransactionLaggedError(...))) and keep Queue.end for a normal end of response. If it's out of scope here, a follow-up ticket works too.
| // cast needed as fast-check doesn't support Uint8Array<ArrayBuffer> yet | ||
| .map((array) => array as Uint8Array<ArrayBuffer>) | ||
| .map(makeUnchecked); | ||
| export const arbitrary = Arbitrary.array( |
There was a problem hiding this comment.
These test-data generators now use effect/Arbitrary, which is marked @stability unstable. They're also built when the module is imported: the docs say derivation "is immediate and throws" when it can't compile. These modules are on hash-api's import path, so an effect minor release could break hash-api at startup. Before, they were lazy (fc) => … functions. Could we make them thunks again, or move them into tests/? The same applies to every arbitrary in types/ and wire-protocol/models/.
There was a problem hiding this comment.
Why? This is the exact same thing that already exists in e.g. zod, Effect.Schema, etc. We pin our dependencies, so minor changes will not silently take effect, and the arbitrary derivation - much like w/ schemas are core to the actual types defined. (also note that Arbitrary itself has been stable for quite a while, so I don't see much of a risk)


🌟 Purpose
Upgrade to stable Effect 4.0.0. Straightforward move, allows us to have less dependencies, use the new maintained version that has LTS built-in, required to make #10013 work.
🔍 Changes
Resultinstead ofEither. Callers useEffect.fromResultwhen composing those results into effects.Decoder.decodeandEncoder.encodeacceptSchema.Codecwith separate decoding and encoding service requirements. Schema failures useSchema.SchemaError, and JSON codecs useSchema.fromJsonString.Transaction.readconsumes responses throughStream.fromQueue, and dropping a transaction ends its queue withQueue.end.