-
Notifications
You must be signed in to change notification settings - Fork 24
Define Variable/Value operations that will be used before evaluation #723
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
Define Variable/Value operations that will be used before evaluation #723
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## signing-epic #723 +/- ##
===============================================
Coverage ? 89.19%
===============================================
Files ? 95
Lines ? 8042
Branches ? 505
===============================================
Hits ? 7173
Misses ? 829
Partials ? 40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a1852c0 to
dd15906
Compare
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.
Pull Request Overview
This PR implements variable/value operations that can be performed before evaluation in the TACO web package, following the work done in the referenced nucypher PR. The implementation adds support for mathematical and type conversion operations that can be applied to condition variables and return values.
Key changes:
- Added a comprehensive set of operation functions including arithmetic, aggregation, and type conversion operations
- Implemented schema validation for variable operations with proper value requirement handling
- Integrated operations into sequential conditions and return value tests
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/taco/src/conditions/schemas/variable-operation.ts | Core implementation defining operation types and validation schema |
| packages/taco/src/conditions/schemas/sequential.ts | Added operations field to condition variables |
| packages/taco/src/conditions/schemas/return-value-test.ts | Added operations field to return value tests |
| packages/taco/test/conditions/variable-operation.test.ts | Comprehensive test suite for operation validation |
| packages/taco/test/conditions/sequential.test.ts | Added tests for operations in sequential conditions |
| packages/taco/test/conditions/return-value-test.test.ts | Added tests for operations in return value tests |
| packages/taco/integration-test/encrypt-decrypt.test.ts | Integration test demonstrating operations usage |
| packages/taco/integration-test/condition-lingo.test.ts | Extended integration tests with operations and increased timeout |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
dd15906 to
2d3b3e9
Compare
…variable (within SequentialCondition) and return value test.
…st and condition variable (sequential condition).
…able to schema-docs via script.
2d3b3e9 to
6d1f44e
Compare
… on a variable/result. Update tests and schema docs accordingly.
…e to possible expensive nature of operation and possible ddos vector.
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.
LGTM!
🚀
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.
LGTM! I left a question though
|
|
||
| | Property | Type | | ||
| | :------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | ||
| | **`operation`** (\*) | `'+=' \| '-=' \| '*=' \| '/=' \| '%=' \| '^=' \| 'index' \| 'round' \| 'abs' \| 'avg' \| 'ceil' \| 'ethToWei' \| 'floor' \| 'len' \| 'max' \| 'min' \| 'sum' \| 'weiToEth' \| 'bool' \| 'float' \| ...` | |
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.
in nucypher#3658 we also have 'int' and 'str'. Should we add them here?
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.
or is this automatically generated? 🧐
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.
That is produced by a script and seems like it can only provide a certain number of operations in the list and it is getting truncated to '...'.
cc @Muhammad-Altabba - we can address that truncation separately.
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.
got it. Thanks
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.
Filed #727 (cc @Muhammad-Altabba)
Type of PR:
Required reviews:
What this does:
taco-webimplementation of work done in nucypher/nucypher#3658.Issues fixed/closed:
Why it's needed:
Notes for reviewers: