More NFT game mods #681
More NFT game mods #681
Conversation
WalkthroughThis pull request introduces a new enumeration class, Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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: 0
🧹 Nitpick comments (3)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (3)
7-10: Consider grouping related imports.The contract-related imports could be grouped with other contract imports or organized in a more logical manner.
from prediction_market_agent_tooling.tools.contract import ( ContractOwnableERC721OnGnosisChain, SimpleTreasuryContract, ) + +# Other contract-related imports +from web3 import Web3
106-109: Consider adding docstring and error handling.The method could benefit from documentation and handling potential contract interaction failures.
def get_holding_n_nft_keys(self) -> int: + """ + Returns the number of NFT keys held by this agent. + + Returns: + int: Number of NFT keys owned by the agent + Raises: + ContractError: If the contract interaction fails + """ contract = ContractOwnableERC721OnGnosisChain(address=NFT_TOKEN_FACTORY) - return contract.balanceOf(Web3.to_checksum_address(self.wallet_address)) + try: + return contract.balanceOf(Web3.to_checksum_address(self.wallet_address)) + except Exception as e: + logger.error(f"Failed to get NFT balance: {e}") + raise
115-117: Consider extracting condition to a descriptive method.The condition check could be more readable if extracted to a method with a descriptive name.
+def _is_seller_without_keys(self) -> bool: + """Returns True if this agent is a seller who has no NFT keys.""" + return self.role == Role.seller and not self.get_holding_n_nft_keys() + -is_seller_without_keys = ( - self.role == Role.seller and not self.get_holding_n_nft_keys() -) +is_seller_without_keys = self._is_seller_without_keys()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py(19 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test-build-image
- GitHub Check: pytest
- GitHub Check: pytest-docker
- GitHub Check: mypy
- GitHub Check: build-and-push-image
🔇 Additional comments (5)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (5)
44-46: LGTM! Well-defined enum class.The Role enum is well-structured and follows Python enum best practices.
66-66: LGTM! Well-typed role attribute.The role attribute is properly typed with the Role enum.
193-193: LGTM! Consistent role assignments.The role assignments are well-distributed with 5 sellers and 2 buyers, which creates an interesting market dynamic.
Also applies to: 214-214, 234-234, 257-257, 277-277, 298-298, 320-320
241-244: Consider ethical implications of agent behavior.The agent is programmed to trick and deceive other agents, which might lead to undesirable behavior in the game.
Please ensure this behavior aligns with the game's intended design and ethical guidelines.
383-409: LGTM! Well-structured role-specific prompts.The buyer and seller prompts are comprehensive and provide clear strategies aligned with their roles.
Also applies to: 412-422
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (3)
66-66: Consider adding role validation in the abstract class.While the role integration looks good, consider adding validation to ensure the role is set for all agent instances.
class DeployableAgentNFTGameAbstract(DeployableMicrochainAgentAbstract): # Setup per-nft-agent class. name: str wallet_address: ChecksumAddress role: Role + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if not hasattr(self, 'role'): + raise ValueError(f"Role must be set for {self.__class__.__name__}")Also applies to: 98-104
106-108: Consider caching NFT key count.The
get_holding_n_nft_keysmethod makes a contract call which could be expensive if called frequently.+ _nft_keys_cache = {} + _nft_keys_cache_time = {} + CACHE_TTL = 60 # seconds + def get_holding_n_nft_keys(self) -> int: + current_time = time.time() + cache_key = str(self.wallet_address) + + if (cache_key in self._nft_keys_cache and + current_time - self._nft_keys_cache_time.get(cache_key, 0) < self.CACHE_TTL): + return self._nft_keys_cache[cache_key] + contract = ContractOwnableERC721OnGnosisChain(address=NFT_TOKEN_FACTORY) - return contract.balanceOf(Web3.to_checksum_address(self.wallet_address)) + result = contract.balanceOf(Web3.to_checksum_address(self.wallet_address)) + + self._nft_keys_cache[cache_key] = result + self._nft_keys_cache_time[cache_key] = current_time + return result
386-412: Consider adding price negotiation strategies in buyer prompt.The buyer prompt could benefit from more specific strategies for price negotiation, especially given the competitive nature of the game.
[KEY ACQUISITION STRATEGY] - Monitor the treasury's current xDai balance closely - Track how many keys you already own - Calculate maximum acceptable price per key: * Treasury Value ÷ 3 = Maximum Total Budget * Adjust individual key prices based on how many you already own * Earlier keys can be cheaper since you'll need all 3 to profit + +[NEGOTIATION TACTICS] +- Start with lower offers to gauge seller's expectations +- Consider bulk purchase offers for multiple keys +- Monitor other buyers' activities to adjust negotiation strategy +- Use time pressure to your advantage when sellers are competing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py(20 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test-build-image
- GitHub Check: pytest-docker
- GitHub Check: pytest
- GitHub Check: mypy
- GitHub Check: build-and-push-image
🔇 Additional comments (3)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (3)
44-46: LGTM! Well-structured enum for agent roles.The
Roleenum provides a clean and type-safe way to define agent roles.
115-117: Verify the impact of seller without keys condition.The new condition
is_seller_without_keysis used in multiple places. Let's verify its usage across the codebase.Also applies to: 124-124, 134-134, 165-167
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other potential places where seller's key ownership might be relevant rg -l "seller.*key" --type py rg -l "get_holding_n_nft_keys" --type pyLength of output: 260
Seller Without Keys Condition Usage Verified
I verified that the only occurrences of the seller key-related functionality (including get_holding_n_nft_keys and the is_seller_without_keys condition) are confined to the deploy_nft_treasury_game.py file – specifically at lines 115, 124, 134, and 165–167. There are no cross-file dependencies impacted by this change.
196-196: Verify role distribution among agents.The role distribution shows 5 sellers (agents 1-5) and 2 buyers (agents 6-7). This seems intentional based on the game design where buyers need to acquire keys from sellers.
Let's verify if this distribution aligns with the treasury threshold:
Also applies to: 217-217, 237-237, 260-260, 280-280, 301-301, 323-323
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for treasury threshold configuration rg "retrieve_treasury_thresold|required_nft_balance" --type pyLength of output: 800
I'll now run a shell script to search for occurrences of Role.buyer to verify the role distribution in the codebase.
🏁 Script executed:
#!/bin/bash rg "Role\.buyer" --type pyLength of output: 389
Below is the final verification:
- We confirmed that the code designates sellers via assignments like "role = Role.seller" (e.g., line 196) and that Role.buyer is assigned in two locations.
- The treasury configuration uses SimpleTreasuryContract().required_nft_balance() (via retrieve_treasury_thresold) to set the key threshold, and the in-code messaging (“Your goal is to acquire … out of … NFT keys”) confirms that the intended distribution of 5 sellers (agents 1–5) and 2 buyers (agents 6–7) aligns with the treasury threshold logic.
There is no discrepancy between the role assignments and the treasury threshold requirement.
Actionable directive:
No changes are required based on the role distribution verification. The current design of 5 sellers and 2 buyers appears intentional and properly integrated with the treasury threshold configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (1)
110-187: Consider refactoring complex callback logic.The
before_iteration_callbackmethod has complex nested conditions that could be simplified for better maintainability.Consider extracting the game status checks and reflection logic into separate methods:
+ def _should_sleep_and_stop(self, is_seller_without_keys: bool) -> bool: + return ( + is_seller_without_keys + or get_nft_game_status() == NFTGameStatus.finished + ) + + def _handle_game_reflection(self, system_prompt: str) -> None: + if self.agent.llm.generator.model == SupportedModel.gpt_4o_mini.value: + self.agent.llm.generator.model = SupportedModel.gpt_4o.value + self._update_history_for_reflection(system_prompt) + self.game_finished_already_detected = True + def before_iteration_callback(self) -> CallbackReturn: system_prompt = self.agent.history[0] if self.agent.history else None is_seller_without_keys = ( self.role == Role.seller and not self.get_holding_n_nft_keys() ) - if len(self.agent.history) <= 1 and (...): + if len(self.agent.history) <= 1 and self._should_sleep_and_stop(is_seller_without_keys): logger.info( "The game is not ready yet and agent didn't have any previous interactions, sleeping and stopping." ) time.sleep(60) return CallbackReturn.STOP elif self.agent.history and GameRoundEnd.GAME_ROUND_END_OUTPUT in str( self.agent.history[-1] ): - if (is_seller_without_keys or get_nft_game_status() == NFTGameStatus.finished): + if self._should_sleep_and_stop(is_seller_without_keys): logger.info("Agent is done with the game, sleeping and stopping.") time.sleep(60) return CallbackReturn.STOP else: self._update_history_for_new_game(system_prompt) elif not self.game_finished_already_detected and self._should_sleep_and_stop(is_seller_without_keys): logger.info("Game is finished, forcing agent to reflect on the past game.") - if self.agent.llm.generator.model == SupportedModel.gpt_4o_mini.value: - self.agent.llm.generator.model = SupportedModel.gpt_4o.value - self.agent.history = [...] - self.save_agent_history(check_not_none(system_prompt), 2) - self.game_finished_already_detected = True + self._handle_game_reflection(system_prompt) return CallbackReturn.CONTINUE
🧹 Nitpick comments (4)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (4)
7-10: Consider grouping related imports together.The contract-related imports could be moved closer to other related imports for better organization.
-from prediction_market_agent_tooling.tools.contract import ( - ContractOwnableERC721OnGnosisChain, - SimpleTreasuryContract, -) +# Group with other contract-related imports +from prediction_market_agent_tooling.tools.contract import ( + ContractOwnableERC721OnGnosisChain, + SimpleTreasuryContract, +)
106-109: Consider caching NFT key count.The
get_holding_n_nft_keysmethod makes a contract call which could be expensive if called frequently. Consider caching the result and implementing an update mechanism.+ _nft_keys_cache: dict[str, int] = {} + _nft_keys_cache_timestamp: dict[str, float] = {} + _CACHE_TTL = 60 # seconds + def get_holding_n_nft_keys(self) -> int: + current_time = time.time() + cache_key = str(self.wallet_address) + + if ( + cache_key in self._nft_keys_cache + and current_time - self._nft_keys_cache_timestamp[cache_key] < self._CACHE_TTL + ): + return self._nft_keys_cache[cache_key] + contract = ContractOwnableERC721OnGnosisChain(address=NFT_TOKEN_FACTORY) - return contract.balanceOf(Web3.to_checksum_address(self.wallet_address)) + balance = contract.balanceOf(Web3.to_checksum_address(self.wallet_address)) + + self._nft_keys_cache[cache_key] = balance + self._nft_keys_cache_timestamp[cache_key] = current_time + return balance
190-337: Consider making role assignments more explicit and DRY.The role assignments could be made more maintainable by using a class decorator or metaclass.
+from functools import wraps +from typing import Type + +def assign_role(role: Role): + def decorator(cls: Type[DeployableAgentNFTGameAbstract]) -> Type[DeployableAgentNFTGameAbstract]: + cls.role = role + return cls + return decorator + +@assign_role(Role.seller) class DeployableAgentNFTGame1(DeployableAgentNFTGameAbstract): name = "Sneaky Pete" identifier = AgentIdentifier.NFT_TREASURY_GAME_AGENT_1 wallet_address = Web3.to_checksum_address( "0x2A537F3403a3F5F463996c36D31e94227c9833CE" ) - role = Role.sellerAlso consider extracting common personality traits into reusable mixins:
class ProtectiveResourcesMixin: @classmethod def get_protective_prompt(cls) -> str: return "and you are very protective of your resources.\n" class DeployableAgentNFTGame2(DeployableAgentNFTGameAbstract, ProtectiveResourcesMixin): # ... existing code ... def get_initial_system_prompt(cls) -> str: return ( f"""Your name is {cls.name}.\n\n""" "You respond in the style of characters from historical Bridgeton movie " + cls.get_protective_prompt() + super().get_initial_system_prompt() )
386-413: Consider extracting magic numbers and calculations into constants and helper methods.The buyer prompt contains magic numbers and calculations that could be made more maintainable.
+class TreasuryConstants: + MAX_BUDGET_DIVISOR = 3 # Treasury Value ÷ 3 = Maximum Total Budget + +class TreasuryCalculator: + @staticmethod + def calculate_max_budget(treasury_value: float) -> float: + return treasury_value / TreasuryConstants.MAX_BUDGET_DIVISOR + + @staticmethod + def is_profitable_purchase(total_spent: float, treasury_value: float) -> bool: + return total_spent < treasury_value def nft_treasury_game_buyer_prompt() -> str: return f"""You participate in the NFT Treasury game as a key buyer. # ... existing code ... - * Treasury Value ÷ 3 = Maximum Total Budget + * Treasury Value ÷ {TreasuryConstants.MAX_BUDGET_DIVISOR} = Maximum Total Budget # ... rest of the code ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py(18 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test-build-image
- GitHub Check: pytest-docker
- GitHub Check: pytest
- GitHub Check: mypy
- GitHub Check: build-and-push-image
🔇 Additional comments (1)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (1)
428-436: LGTM!The deployment list is well-organized and serves its purpose effectively.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
prediction_market_agent/run_reset_game.py (3)
29-34: Document the new parameters in the docstring.The main function has been updated with new parameters, but their purpose and impact are not documented. Add a docstring to explain each parameter's role.
@APP.command() def main( rpc_url: str, check_game_finished: bool = True, sleep: bool = True, generate_report: bool = True, ) -> None: + """Reset the NFT game configuration. + + Args: + rpc_url: The RPC URL for the Anvil instance + check_game_finished: If True, verify game is in finished state before resetting + sleep: If True, wait 10 minutes before resetting to allow reflection + generate_report: If True, generate a game report before resetting + """🧰 Tools
🪛 Ruff (0.8.2)
33-33: Redefinition of unused
generate_reportfrom line 17(F811)
36-36: Remove unnecessary f-string prefix.The string doesn't contain any placeholders, so the f-string prefix is not needed.
- logger.info(f"Treasury not empty, exiting.") + logger.info("Treasury not empty, exiting.")🧰 Tools
🪛 Ruff (0.8.2)
36-36: f-string without any placeholders
Remove extraneous
fprefix(F541)
33-33: Resolve the redefinition ofgenerate_report.The parameter name
generate_reportshadows the imported function of the same name. Consider renaming the parameter to avoid confusion.def main( rpc_url: str, check_game_finished: bool = True, sleep: bool = True, - generate_report: bool = True, + should_generate_report: bool = True, ) -> None: # ... - if generate_report: + if should_generate_report: generate_report( rpc_url=rpc_url, initial_xdai_balance_per_agent=STARTING_AGENT_BALANCE, )Also applies to: 43-47
🧰 Tools
🪛 Ruff (0.8.2)
33-33: Redefinition of unused
generate_reportfrom line 17(F811)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (3)
115-117: Consider extracting the role check to a descriptive method.The role check logic could be more readable if extracted to a method with a descriptive name.
+ def is_inactive_seller(self) -> bool: + """Check if the agent is a seller without any NFT keys.""" + return self.role == Role.seller and not self.get_holding_n_nft_keys() + def before_iteration_callback(self) -> CallbackReturn: - is_seller_without_keys = ( - self.role == Role.seller and not self.get_holding_n_nft_keys() - ) + is_seller_without_keys = self.is_inactive_seller()
425-427: Remove unnecessary f-string prefix in seller prompt.The string doesn't contain any placeholders, so the f-string prefix can be removed.
- return f"""You participate in the NFT Treasury game as a key seller. + return """You participate in the NFT Treasury game as a key seller.🧰 Tools
🪛 Ruff (0.8.2)
416-427: f-string without any placeholders
Remove extraneous
fprefix(F541)
190-196: Consider using a class decorator for role assignment.The role assignment is repeated across all agent classes. Consider using a class decorator to make the role assignment more maintainable.
def with_role(role: Role): def decorator(cls: type[DeployableAgentNFTGameAbstract]) -> type[DeployableAgentNFTGameAbstract]: cls.role = role return cls return decorator @with_role(Role.seller) class DeployableAgentNFTGame1(DeployableAgentNFTGameAbstract): # ... rest of the class implementation @with_role(Role.buyer) class DeployableAgentNFTGame6(DeployableAgentNFTGameAbstract): # ... rest of the class implementationAlso applies to: 211-217, 231-237, 254-260, 274-280, 295-301, 317-323
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/app_nft_treasury_game.py(1 hunks)prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py(19 hunks)prediction_market_agent/run_reset_game.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- prediction_market_agent/agents/microchain_agent/nft_treasury_game/app_nft_treasury_game.py
🧰 Additional context used
🪛 Ruff (0.8.2)
prediction_market_agent/run_reset_game.py
33-33: Redefinition of unused generate_report from line 17
(F811)
36-36: f-string without any placeholders
Remove extraneous f prefix
(F541)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py
416-427: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test-build-image
- GitHub Check: pytest-docker
- GitHub Check: pytest
- GitHub Check: mypy
- GitHub Check: build-and-push-image
🔇 Additional comments (1)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (1)
44-46: LGTM! Role enum is well-defined.The Role enum clearly defines the possible roles (buyer and seller) for NFT game agents.
| not self.game_finished_already_detected | ||
| and get_nft_game_status() == NFTGameStatus.finished | ||
| # If agent did the reflection (from the later if-clause), then... | ||
| elif self.agent.history and GameRoundEnd.GAME_ROUND_END_OUTPUT in str( |
There was a problem hiding this comment.
I added is_seller_without_keys to the conditions to stop the game, but otherwise it's the same thing as before, just reordered the clauses.
| return system_prompt | ||
|
|
||
| def get_holding_n_nft_keys(self) -> int: | ||
| contract = ContractOwnableERC721OnGnosisChain(address=NFT_TOKEN_FACTORY) |
There was a problem hiding this comment.
| contract = ContractOwnableERC721OnGnosisChain(address=NFT_TOKEN_FACTORY) | |
| contract = ContractERC721BaseClass(address=NFT_TOKEN_FACTORY) |
There was a problem hiding this comment.
Suggesting the above because no need for it to be ownable - just the ERC721 component has balanceOf functionality
There was a problem hiding this comment.
But it is ownable https://github.com/gnosis/labs-contracts/blob/dd748a506c9ee0cdc90778417b2743cc82e26aeb/src/NFT/AgentNFT.sol#L8. The idea for these classes is to be 1:1 with an underlying contract.
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py
Outdated
Show resolved
Hide resolved
…ame/deploy_nft_treasury_game.py Co-authored-by: Gabriel Fior <fior.gbm@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
prediction_market_agent/run_reset_game.py (5)
36-36: Remove unnecessary f-string prefix.The string doesn't contain any placeholders, so the
fprefix can be removed.- logger.info(f"Treasury not empty, exiting.") + logger.info("Treasury not empty, exiting.")🧰 Tools
🪛 Ruff (0.8.2)
36-36: f-string without any placeholders
Remove extraneous
fprefix(F541)
36-36: Remove unnecessary f-string prefix.The string doesn't contain any placeholders, so the
fprefix can be removed.- logger.info(f"Treasury not empty, exiting.") + logger.info("Treasury not empty, exiting.")🧰 Tools
🪛 Ruff (0.8.2)
36-36: f-string without any placeholders
Remove extraneous
fprefix(F541)
115-117: Consider caching the NFT key count.The
get_holding_n_nft_keys()call might be expensive as it involves a contract call. Consider caching the result since it's used multiple times in thebefore_iteration_callbackmethod.def before_iteration_callback(self) -> CallbackReturn: system_prompt = self.agent.history[0] if self.agent.history else None + holding_n_keys = self.get_holding_n_nft_keys() is_seller_without_keys = ( - self.role == Role.seller and not self.get_holding_n_nft_keys() + self.role == Role.seller and not holding_n_keys )
425-427: Remove unnecessary f-string prefix.These strings don't contain any placeholders, so the
fprefix can be removed.- f"""You participate in the NFT Treasury game as a key seller. + """You participate in the NFT Treasury game as a key seller.
36-36: Remove unnecessary f-string.The log message doesn't contain any placeholders.
- logger.info(f"Treasury not empty, exiting.") + logger.info("Treasury not empty, exiting.")🧰 Tools
🪛 Ruff (0.8.2)
36-36: f-string without any placeholders
Remove extraneous
fprefix(F541)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (4)
115-117: Consider caching the NFT key count.The
get_holding_n_nft_keys()call might be expensive as it involves a contract call. Consider caching the result if called frequently within the same iteration.+ _nft_keys_cache: tuple[int, float] | None = None + _nft_keys_cache_ttl = 60 # seconds + + def get_holding_n_nft_keys(self) -> int: + current_time = time.time() + if self._nft_keys_cache and current_time - self._nft_keys_cache[1] < self._nft_keys_cache_ttl: + return self._nft_keys_cache[0] + + contract = ContractOwnableERC721OnGnosisChain(address=NFT_TOKEN_FACTORY) + keys = contract.balanceOf(self.wallet_address) + self._nft_keys_cache = (keys, current_time) + return keys
425-427: Remove unnecessary f-string prefix in seller prompt.The string doesn't contain any placeholders, so the
fprefix can be removed.- return f"""You participate in the NFT Treasury game as a key seller. + return """You participate in the NFT Treasury game as a key seller.🧰 Tools
🪛 Ruff (0.8.2)
416-427: f-string without any placeholders
Remove extraneous
fprefix(F541)
386-412: Consider adding price negotiation guidance in buyer prompt.The buyer prompt could benefit from additional guidance on price negotiation strategies, especially for multi-key deals.
Add a section like:
[VALUE ASSESSMENT] - For each potential purchase, consider: * Current treasury balance * Number of keys you already own * Remaining keys available in the market * Time pressure from other buyers - Remember: Spending more than 1/3 of treasury value per key is risky + +[NEGOTIATION TACTICS] +- Consider bulk purchase offers for multiple keys +- Start negotiations below your maximum price +- Use time pressure to your advantage +- Track seller behaviors and adapt strategies
415-415: Remove unnecessary f-string.The seller prompt doesn't contain any placeholders.
-def nft_treasury_game_seller_prompt() -> str: +def nft_treasury_game_seller_prompt() -> str: - return f"""You participate in the NFT Treasury game as a key seller. + return """You participate in the NFT Treasury game as a key seller.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py(19 hunks)prediction_market_agent/run_reset_game.py(1 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- prediction_market_agent/run_reset_game.py
- prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py
🧰 Additional context used
🪛 Ruff (0.8.2)
prediction_market_agent/run_reset_game.py
36-36: f-string without any placeholders
Remove extraneous f prefix
(F541)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py
416-427: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: pytest-docker
- GitHub Check: test-build-image
- GitHub Check: pytest
- GitHub Check: mypy
- GitHub Check: build-and-push-image
🔇 Additional comments (19)
prediction_market_agent/run_reset_game.py (10)
29-34: LGTM! Good addition of configurable parameters.The new parameters enhance the flexibility of the script by making key behaviors configurable.
35-42: LGTM! Clean implementation of conditional logic.The conditional checks are well-structured and maintain the original behavior while adding configurability.
🧰 Tools
🪛 Ruff (0.8.2)
36-36: f-string without any placeholders
Remove extraneous
fprefix(F541)
43-47: LGTM! Report generation is now optional.Making report generation configurable is a good improvement for scenarios where it's not needed.
29-34: LGTM! The new parameters enhance script configurability.The addition of optional boolean parameters makes the script more flexible by allowing callers to:
- Skip game finish check with
check_game_finished- Skip sleep delay with
sleep- Skip report generation with
do_report
44-46: LGTM! Clear and concise Role enumeration.The
Roleenum effectively models the two possible roles (buyer and seller) in the NFT game.
98-104: LGTM! Well-structured role-based prompt generation.The
get_initial_system_promptmethod effectively combines the base prompt with role-specific prompts.
196-196: LGTM! Clear role assignments for each agent.The role assignments are well-distributed:
- Agents 1-5 are sellers
- Agents 6-7 are buyers
This creates an interesting market dynamic with more sellers than buyers.
Also applies to: 217-217, 237-237, 260-260, 280-280, 301-301, 323-323
29-34: LGTM! Well-structured function signature changes.The new boolean parameters with default values improve configurability while maintaining backward compatibility.
35-42: LGTM! Clean implementation of conditional checks.The conditional logic for game finish check and sleep delay is well-implemented.
🧰 Tools
🪛 Ruff (0.8.2)
36-36: f-string without any placeholders
Remove extraneous
fprefix(F541)
43-47: LGTM! Clean implementation of conditional report generation.The conditional logic for report generation is well-implemented.
prediction_market_agent/agents/microchain_agent/nft_treasury_game/deploy_nft_treasury_game.py (9)
44-46: LGTM! Clear and simple Role enumeration.The Role enum effectively models the buyer/seller dichotomy in the NFT game.
66-66: LGTM! Role attribute addition.The role attribute in the abstract class provides a clean way to distinguish between buyers and sellers.
98-104: LGTM! Well-structured role-based prompt handling.The get_initial_system_prompt method cleanly handles different prompts based on the agent's role.
196-196: LGTM! Clear role assignments for each agent.The role assignments are consistent with the game design:
- Agents 1-5 are sellers
- Agents 6-7 are buyers
Also applies to: 217-217, 237-237, 260-260, 280-280, 301-301, 323-323
2-2: LGTM! Well-structured Role enum and imports.The Role enum and imports are well-organized and necessary for the implementation.
Also applies to: 7-10, 44-47
66-66: LGTM! Well-implemented role-based behavior.The role attribute and supporting methods are well-designed and implemented.
Also applies to: 98-109
115-117: LGTM! Well-structured role-based callback logic.The role-based checks in the callback are well-implemented and clearly documented.
Also applies to: 124-124, 138-139
196-196: LGTM! Consistent role assignments.The role assignments align well with each agent's character and purpose in the game.
Also applies to: 217-217, 237-237, 260-260, 280-280, 301-301, 323-323
425-427: LGTM! Clear and comprehensive seller guidance.The additional seller guidance provides valuable strategic information.
🧰 Tools
🪛 Ruff (0.8.2)
416-427: f-string without any placeholders
Remove extraneous
fprefix(F541)
build please