-
Notifications
You must be signed in to change notification settings - Fork 5
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
Bump PMAT #525
Bump PMAT #525
Conversation
WalkthroughThe pull request introduces several modifications across multiple 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 (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
prediction_market_agent/agents/microchain_agent/market_functions.py (1)
Line range hint
380-384
: LGTM: Improved clarity in GetResolvedBetsWithOutcomes descriptionThe updated description for the
GetResolvedBetsWithOutcomes
class now clearly specifies the time frame for looking back at resolved bets. This change enhances clarity for users by providing context for then_days
parameter.Consider adding a brief explanation of what "resolved bets" means in this context to further improve clarity for users who might be unfamiliar with the term.
prediction_market_agent/agents/prophet_agent/deploy.py (2)
Line range hint
39-46
: Refactor duplicate code in theload
methodsThe
load
methods inDeployablePredictionProphetGPT4oAgent
,DeployablePredictionProphetGPT4TurboPreviewAgent
, andDeployablePredictionProphetGPT4TurboFinalAgent
contain similar code with minor differences in model names and parameters. To enhance maintainability and reduce code duplication, consider refactoring these methods to utilize a shared helper method or pass parameters to a common constructor.Here is an example of how you might refactor the code:
class DeployableTraderAgentER(DeployableTraderAgent): # ... existing code ... def initialize_agent( self, model_name: str, max_bet_amount: int, max_price_impact: float | None, ) -> None: self.agent = PredictionProphetAgent( model=model_name, include_reasoning=True, tavily_storage=TavilyStorage(agent_id=self.__class__.__name__), logger=logger, ) self.betting_strategy = KellyBettingStrategy( max_bet_amount=max_bet_amount, max_price_impact=max_price_impact, ) class DeployablePredictionProphetGPT4oAgent(DeployableTraderAgentER): bet_on_n_markets_per_run = 20 def load(self) -> None: super().load() self.initialize_agent( model_name="gpt-4o-2024-08-06", max_bet_amount=5, max_price_impact=0.7, ) # Apply similar changes in the other subclassesAlso applies to: 56-63, 73-80
Line range hint
35-35
: Inconsistent setting ofbet_on_n_markets_per_run
across agentsThe
bet_on_n_markets_per_run
attribute is set to20
inDeployablePredictionProphetGPT4oAgent
but is not defined in other agent classes. To maintain consistency and clarity, consider definingbet_on_n_markets_per_run
for all agent subclasses or documenting why it differs among agents.Also applies to: 51-51, 67-67, 83-83
prediction_market_agent/agents/think_thoroughly_agent/think_thoroughly_agent.py (3)
Line range hint
172-184
: Typo in method name: 'get_hypohetical_scenarios' should be 'get_hypothetical_scenarios'The method name
get_hypohetical_scenarios
is misspelled. Correcting it toget_hypothetical_scenarios
improves code readability and prevents potential confusion.Apply this diff to correct the method name:
-def get_hypohetical_scenarios(self, question: str) -> Scenarios: +def get_hypothetical_scenarios(self, question: str) -> Scenarios:Also, update all references to this method accordingly:
- hypothetical_scenarios = self.get_hypohetical_scenarios(question) + hypothetical_scenarios = self.get_hypothetical_scenarios(question)
Line range hint
469-470
: Typographical error in comment: 'pickable' should be 'pickleable'In the comment, the word 'pickable' should be 'pickleable' to correctly refer to the ability of an object to be serialized using the pickle module.
Apply this diff to fix the typographical error:
-# Needs to be a normal function outside of class, because `lambda` and `self` aren't pickable for processpool executor, +# Needs to be a normal function outside of class, because `lambda` and `self` aren't pickleable for processpool executor,
Line range hint
410-415
: Avoid catching broad exceptionsCatching all exceptions with a bare
except Exception
can mask unexpected errors and make debugging difficult. It's better to catch specific exceptions that are anticipated.Specify the exceptions you expect, for example:
- except Exception as e: + except (APIError, HTTPError, SomeSpecificException) as e:Replace
SomeSpecificException
with any other specific exception types you expect might occur.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
poetry.lock
is excluded by!**/*.lock
,!**/*.lock
pyproject.toml
is excluded by!**/*.toml
📒 Files selected for processing (4)
- prediction_market_agent/agents/microchain_agent/market_functions.py (1 hunks)
- prediction_market_agent/agents/prophet_agent/deploy.py (1 hunks)
- prediction_market_agent/agents/think_thoroughly_agent/think_thoroughly_agent.py (1 hunks)
- prediction_market_agent/tools/prediction_prophet/research.py (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- prediction_market_agent/tools/prediction_prophet/research.py
🧰 Additional context used
🔇 Additional comments (5)
prediction_market_agent/agents/microchain_agent/market_functions.py (4)
19-19
: LGTM: Import statement update for TavilyStorageThe change in the import statement for
TavilyStorage
aligns with similar updates in other files, reflecting a broader restructuring of the module organization. This modification improves code organization and potentially reduces import complexity.
Line range hint
132-133
: LGTM: Improved error handling in PredictProbabilityForQuestionThe addition of a check for
prediction.outcome_prediction
beingNone
and raising aValueError
in such cases is a valuable improvement. This change enhances error handling by ensuring that a failed prediction is explicitly communicated, preventing silent failures and improving the overall robustness of the code.
Line range hint
220-226
: LGTM: Enhanced transaction reliability in BuyTokens and SellTokensThe additions to the
BuyTokens
andSellTokens
classes improve the reliability of token transactions:
- The account balance check prevents transactions that would fail due to insufficient funds.
- The
withdraw_wxdai_to_xdai_to_keep_balance
call for OMEN markets ensures proper token balance management.These changes enhance the overall consistency and reliability of token transactions across different market types.
Also applies to: 321-325
Line range hint
1-424
: Overall assessment: Improvements in error handling, reliability, and documentationThe changes in this file consistently enhance the code quality through:
- Improved error handling in the
PredictProbabilityForQuestion
class.- Enhanced transaction reliability in the
BuyTokens
andSellTokens
classes.- Better documentation in the
GetResolvedBetsWithOutcomes
class.- Consistent import restructuring.
These modifications align with best practices and contribute to a more robust and maintainable codebase.
prediction_market_agent/agents/prophet_agent/deploy.py (1)
Line range hint
70-74
: Ensure handling ofmax_price_impact
when set toNone
In
DeployablePredictionProphetGPT4TurboFinalAgent
, theget_betting_strategy
method setsmax_price_impact
toNone
:def get_betting_strategy(self, market: AgentMarket) -> BettingStrategy: return KellyBettingStrategy(max_bet_amount=5, max_price_impact=None)Please confirm that setting
max_price_impact
toNone
is intentional and that theKellyBettingStrategy
and any downstream code can handle aNone
value without causing errors or unintended behavior.
from prediction_market_agent_tooling.tools.tavily_storage.tavily_models import ( | ||
TavilyStorage, | ||
) | ||
from prediction_market_agent_tooling.tools.tavily.tavily_storage import TavilyStorage |
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.
💡 Codebase verification
Invalid Import Path: prediction_market_agent_tooling.tools.tavily.tavily_storage
Does Not Exist
The module prediction_market_agent_tooling.tools.tavily.tavily_storage
does not exist, and TavilyStorage
is not defined within it. Please verify and correct the import path or ensure that the tavily_storage
module is created with the appropriate definition of TavilyStorage
.
- File Affected:
prediction_market_agent/agents/prophet_agent/deploy.py
🔗 Analysis chain
Verify the updated import path for TavilyStorage
Ensure that the module prediction_market_agent_tooling.tools.tavily.tavily_storage
exists and that TavilyStorage
is correctly defined within it. This change reflects a module reorganization; verify that all references to TavilyStorage
are updated accordingly throughout the codebase.
Run the following script to confirm the existence of TavilyStorage
in the new import path:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that TavilyStorage is accessible from the new import path.
# Test: Search for the definition of TavilyStorage in the codebase.
rg -A 5 'class TavilyStorage'
# Expected: Should find the class definition of TavilyStorage in the correct module file.
Length of output: 31
Script:
#!/bin/bash
# Description: Search for any definition of TavilyStorage in the codebase.
ast-grep --pattern $'$_ TavilyStorage($_)' --language python
Length of output: 250
Script:
#!/bin/bash
# Description: Verify the existence of the module path prediction_market_agent_tooling.tools.tavily.tavily_storage.
fd 'tavily_storage.py' .
Length of output: 24
Script:
#!/bin/bash
# Description: Corrected search for any definition of TavilyStorage in the codebase.
ast-grep --pattern $'$_ TavilyStorage($_)' --lang python
Length of output: 500
Script:
#!/bin/bash
# Description: Enhanced search for files related to tavily_storage in the codebase.
fd -i 'tavily_storage' .
Length of output: 24
Script:
#!/bin/bash
# Description: Search for any references to TavilyStorage in the codebase.
rg 'TavilyStorage'
Length of output: 1806
Script:
#!/bin/bash
# Description: List all files in the tavily directory to verify the existence of tavily_storage.py.
ls prediction_market_agent_tooling/tools/tavily/
Length of output: 143
Script:
#!/bin/bash
# Description: Search for any definitions of TavilyStorage (class, function, variable) in the codebase.
rg '^\s*(class|def)\s+TavilyStorage\b' --context 2
Length of output: 52
Script:
#!/bin/bash
# Description: Verify the directory structure for the TavilyStorage import path.
ls prediction_market_agent_tooling/tools/tavily/
Length of output: 143
No description provided.