ACP Developer Experience Improvements #34
Conversation
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
celesteanglm
requested changes
Jun 27, 2025
| @@ -11,8 +11,14 @@ def test_helper_functions(): | |||
| acp = VirtualsACP( | |||
| wallet_private_key=env.WHITELISTED_WALLET_PRIVATE_KEY, | |||
| agent_wallet_address=env.BUYER_AGENT_WALLET_ADDRESS, | |||
| config=BASE_SEPOLIA_CONFIG | |||
| config=BASE_MAINNET_CONFIG, | |||
There was a problem hiding this comment.
could you remove the config param? we removed these on purpose because the default behaviour would be mainnet-only moving forward (except for exceptional scenarios)
| @@ -47,8 +48,11 @@ def on_evaluate(job: ACPJob): | |||
| agent_wallet_address=env.BUYER_AGENT_WALLET_ADDRESS, | |||
| on_new_task=on_new_task, | |||
| on_evaluate=on_evaluate, | |||
| entity_id=env.BUYER_ENTITY_ID | |||
| entity_id=env.BUYER_ENTITY_ID, | |||
| config=BASE_MAINNET_CONFIG | |||
There was a problem hiding this comment.
please remove the config param
| @@ -40,8 +41,11 @@ def on_new_task(job: ACPJob): | |||
| wallet_private_key=env.WHITELISTED_WALLET_PRIVATE_KEY, | |||
| agent_wallet_address=env.SELLER_AGENT_WALLET_ADDRESS, | |||
| on_new_task=on_new_task, | |||
| entity_id=env.SELLER_ENTITY_ID | |||
| entity_id=env.SELLER_ENTITY_ID, | |||
| config=BASE_MAINNET_CONFIG | |||
There was a problem hiding this comment.
please remove the config param
| ) | ||
|
|
||
| print(f"ACP client: {acp_client.agent_address}") |
There was a problem hiding this comment.
please remove this print statement
| @@ -14,8 +14,33 @@ | |||
| ) | |||
| from .exceptions import ( | |||
There was a problem hiding this comment.
I tihnk these are a bit overkill especially if not use - would leave it to individual devs to customise their own error messages so we left it quite generic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR improves the ACP Python SDK with better error handling, developer docs, and test reliability.
🔧 Improvements
1. Fixed Legacy Imports & Restored Tests
acp_sdkwithvirtuals_acpin examplesVirtualsACP.__init__()errors and restoredtest_helper_functions2. Improved Configuration Validation
ConfigDictfor clearer Pydantic config errors3. Docstrings Added
Note
Added full Google-style docstrings to all
VirtualsACPmethods for IDE support and easier onboarding (more required for other files for better devving experience❗)4. Exception Hierarchy Refactor
5. Fixed Broken SDK Docs Link
examples/acp_base/README.mdImpact for devs
Suggestions (Optional Follow-Up Work)
Code Organization
/buyers,/sellers,/evaluators, and/hybriduse casesDocumentation Enhancement
contract_manager.py,memo.py, and other core modules similar to theclient.py