Allow multiple general agents #305
Conversation
kongzii
commented
Jul 3, 2024
•
kongzii
commented
|
Warning Rate limit exceeded@kongzii has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 40 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes involve updating the Changes
Sequence DiagramsOld Flow for Retrieving BalancesequenceDiagram
participant User
participant MarketFunctions
participant Utils
User->>MarketFunctions: get_balance(market_type)
MarketFunctions->>Utils: get_balance(market_type)
Utils-->>MarketFunctions: balance
MarketFunctions-->>User: balance
New Flow for Retrieving BalancesequenceDiagram
participant User
participant MarketFunctions
participant Utils
User->>MarketFunctions: get_balance(keys, market_type)
MarketFunctions->>Utils: get_balance(keys, market_type)
Utils-->>MarketFunctions: balance
MarketFunctions-->>User: balance
Old Flow for Task Description InitializationsequenceDiagram
participant Deploy
participant AgentIdentifier
Deploy->>AgentIdentifier: MICROCHAIN_AGENT_OMEN
Deploy-->>AgentIdentifier: task_description
New Flow for Task Description InitializationsequenceDiagram
participant Deploy
participant AgentIdentifier
Deploy->>AgentIdentifier: MICROCHAIN_AGENT_OMEN
Deploy-->>AgentIdentifier: task_description
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
| @@ -18,16 +17,22 @@ | |||
| class AgentIdentifier(str, Enum): | |||
| THINK_THOROUGHLY = "think-thoroughly-agent" | |||
| MICROCHAIN_AGENT_OMEN = "microchain-agent-deployment-omen" | |||
| MICROCHAIN_AGENT_OMEN_TEST = "microchain-agent-deployment-omen_test" | |||
| MICROCHAIN_AGENT_OMEN_LEARNING_0 = "general-agent-0" | |||
There was a problem hiding this comment.
Named them just general-agent-X, because it was too long for the select box 😄
| from prediction_market_agent.utils import APIKeys | ||
|
|
||
|
|
||
| class DeployedGeneralAgentKeys(APIKeys): | ||
| START_TIME: datetime |
There was a problem hiding this comment.
Deleted starting time, as we can get it easily from the chat history object below. So we don't need to track this in the env for all the general agents.
There was a problem hiding this comment.
nice, that's better!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- prediction_market_agent/agents/microchain_agent/app.py (4 hunks)
- prediction_market_agent/agents/microchain_agent/deploy.py (3 hunks)
- prediction_market_agent/agents/microchain_agent/market_functions.py (3 hunks)
- prediction_market_agent/agents/microchain_agent/microchain_agent.py (1 hunks)
- prediction_market_agent/agents/microchain_agent/omen_functions.py (1 hunks)
- prediction_market_agent/agents/microchain_agent/utils.py (2 hunks)
- prediction_market_agent/agents/utils.py (2 hunks)
- prediction_market_agent/run_agent.py (3 hunks)
- scripts/deployed_general_agent_viewer.py (5 hunks)
- tests/agents/microchain/test_functions.py (2 hunks)
Additional context used
Ruff
scripts/deployed_general_agent_viewer.py
91-91: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (18)
prediction_market_agent/agents/microchain_agent/omen_functions.py (1)
21-24: LGTM! But verify the usage ofRedeemWinningBetsclass.The code changes are approved.
However, ensure that all instantiations and calls to
RedeemWinningBetsmatch the new signature.prediction_market_agent/run_agent.py (2)
20-22: Imports look good.The added imports for
DeployableMicrochainModifiableSystemPromptAgent0,DeployableMicrochainModifiableSystemPromptAgent1, andDeployableMicrochainModifiableSystemPromptAgent2are appropriate.
41-43: Enum values and dictionary entries look good.The new enum values
microchain_modifiable_system_prompt_0,microchain_modifiable_system_prompt_1, andmicrochain_modifiable_system_prompt_2in theRunnableAgentclass and the corresponding entries in theRUNNABLE_AGENTSdictionary are appropriate.Also applies to: 54-56
prediction_market_agent/agents/microchain_agent/deploy.py (4)
25-25: Attribute addition looks good.The
task_descriptionattribute added to theDeployableMicrochainAgentclass is appropriate and aligns with the newAgentIdentifiervalues.
35-38: Initialization updates look good.The updates to the
long_term_memoryandprompt_handlerinitialization in therunmethod ofDeployableMicrochainAgentto use thetask_descriptionattribute are appropriate.
54-58: Class update looks good.The
DeployableMicrochainModifiableSystemPromptAgent0class updated with atask_descriptionattribute is appropriate and aligns with the newAgentIdentifiervalues.
60-69: Class updates look good.The
DeployableMicrochainModifiableSystemPromptAgent1andDeployableMicrochainModifiableSystemPromptAgent2classes updated withtask_descriptionattributes are appropriate and align with the newAgentIdentifiervalues.prediction_market_agent/agents/microchain_agent/utils.py (2)
54-57: Function update looks good.The
get_balancefunction updated to include anAPIKeysparameter is appropriate and improves security and consistency in API key management.
67-69: Function update looks good.The
get_total_asset_valuefunction updated to include anAPIKeysparameter is appropriate and improves security and consistency in API key management.prediction_market_agent/agents/utils.py (2)
20-23: Enum values look good.The new
AgentIdentifiervaluesMICROCHAIN_AGENT_OMEN_TEST,MICROCHAIN_AGENT_OMEN_LEARNING_0,MICROCHAIN_AGENT_OMEN_LEARNING_1, andMICROCHAIN_AGENT_OMEN_LEARNING_2added to theAgentIdentifierenum are appropriate and align with the new deployable agent classes.
27-35: Method addition looks good.The
general_agent_identifiersmethod added to theAgentIdentifierclass is appropriate and improves code modularity and maintainability.scripts/deployed_general_agent_viewer.py (1)
35-54: LGTM!The
DeployedGeneralAgentSettingsclass is correctly utilizing Pydantic for settings management.prediction_market_agent/agents/microchain_agent/microchain_agent.py (1)
112-117: LGTM!The changes in the
mainfunction align with the PR objectives and look good.tests/agents/microchain/test_functions.py (1)
103-105: LGTM!The changes in the
test_buy_sell_tokensfunction align with the PR objectives and look good.prediction_market_agent/agents/microchain_agent/app.py (1)
109-109: LGTM!The changes in the
maybe_initialize_long_term_memoryfunction align with the PR objectives and look good.prediction_market_agent/agents/microchain_agent/market_functions.py (3)
122-125: LGTM!The changes in the
PredictProbabilityForQuestionRemotefunction align with the PR objectives and look good.
179-180: LGTM!The changes in the
BuyTokensfunction align with the PR objectives and look good.
288-289: LGTM!The changes in the
GetBalancefunction align with the PR objectives and look good.
| label=f"Agent Wallet Address", | ||
| value=keys.bet_from_address, | ||
| ) | ||
| st.metric(label=f"Agent Wallet Address", value=keys.bet_from_address) |
There was a problem hiding this comment.
Remove extraneous f prefix.
The f-string does not contain any placeholders and the f prefix is unnecessary.
- f"To see the agent's transaction history, click [here]({MARKET_TYPE.market_class.get_user_url(keys=keys)})."
+ "To see the agent's transaction history, click [here]({MARKET_TYPE.market_class.get_user_url(keys=keys)})."Committable suggestion was skipped due to low confidence.
Tools
Ruff
91-91: f-string without any placeholders
Remove extraneous
fprefix(F541)
| @@ -57,10 +68,22 @@ def starting_balance(self) -> float: | |||
| ) | |||
| st.title("Deployed Trader Agent Viewer") | |||
|
|
|||
| check_required_api_keys(["BET_FROM_PRIVATE_KEY"]) | |||
There was a problem hiding this comment.
check_required_api_keys not needed anymore, because now the requried env vars aren't optional in DeployedGeneralAgentSettings
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
scripts/deployed_general_agent_viewer.py (1)
96-96: Remove extraneousfprefix.The f-string does not contain any placeholders and the
fprefix is unnecessary.- f"Agent Wallet Address" + "Agent Wallet Address"Tools
Ruff
96-96: f-string without any placeholders
Remove extraneous
fprefix(F541)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- prediction_market_agent/agents/utils.py (2 hunks)
- scripts/deployed_general_agent_viewer.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- prediction_market_agent/agents/utils.py
Additional context used
Ruff
scripts/deployed_general_agent_viewer.py
96-96: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (11)
scripts/deployed_general_agent_viewer.py (11)
35-38: Ensure proper handling of environment variables inDeployedGeneralAgentSettings.The
model_configinSettingsConfigDictis correctly defined to read from the.envfile with UTF-8 encoding. Ensure the.envfile is properly managed and secured.
40-41: Ensure correct initialization ofSTARTING_BALANCEandAGENT_IDENTIFIER_TO_PRIVATE_KEY.The
STARTING_BALANCEandAGENT_IDENTIFIER_TO_PRIVATE_KEYattributes are crucial for the agent's operations. Ensure they are correctly initialized and validated.
47-49: Ensureagent_identifier_to_private_keyproperty returns the correct mapping.The
agent_identifier_to_private_keyproperty should return the correct mapping ofAgentIdentifiertoPrivateKey. Verify its correctness.
51-53: Ensureavailable_agentsproperty lists all valid agents.The
available_agentsproperty should return a list of all validAgentIdentifierkeys. Verify its correctness.
55-58: Ensureto_api_keysmethod correctly converts identifiers toAPIKeys.The
to_api_keysmethod should correctly convert anAgentIdentifiertoAPIKeys. Verify its correctness.
71-72: Ensure proper initialization ofDeployedGeneralAgentSettings.The
settingsobject is correctly initialized fromDeployedGeneralAgentSettings. Ensure the initialization is error-free.
73-80: Ensure correct agent selection in the sidebar.The agent selection dropdown should correctly list and handle all available agents. Verify its functionality.
85-86: Ensure correct conversion of agent identifier toAPIKeysand initialization ofstarting_balance.The
keysandstarting_balanceshould be correctly initialized from the selected agent. Verify their correctness.
96-96: Remove extraneousfprefix.The f-string does not contain any placeholders and the
fprefix is unnecessary.Tools
Ruff
96-96: f-string without any placeholders
Remove extraneous
fprefix(F541)
113-116: Ensurechat_historyandsessionsare correctly initialized.The
chat_historyandsessionsshould be correctly initialized from the long-term memory. Verify their correctness.
116-118: Ensure correct calculation oftotal_asset_valueandroi.The
total_asset_valueandroishould be correctly calculated. Verify their correctness.
| system_prompt_choice: SystemPromptChoice = SystemPromptChoice.JUST_BORN | ||
| load_historical_prompt: bool = True | ||
| task_description = AgentIdentifier.MICROCHAIN_AGENT_OMEN_LEARNING_0 |
There was a problem hiding this comment.
This could be set via env var, and then you wouldn't need to duplicate this python code, and all the duplication would just be in the terraform modules
There was a problem hiding this comment.
I know but I actually liked that we have this explicit list of available agents:
But I agree there is a ton of duplication, what do you think about it now? I did a small refactor.
There was a problem hiding this comment.
I'm not sure. My instinct is that multiple copies of the same agent should be specified only in the terraform code. But by that logic I should have made load_historical_prompt settable by env var too, and not have DeployableMicrochainModifiableSystemPromptAgent either. So happy to go with this for now :)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- prediction_market_agent/agents/microchain_agent/deploy.py (3 hunks)
Additional comments not posted (6)
prediction_market_agent/agents/microchain_agent/deploy.py (6)
59-59: LGTM!The introduction of the
task_descriptionattribute improves consistency.
65-65: LGTM!The introduction of the
task_descriptionattribute improves consistency.
71-71: LGTM!The introduction of the
task_descriptionattribute improves consistency.
77-77: LGTM!The introduction of the
task_descriptionattribute improves consistency.
25-25: LGTM! Verify the usage oftask_description.The introduction of the
task_descriptionattribute improves consistency. Ensure that all usages oftask_descriptionare correct and consistent across the codebase.Verification successful
LGTM! Verify the usage of
task_description.The introduction of the
task_descriptionattribute improves consistency. Ensure that all usages oftask_descriptionare correct and consistent across the codebase.
prediction_market_agent/agents/microchain_agent/deploy.pyprediction_market_agent/agents/microchain_agent/microchain_agent.pyprediction_market_agent/db/long_term_memory_table_handler.pyscripts/deployed_general_agent_viewer.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `task_description` in the codebase to ensure consistency. # Test: Search for the usage of `task_description`. Expect: Consistent usage across the codebase. rg --type python $'task_description'Length of output: 66
Script:
#!/bin/bash # Description: Verify all usages of `task_description` in the codebase to ensure consistency. # Test: Search for the usage of `task_description`. Expect: Consistent usage across the codebase. rg --type py 'task_description'Length of output: 3366
35-38: LGTM! Ensure proper initialization oflong_term_memoryandprompt_handler.The initialization of
long_term_memoryandprompt_handlerusingtask_descriptionis correct. Verify that thePromptTableHandlerandLongTermMemoryTableHandlerclasses handle thetask_descriptioncorrectly.Verification successful
Initialization of
long_term_memoryandprompt_handleris correct.The
PromptTableHandlerandLongTermMemoryTableHandlerclasses correctly handle thetask_descriptionparameter by assigning it to instance variables.
PromptTableHandleris defined inprediction_market_agent/db/prompt_table_handler.pyand handlessession_identifiercorrectly.LongTermMemoryTableHandleris defined inprediction_market_agent/db/long_term_memory_table_handler.pyand handlestask_descriptioncorrectly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization of `long_term_memory` and `prompt_handler` in `PromptTableHandler` and `LongTermMemoryTableHandler` classes. # Test: Search for the initialization of `long_term_memory` and `prompt_handler`. Expect: Correct handling of `task_description` in these classes. rg --type python $'PromptTableHandler' rg --type python $'LongTermMemoryTableHandler'Length of output: 145
Script:
#!/bin/bash # Description: Verify the initialization of `long_term_memory` and `prompt_handler` in `PromptTableHandler` and `LongTermMemoryTableHandler` classes. # Search for the definition of `PromptTableHandler` class and inspect its constructor rg --pcre2 --glob '*.py' 'class PromptTableHandler' -A 10 # Search for the definition of `LongTermMemoryTableHandler` class and inspect its constructor rg --pcre2 --glob '*.py' 'class LongTermMemoryTableHandler' -A 10Length of output: 2230
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
scripts/deployed_general_agent_viewer.py (2)
50-51: Consider adding type hints for dictionary keys and values.Adding type hints for the
AGENT_IDENTIFIER_TO_PRIVATE_KEYdictionary can improve code readability and maintainability.AGENT_IDENTIFIER_TO_PRIVATE_KEY: dict[AgentIdentifier, PrivateKey]
106-106: Remove extraneousfprefix.The f-string does not contain any placeholders and the
fprefix is unnecessary.- f"To see the agent's transaction history, click [here]({MARKET_TYPE.market_class.get_user_url(keys=keys)})." + "To see the agent's transaction history, click [here]({MARKET_TYPE.market_class.get_user_url(keys=keys)})."Tools
Ruff
106-106: f-string without any placeholders
Remove extraneous
fprefix(F541)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- prediction_market_agent/agents/microchain_agent/app.py (4 hunks)
- prediction_market_agent/agents/microchain_agent/deploy.py (3 hunks)
- prediction_market_agent/agents/microchain_agent/microchain_agent.py (1 hunks)
- prediction_market_agent/agents/microchain_agent/utils.py (2 hunks)
- prediction_market_agent/agents/utils.py (2 hunks)
- scripts/deployed_general_agent_viewer.py (6 hunks)
Files skipped from review as they are similar to previous changes (5)
- prediction_market_agent/agents/microchain_agent/app.py
- prediction_market_agent/agents/microchain_agent/deploy.py
- prediction_market_agent/agents/microchain_agent/microchain_agent.py
- prediction_market_agent/agents/microchain_agent/utils.py
- prediction_market_agent/agents/utils.py
Additional context used
Ruff
scripts/deployed_general_agent_viewer.py
106-106: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (3)
scripts/deployed_general_agent_viewer.py (3)
45-48: Ensureextraconfiguration is necessary.The
extra="ignore"configuration inSettingsConfigDictprevents extra fields in the.envfile from being considered. Ensure this behavior is intended.
83-93: EnsureAgentIdentifiervalues are valid.The
AgentIdentifiervalues used in theselectboxshould be validated to ensure they match the expected values.
126-127: Ensureget_total_asset_valuehandles API key correctly.Verify that the
get_total_asset_valuefunction correctly handles thekeysparameter.
| def to_api_keys(self, identifier: AgentIdentifier) -> APIKeys: | ||
| return APIKeys( | ||
| BET_FROM_PRIVATE_KEY=self.agent_identifier_to_private_key[identifier] | ||
| ) |
There was a problem hiding this comment.
Ensure safe access to dictionary keys.
Accessing the dictionary without checking if the key exists may result in a KeyError. Consider adding error handling or using .get() method.
def to_api_keys(self, identifier: AgentIdentifier) -> APIKeys:
private_key = self.agent_identifier_to_private_key.get(identifier)
if private_key is None:
raise ValueError(f"No private key found for agent identifier: {identifier}")
return APIKeys(BET_FROM_PRIVATE_KEY=private_key)
| @@ -18,16 +17,22 @@ | |||
| class AgentIdentifier(str, Enum): | |||
| THINK_THOROUGHLY = "think-thoroughly-agent" | |||
| MICROCHAIN_AGENT_OMEN = "microchain-agent-deployment-omen" | |||
| MICROCHAIN_AGENT_OMEN_TEST = "microchain-agent-deployment-omen_test" | |||
| MICROCHAIN_AGENT_OMEN_LEARNING_0 = "general-agent-0" | |||
| from prediction_market_agent.utils import APIKeys | ||
|
|
||
|
|
||
| class DeployedGeneralAgentKeys(APIKeys): | ||
| START_TIME: datetime |
There was a problem hiding this comment.
nice, that's better!
| system_prompt_choice: SystemPromptChoice = SystemPromptChoice.JUST_BORN | ||
| load_historical_prompt: bool = True | ||
| task_description = AgentIdentifier.MICROCHAIN_AGENT_OMEN_LEARNING_0 |
There was a problem hiding this comment.
I'm not sure. My instinct is that multiple copies of the same agent should be specified only in the terraform code. But by that logic I should have made load_historical_prompt settable by env var too, and not have DeployableMicrochainModifiableSystemPromptAgent either. So happy to go with this for now :)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- prediction_market_agent/run_agent.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- prediction_market_agent/run_agent.py
|
Waiting for funding for these new agents by finance people, if they won't send the some xDai soonish I will invest in them by myself 😄 |
