Sending messages from general agent #581
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces multiple changes across several files in the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (11)
prediction_market_agent/agents/microchain_agent/messages_functions.py (4)
27-27: Remove unnecessary f-string in return statementThe return statement does not contain any placeholders, so the
fprefix is unnecessary.Apply this diff to fix the issue:
-return f"Message broadcasted to humans." +return "Message broadcasted to humans."🧰 Tools
🪛 Ruff (0.8.0)
27-27: f-string without any placeholders
Remove extraneous
fprefix(F541)
26-26: Use logger instead of print statementUsing
Apply this diff to replace
logger.info:+from prediction_market_agent_tooling.loggers import logger def __call__(self, message: str) -> str: # TODO: Implement as needed in https://github.com/gnosis/prediction-market-agent/issues/570. - print(message) + logger.info(message) return "Message broadcasted to humans."
33-34: Use gender-neutral language and correct article in descriptionIn the
descriptionproperty, consider correcting "send a message to an another agent, given his wallet address" to "send a message to another agent, given their wallet address" for grammatical correctness and inclusivity.Apply this diff to fix the issue:
-return f"""Use {SendPaidMessageToAnotherAgent.__name__} to send a message to an another agent, given his wallet address. +return f"""Use {SendPaidMessageToAnotherAgent.__name__} to send a message to another agent, given their wallet address. Fee for sending the message is {TRANSACTION_MESSAGE_FEE} xDai."""
54-56: Offer assistance to complete the implementation ofReceiveMessageThe
ReceiveMessageclass contains TODO comments indicating that the number of unseen messages should be added to the description, and the logic is incomplete in the__call__method.Would you like assistance in implementing the required functionality or opening a new GitHub issue to track this task?
tests/agents/microchain/test_utils.py (1)
7-10: Enhance test coverage for message compressionCurrently, the test only verifies a simple message. Consider adding test cases for edge cases, such as:
- Empty strings
- Very long messages
- Messages containing special characters or Unicode characters
This will ensure that the compression and decompression functions handle all scenarios correctly.
prediction_market_agent/agents/microchain_agent/microchain_agent_keys.py (2)
18-18: Fix typo in log messageThere is a typo in the word "Caping"; it should be "Capping".
Apply this diff to fix the typo:
-logger.warning(f"Caping sending xDai value to {amount}.") +logger.warning(f"Capping sending xDai value to {amount}.")
12-12: Consider using precise types for monetary valuesThe attribute
SENDING_XDAI_CAPis defined asfloat | None. Using floats for currency amounts may lead to precision issues. Consider using a fixed-precision decimal type or thexDaitype for accurate monetary calculations.Apply this diff to update the type annotation:
- SENDING_XDAI_CAP: float | None = OMEN_TINY_BET_AMOUNT + SENDING_XDAI_CAP: xDai | None = xDai(OMEN_TINY_BET_AMOUNT)prediction_market_agent/agents/microchain_agent/sending_functions.py (1)
Line range hint
13-13: Fix class name reference in descriptionThe
descriptionproperty usesSendXDAI.__class__, which returns<class 'type'>instead of the class name. To correctly reference the class name, useSendXDAI.__name__.Apply this diff to fix the issue:
-return f"Use {SendXDAI.__class__} to send xDai (stable coin, where 1 xDai equal $1) to a specified blockchain address on Gnosis Chain." +return f"Use {SendXDAI.__name__} to send xDai (stable coin, where 1 xDai equals $1) to a specified blockchain address on Gnosis Chain."prediction_market_agent/agents/social_media_agent/social_media/twitter_handler.py (2)
60-65: Consider adding error handling for quote_tweet_idThe method correctly handles the optional reasoning_reply_tweet parameter, but there's no validation of the quote_tweet_id before using it in the nested call.
Consider adding validation:
def post(self, text: str, reasoning_reply_tweet: str | None = None) -> None: quote_tweet_id = self.post_else_retry_with_summarization(text) - if reasoning_reply_tweet is not None: + if reasoning_reply_tweet is not None and quote_tweet_id is not None: self.post_else_retry_with_summarization( reasoning_reply_tweet, quote_tweet_id=quote_tweet_id )
Line range hint
85-96: Enhance error handling in post_tweet methodThe method has good basic error handling but could be improved.
Consider these enhancements:
def post_tweet(self, text: str, quote_tweet_id: str | None = None) -> str | None: """ Posts the provided text on Twitter. """ + if not text: + logger.warning("Attempted to post empty tweet") + return None + posted_tweet = self.client.create_tweet( text=text, quote_tweet_id=quote_tweet_id ) logger.info(f"Tweeted {text} - {posted_tweet}") try: return str(posted_tweet.data["id"]) - except Exception as e: - logger.exception("Could not extract tweet ID. Exception: ", e) + except (KeyError, AttributeError) as e: + logger.exception(f"Could not extract tweet ID from response {posted_tweet}. Error: {e}") return Noneprediction_market_agent/agents/microchain_agent/utils.py (1)
2-2: Consider documenting compression ratio expectationsWhile the compression is correctly implemented for gas optimization, it would be helpful to document expected compression ratios for typical messages to help with gas estimation.
Consider enhancing the docstring:
def compress_message(message: str) -> bytes: - """Used to reduce size of the message before sending it to reduce gas costs.""" + """Compresses a message to reduce gas costs for blockchain transmission. + + For typical English text messages, achieves ~60-70% size reduction using + BEST_COMPRESSION level. Empty or very short messages may result in larger output + due to compression overhead. + + Args: + message: The string message to compress + Returns: + Compressed message as bytes + """Also applies to: 146-148
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lock,!**/*.lockpyproject.tomlis excluded by!**/*.toml
📒 Files selected for processing (8)
prediction_market_agent/agents/microchain_agent/messages_functions.py(2 hunks)prediction_market_agent/agents/microchain_agent/microchain_agent_keys.py(1 hunks)prediction_market_agent/agents/microchain_agent/prompts.py(0 hunks)prediction_market_agent/agents/microchain_agent/sending_functions.py(2 hunks)prediction_market_agent/agents/microchain_agent/twitter_functions.py(1 hunks)prediction_market_agent/agents/microchain_agent/utils.py(2 hunks)prediction_market_agent/agents/social_media_agent/social_media/twitter_handler.py(4 hunks)tests/agents/microchain/test_utils.py(1 hunks)
💤 Files with no reviewable changes (1)
- prediction_market_agent/agents/microchain_agent/prompts.py
🧰 Additional context used
🪛 Ruff (0.8.0)
prediction_market_agent/agents/microchain_agent/messages_functions.py
27-27: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (2)
prediction_market_agent/agents/microchain_agent/twitter_functions.py (1)
22-31: LGTM
The updates to the SendTweet class enhance error handling for tweet length and conditionally post based on social media settings. The changes are correct and improve functionality.
prediction_market_agent/agents/social_media_agent/social_media/twitter_handler.py (1)
16-16: LGTM: Clean dependency update
The switch to SocialMediaAPIKeys and corresponding key usage is implemented correctly.
Also applies to: 39-39
| def __call__(self, address: str, message: str) -> str: | ||
| keys = MicrochainAgentKeys() | ||
| send_xdai_to( | ||
| web3=ContractOnGnosisChain.get_web3(), | ||
| from_private_key=keys.bet_from_private_key, | ||
| to_address=Web3.to_checksum_address(address), | ||
| value=xdai_to_wei(keys.cap_sending_xdai(TRANSACTION_MESSAGE_FEE)), | ||
| data_text=compress_message(message), | ||
| ) | ||
| return "Message sent to the agent." |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for transaction failures
The __call__ method does not handle exceptions that may occur during the send_xdai_to function call. To enhance robustness, add error handling to manage potential errors such as transaction failures, invalid addresses, or insufficient funds.
Apply this diff to add error handling:
+from prediction_market_agent_tooling.loggers import logger
def __call__(self, address: str, message: str) -> str:
keys = MicrochainAgentKeys()
try:
send_xdai_to(
web3=ContractOnGnosisChain.get_web3(),
from_private_key=keys.bet_from_private_key,
to_address=Web3.to_checksum_address(address),
value=xdai_to_wei(keys.cap_sending_xdai(TRANSACTION_MESSAGE_FEE)),
data_text=compress_message(message),
)
return "Message sent to the agent."
+ except Exception as e:
+ logger.error(f"Failed to send message: {e}")
+ return "Failed to send message due to an error."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __call__(self, address: str, message: str) -> str: | |
| keys = MicrochainAgentKeys() | |
| send_xdai_to( | |
| web3=ContractOnGnosisChain.get_web3(), | |
| from_private_key=keys.bet_from_private_key, | |
| to_address=Web3.to_checksum_address(address), | |
| value=xdai_to_wei(keys.cap_sending_xdai(TRANSACTION_MESSAGE_FEE)), | |
| data_text=compress_message(message), | |
| ) | |
| return "Message sent to the agent." | |
| def __call__(self, address: str, message: str) -> str: | |
| keys = MicrochainAgentKeys() | |
| try: | |
| send_xdai_to( | |
| web3=ContractOnGnosisChain.get_web3(), | |
| from_private_key=keys.bet_from_private_key, | |
| to_address=Web3.to_checksum_address(address), | |
| value=xdai_to_wei(keys.cap_sending_xdai(TRANSACTION_MESSAGE_FEE)), | |
| data_text=compress_message(message), | |
| ) | |
| return "Message sent to the agent." | |
| except Exception as e: | |
| logger.error(f"Failed to send message: {e}") | |
| return "Failed to send message due to an error." |
| keys = MicrochainAgentKeys() | ||
| web3 = ContractOnGnosisChain.get_web3() | ||
| address_checksum = Web3.to_checksum_address(address) | ||
| send_xdai_to( | ||
| web3, | ||
| keys.bet_from_private_key, | ||
| address_checksum, | ||
| xdai_to_wei(xdai_type(amount)), | ||
| xdai_to_wei(keys.cap_sending_xdai(xdai_type(amount))), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for transaction failures
The __call__ method lacks error handling for exceptions that may occur during the send_xdai_to function call. Incorporate error handling to manage issues such as invalid addresses, network errors, or insufficient funds gracefully.
Apply this diff to add error handling:
+from prediction_market_agent_tooling.loggers import logger
def __call__(
self,
address: str,
amount: float,
) -> str:
keys = MicrochainAgentKeys()
web3 = ContractOnGnosisChain.get_web3()
address_checksum = Web3.to_checksum_address(address)
+ try:
send_xdai_to(
web3,
keys.bet_from_private_key,
address_checksum,
xdai_to_wei(keys.cap_sending_xdai(xdai_type(amount))),
)
- return f"Sent {amount} xDAI to {address_checksum}."
+ return f"Sent {amount} xDai to {address_checksum}."
+ except Exception as e:
+ logger.error(f"Failed to send xDai: {e}")
+ return "Failed to send xDai due to an error."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| keys = MicrochainAgentKeys() | |
| web3 = ContractOnGnosisChain.get_web3() | |
| address_checksum = Web3.to_checksum_address(address) | |
| send_xdai_to( | |
| web3, | |
| keys.bet_from_private_key, | |
| address_checksum, | |
| xdai_to_wei(xdai_type(amount)), | |
| xdai_to_wei(keys.cap_sending_xdai(xdai_type(amount))), | |
| from prediction_market_agent_tooling.loggers import logger | |
| keys = MicrochainAgentKeys() | |
| web3 = ContractOnGnosisChain.get_web3() | |
| address_checksum = Web3.to_checksum_address(address) | |
| try: | |
| send_xdai_to( | |
| web3, | |
| keys.bet_from_private_key, | |
| address_checksum, | |
| xdai_to_wei(keys.cap_sending_xdai(xdai_type(amount))), | |
| ) | |
| return f"Sent {amount} xDai to {address_checksum}." | |
| except Exception as e: | |
| logger.error(f"Failed to send xDai: {e}") | |
| return "Failed to send xDai due to an error." |
| def decompress_message(message: bytes) -> str: | ||
| return zlib.decompress(message).decode() |
There was a problem hiding this comment.
Add error handling for malformed compressed data
The decompression function should handle potential errors from malformed input.
Add error handling:
def decompress_message(message: bytes) -> str:
- return zlib.decompress(message).decode()
+ """Decompresses a message previously compressed with compress_message.
+
+ Args:
+ message: The compressed message bytes
+ Returns:
+ Original string message
+ Raises:
+ ValueError: If the message is corrupted or not valid compressed data
+ """
+ try:
+ return zlib.decompress(message).decode()
+ except (zlib.error, UnicodeDecodeError) as e:
+ raise ValueError(f"Failed to decompress message: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def decompress_message(message: bytes) -> str: | |
| return zlib.decompress(message).decode() | |
| def decompress_message(message: bytes) -> str: | |
| """Decompresses a message previously compressed with compress_message. | |
| Args: | |
| message: The compressed message bytes | |
| Returns: | |
| Original string message | |
| Raises: | |
| ValueError: If the message is corrupted or not valid compressed data | |
| """ | |
| try: | |
| return zlib.decompress(message).decode() | |
| except (zlib.error, UnicodeDecodeError) as e: | |
| raise ValueError(f"Failed to decompress message: {e}") |
|
|
||
|
|
||
| def compress_message(message: str) -> bytes: | ||
| """Used to reduce size of the message before sending it to reduce gas costs.""" | ||
| return zlib.compress(message.encode(), level=zlib.Z_BEST_COMPRESSION) | ||
|
|
||
|
|
||
| def decompress_message(message: bytes) -> str: | ||
| return zlib.decompress(message).decode() |
There was a problem hiding this comment.
💡 Codebase verification
Compression functions are properly integrated but message decompression is missing in handlers
- The
compress_messagefunction is correctly used inSendPaidMessageToAnotherAgentto compress messages before sending them on-chain - However,
decompress_messageis not implemented in any message receiving logic - The
ReceiveMessagefunction is currently a TODO stub and needs to implement message decompression - The compression utilities are properly tested in isolation
🔗 Analysis chain
Verify compression usage in message handlers
The compression utilities are well implemented, but we should verify their integration with message handlers.
Let's examine the actual usage of these compression functions in the message handlers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of compression functions in message handlers
echo "Searching for compression function usage..."
rg -l "compress_message|decompress_message" --type py
# Look for potential message size constants or limits
echo "Checking for message size limits..."
rg -i "message.*size|size.*limit|max.*size" --type py
Length of output: 609
Script:
#!/bin/bash
# Check the implementation in messages_functions.py
echo "Content of messages_functions.py:"
cat prediction_market_agent/agents/microchain_agent/messages_functions.py
# Check the test implementation
echo -e "\nContent of test_utils.py:"
cat tests/agents/microchain/test_utils.py
Length of output: 3109
No description provided.