Add monitoring of deployed agents #8
Conversation
|
Warning Rate Limit Exceeded@evangriffiths has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 36 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. WalkthroughThe recent updates enhance a cloud deployment script for GCP, introduce a monitoring script for Manifold agents, and expand the prediction market agent tooling with new data models and functionalities. These changes improve the deployment frequency, extend the tooling for better interaction with the Manifold API, and provide detailed monitoring capabilities for prediction market agents, focusing on performance metrics and resolved bets. Changes
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 Configration File (
|
| @@ -13,9 +13,9 @@ | |||
| if __name__ == "__main__": | |||
| current_dir = os.path.dirname(os.path.realpath(__file__)) | |||
| fname = deploy_to_gcp( | |||
| requirements_file=f"{current_dir}/../../pyproject.toml", | |||
| requirements_file=f"{current_dir}/../../../pyproject.toml", | |||
There was a problem hiding this comment.
Bugfix: relative path changed when moving this file to PMAT
| @@ -10,7 +10,7 @@ buy_omen = "scripts.bet_omen:buy" | |||
| sell_omen = "scripts.bet_omen:sell" | |||
|
|
|||
| [tool.poetry.dependencies] | |||
| python = ">=3.9,<3.12" | |||
| python = ">=3.10,<3.12" | |||
There was a problem hiding this comment.
streamlit requires python >=3.10
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 10
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
poetry.lockis excluded by:!**/*.lockpyproject.tomlis excluded by:!**/*.toml
Files selected for processing (6)
- examples/cloud_deployment/gcp/deploy.py (2 hunks)
- examples/monitor/monitor.py (1 hunks)
- prediction_market_agent_tooling/markets/data_models.py (6 hunks)
- prediction_market_agent_tooling/markets/manifold.py (2 hunks)
- prediction_market_agent_tooling/monitor/markets/manifold.py (1 hunks)
- prediction_market_agent_tooling/monitor/monitor.py (1 hunks)
Additional comments: 8
examples/monitor/monitor.py (2)
- 11-11: Ensure that the
start_timecalculation aligns with the intended monitoring period. Subtracting one week from the current time might not always fit the monitoring requirements.- 15-15: Validate that
get_authenticated_user().idcorrectly fetches the user ID every time without exceptions or errors, especially in cases where authentication might fail.examples/cloud_deployment/gcp/deploy.py (2)
- 16-16: Validate that the updated path to
pyproject.tomlcorrectly points to the project's root directory, ensuring that dependencies are properly resolved during deployment.- 33-34: The updated scheduling frequency to run once every 2 hours is appropriate for most use cases. However, ensure this frequency aligns with the specific requirements of the deployed function and does not lead to unnecessary invocations or cost implications.
prediction_market_agent_tooling/monitor/monitor.py (1)
- 12-18: The
DeployedAgentclass is well-defined. Ensure that subclasses properly implement theget_resolved_betsmethod, as it's crucial for the monitoring functionality.prediction_market_agent_tooling/markets/data_models.py (3)
- 30-32: The
ProfitAmountclass is correctly defined. Ensure that theamountfield's data type aligns with the expected precision and scale for monetary values.- 42-50: The
ResolvedBetclass and itsis_correctproperty are well-implemented. Ensure that the logic for determining if a bet is correct aligns with the business rules.- 156-157: The
ManifoldMarketclass has been updated withresolutionandresolutionTimefields. Validate that these fields are correctly populated from the Manifold API responses and used appropriately in the application.
| class DeployedManifoldAgent(DeployedAgent): | ||
| manifold_user_id: str |
There was a problem hiding this comment.
Ensure the manifold_user_id attribute is properly documented, including its expected format and how it is used within the class.
| def get_resolved_bets(self) -> list[ResolvedBet]: | ||
| manifold_bets = get_resolved_manifold_bets( | ||
| user_id=self.manifold_user_id, | ||
| start_time=self.start_time, | ||
| end_time=None, | ||
| ) | ||
| return [manifold_to_generic_resolved_bet(b) for b in manifold_bets] |
There was a problem hiding this comment.
The method get_resolved_bets lacks error handling for API calls and data transformation. Consider adding try-except blocks to handle potential exceptions from the API call and data processing.
+ try:
manifold_bets = get_resolved_manifold_bets(
user_id=self.manifold_user_id,
start_time=self.start_time,
end_time=None,
)
return [manifold_to_generic_resolved_bet(b) for b in manifold_bets]
+ except Exception as e:
+ # Handle or log the exception appropriately
+ raiseCommittable 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.
| def get_resolved_bets(self) -> list[ResolvedBet]: | |
| manifold_bets = get_resolved_manifold_bets( | |
| user_id=self.manifold_user_id, | |
| start_time=self.start_time, | |
| end_time=None, | |
| ) | |
| return [manifold_to_generic_resolved_bet(b) for b in manifold_bets] | |
| def get_resolved_bets(self) -> list[ResolvedBet]: | |
| try: | |
| manifold_bets = get_resolved_manifold_bets( | |
| user_id=self.manifold_user_id, | |
| start_time=self.start_time, | |
| end_time=None, | |
| ) | |
| return [manifold_to_generic_resolved_bet(b) for b in manifold_bets] | |
| except Exception as e: | |
| # Handle or log the exception appropriately | |
| raise |
| extra_deps=[ | ||
| "git+https://github.com/gnosis/prediction-market-agent.git@evan/deploy-agent" | ||
| "git+https://github.com/gnosis/prediction-market-agent-tooling.git" |
There was a problem hiding this comment.
Confirm that the updated dependency URL is correct and points to the intended version of the prediction-market-agent-tooling repository. Consider pinning to a specific commit or tag for more predictable builds.
| def monitor_agent(agent: DeployedAgent) -> None: | ||
| agent_bets = agent.get_resolved_bets() | ||
| bets_info = { | ||
| "Market Question": [bet.market_question for bet in agent_bets], | ||
| "Bet Amount": [bet.amount.amount for bet in agent_bets], | ||
| "Bet Outcome": [bet.outcome for bet in agent_bets], | ||
| "Created Time": [bet.created_time for bet in agent_bets], | ||
| "Resolved Time": [bet.resolved_time for bet in agent_bets], | ||
| "Is Correct": [bet.is_correct for bet in agent_bets], | ||
| "Profit": [bet.profit.amount for bet in agent_bets], | ||
| } | ||
| bets_df = pd.DataFrame(bets_info).sort_values(by="Resolved Time") | ||
|
|
||
| st.set_page_config(layout="wide") | ||
| st.title(f"Monitoring Agent: '{agent.name}'") | ||
|
|
||
| # Metrics | ||
| col1, col2 = st.columns(2) | ||
| col1.metric(label="Number of bets", value=f"{len(agent_bets)}") | ||
| col2.metric(label="% Correct", value=f"{100 * bets_df['Is Correct'].mean():.2f}%") | ||
|
|
||
| # Chart of cumulative profit per day | ||
| profit_info = { | ||
| "Time": bets_df["Resolved Time"], | ||
| "Cumulative Profit": bets_df["Profit"].astype(float), | ||
| } | ||
| profit_df = pd.DataFrame(profit_info) | ||
| profit_df["Date"] = pd.to_datetime(profit_df["Time"].dt.date) | ||
| profit_df = ( | ||
| profit_df.groupby("Date")["Cumulative Profit"].sum().cumsum().reset_index() | ||
| ) | ||
| profit_df["Cumulative Profit"] = profit_df["Cumulative Profit"].astype(float) | ||
| st.empty() | ||
| st.altair_chart( | ||
| alt.Chart(profit_df) | ||
| .mark_line() | ||
| .encode( | ||
| x=alt.X("Date", axis=alt.Axis(format="%Y-%m-%d"), title=None), | ||
| y=alt.Y("Cumulative Profit", axis=alt.Axis(format=".2f")), | ||
| ) | ||
| .interactive(), | ||
| use_container_width=True, | ||
| ) | ||
|
|
||
| # Table of resolved bets | ||
| st.empty() | ||
| st.subheader("Resolved Bet History") | ||
| st.table(bets_df) |
There was a problem hiding this comment.
The monitor_agent function correctly fetches and processes bets data, then visualizes it using streamlit and altair. However, consider adding error handling for data fetching and processing to improve robustness.
+ try:
agent_bets = agent.get_resolved_bets()
# Existing processing and visualization code
+ except Exception as e:
+ st.error("Failed to fetch or process bets data.")
+ raiseCommittable 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.
| def monitor_agent(agent: DeployedAgent) -> None: | |
| agent_bets = agent.get_resolved_bets() | |
| bets_info = { | |
| "Market Question": [bet.market_question for bet in agent_bets], | |
| "Bet Amount": [bet.amount.amount for bet in agent_bets], | |
| "Bet Outcome": [bet.outcome for bet in agent_bets], | |
| "Created Time": [bet.created_time for bet in agent_bets], | |
| "Resolved Time": [bet.resolved_time for bet in agent_bets], | |
| "Is Correct": [bet.is_correct for bet in agent_bets], | |
| "Profit": [bet.profit.amount for bet in agent_bets], | |
| } | |
| bets_df = pd.DataFrame(bets_info).sort_values(by="Resolved Time") | |
| st.set_page_config(layout="wide") | |
| st.title(f"Monitoring Agent: '{agent.name}'") | |
| # Metrics | |
| col1, col2 = st.columns(2) | |
| col1.metric(label="Number of bets", value=f"{len(agent_bets)}") | |
| col2.metric(label="% Correct", value=f"{100 * bets_df['Is Correct'].mean():.2f}%") | |
| # Chart of cumulative profit per day | |
| profit_info = { | |
| "Time": bets_df["Resolved Time"], | |
| "Cumulative Profit": bets_df["Profit"].astype(float), | |
| } | |
| profit_df = pd.DataFrame(profit_info) | |
| profit_df["Date"] = pd.to_datetime(profit_df["Time"].dt.date) | |
| profit_df = ( | |
| profit_df.groupby("Date")["Cumulative Profit"].sum().cumsum().reset_index() | |
| ) | |
| profit_df["Cumulative Profit"] = profit_df["Cumulative Profit"].astype(float) | |
| st.empty() | |
| st.altair_chart( | |
| alt.Chart(profit_df) | |
| .mark_line() | |
| .encode( | |
| x=alt.X("Date", axis=alt.Axis(format="%Y-%m-%d"), title=None), | |
| y=alt.Y("Cumulative Profit", axis=alt.Axis(format=".2f")), | |
| ) | |
| .interactive(), | |
| use_container_width=True, | |
| ) | |
| # Table of resolved bets | |
| st.empty() | |
| st.subheader("Resolved Bet History") | |
| st.table(bets_df) | |
| def monitor_agent(agent: DeployedAgent) -> None: | |
| try: | |
| agent_bets = agent.get_resolved_bets() | |
| bets_info = { | |
| "Market Question": [bet.market_question for bet in agent_bets], | |
| "Bet Amount": [bet.amount.amount for bet in agent_bets], | |
| "Bet Outcome": [bet.outcome for bet in agent_bets], | |
| "Created Time": [bet.created_time for bet in agent_bets], | |
| "Resolved Time": [bet.resolved_time for bet in agent_bets], | |
| "Is Correct": [bet.is_correct for bet in agent_bets], | |
| "Profit": [bet.profit.amount for bet in agent_bets], | |
| } | |
| bets_df = pd.DataFrame(bets_info).sort_values(by="Resolved Time") | |
| st.set_page_config(layout="wide") | |
| st.title(f"Monitoring Agent: '{agent.name}'") | |
| # Metrics | |
| col1, col2 = st.columns(2) | |
| col1.metric(label="Number of bets", value=f"{len(agent_bets)}") | |
| col2.metric(label="% Correct", value=f"{100 * bets_df['Is Correct'].mean():.2f}%") | |
| # Chart of cumulative profit per day | |
| profit_info = { | |
| "Time": bets_df["Resolved Time"], | |
| "Cumulative Profit": bets_df["Profit"].astype(float), | |
| } | |
| profit_df = pd.DataFrame(profit_info) | |
| profit_df["Date"] = pd.to_datetime(profit_df["Time"].dt.date) | |
| profit_df = ( | |
| profit_df.groupby("Date")["Cumulative Profit"].sum().cumsum().reset_index() | |
| ) | |
| profit_df["Cumulative Profit"] = profit_df["Cumulative Profit"].astype(float) | |
| st.empty() | |
| st.altair_chart( | |
| alt.Chart(profit_df) | |
| .mark_line() | |
| .encode( | |
| x=alt.X("Date", axis=alt.Axis(format="%Y-%m-%d"), title=None), | |
| y=alt.Y("Cumulative Profit", axis=alt.Axis(format=".2f")), | |
| ) | |
| .interactive(), | |
| use_container_width=True, | |
| ) | |
| # Table of resolved bets | |
| st.empty() | |
| st.subheader("Resolved Bet History") | |
| st.table(bets_df) | |
| except Exception as e: | |
| st.error("Failed to fetch or process bets data.") | |
| raise |
| from datetime import datetime | ||
| import requests | ||
| import typing as t | ||
| from prediction_market_agent_tooling.gtypes import Mana | ||
| from prediction_market_agent_tooling.config import APIKeys | ||
| from prediction_market_agent_tooling.markets.data_models import ( | ||
| ProfitAmount, | ||
| ResolvedBet, | ||
| BetAmount, | ||
| Currency, | ||
| ManifoldBet, | ||
| ManifoldMarket, | ||
| ManifoldUser, | ||
| ManifoldContractMetric, |
There was a problem hiding this comment.
Ensure all imported modules and classes are used within the file. If any imports are unnecessary, consider removing them to clean up the code.
There was a problem hiding this comment.
And maybe https://github.com/PyCQA/autoflake for unused imports?
There was a problem hiding this comment.
@kongzii: Absolutely, using autoflake can be a great way to automatically remove unused imports and clean up the codebase. It's a useful tool for maintaining code quality, especially in larger projects where manually tracking imports can become cumbersome. You can integrate autoflake into your development workflow, for example, as part of your pre-commit hooks or CI/CD pipelines, to ensure that unused imports are removed automatically. However, it's also beneficial to periodically review the code manually, as automated tools might not always catch every context-specific nuance.
There was a problem hiding this comment.
Yes this too! fwiw there aren't any unused imports here, so not sure why coderabbit is commenting 😆
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| def get_authenticated_user() -> ManifoldUser: | ||
| url = "https://api.manifold.markets/v0/me" | ||
| headers = { | ||
| "Authorization": f"Key {APIKeys().manifold_api_key}", | ||
| "Content-Type": "application/json", | ||
| } | ||
| response = requests.get(url, headers=headers) | ||
| response.raise_for_status() | ||
| return ManifoldUser.model_validate(response.json()) |
There was a problem hiding this comment.
The get_authenticated_user function correctly fetches user information from the Manifold API. Ensure proper error handling is in place for cases where the API call fails or returns unexpected data.
+ try:
response = requests.get(url, headers=headers)
response.raise_for_status()
return ManifoldUser.model_validate(response.json())
+ except requests.exceptions.RequestException as e:
+ # Handle or log the exception appropriately
+ raiseCommittable 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.
| def get_authenticated_user() -> ManifoldUser: | |
| url = "https://api.manifold.markets/v0/me" | |
| headers = { | |
| "Authorization": f"Key {APIKeys().manifold_api_key}", | |
| "Content-Type": "application/json", | |
| } | |
| response = requests.get(url, headers=headers) | |
| response.raise_for_status() | |
| return ManifoldUser.model_validate(response.json()) | |
| def get_authenticated_user() -> ManifoldUser: | |
| url = "https://api.manifold.markets/v0/me" | |
| headers = { | |
| "Authorization": f"Key {APIKeys().manifold_api_key}", | |
| "Content-Type": "application/json", | |
| } | |
| try: | |
| response = requests.get(url, headers=headers) | |
| response.raise_for_status() | |
| return ManifoldUser.model_validate(response.json()) | |
| except requests.exceptions.RequestException as e: | |
| # Handle or log the exception appropriately | |
| raise |
| def get_resolved_manifold_bets( | ||
| user_id: str, | ||
| start_time: datetime, | ||
| end_time: t.Optional[datetime], | ||
| ) -> list[ManifoldBet]: | ||
| url = "https://api.manifold.markets/v0/bets" | ||
|
|
||
| params: dict[str, str] = {"userId": user_id} | ||
| response = requests.get(url, params=params) | ||
| response.raise_for_status() | ||
| bets = [ManifoldBet.model_validate(x) for x in response.json()] | ||
| bets = [b for b in bets if b.createdTime >= start_time] | ||
| if end_time: | ||
| bets = [b for b in bets if b.createdTime < end_time] | ||
| bets = [b for b in bets if get_manifold_market(b.contractId).isResolved] | ||
| return bets |
There was a problem hiding this comment.
The get_resolved_manifold_bets function lacks error handling for API calls and filtering logic. Consider adding try-except blocks to handle potential exceptions from the API call and ensure the filtering logic correctly handles edge cases.
+ try:
response = requests.get(url, params=params)
response.raise_for_status()
bets = [ManifoldBet.model_validate(x) for x in response.json()]
# Existing filtering logic
+ except requests.exceptions.RequestException as e:
+ # Handle or log the exception appropriately
+ raiseCommittable 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.
| def get_resolved_manifold_bets( | |
| user_id: str, | |
| start_time: datetime, | |
| end_time: t.Optional[datetime], | |
| ) -> list[ManifoldBet]: | |
| url = "https://api.manifold.markets/v0/bets" | |
| params: dict[str, str] = {"userId": user_id} | |
| response = requests.get(url, params=params) | |
| response.raise_for_status() | |
| bets = [ManifoldBet.model_validate(x) for x in response.json()] | |
| bets = [b for b in bets if b.createdTime >= start_time] | |
| if end_time: | |
| bets = [b for b in bets if b.createdTime < end_time] | |
| bets = [b for b in bets if get_manifold_market(b.contractId).isResolved] | |
| return bets | |
| def get_resolved_manifold_bets( | |
| user_id: str, | |
| start_time: datetime, | |
| end_time: t.Optional[datetime], | |
| ) -> list[ManifoldBet]: | |
| url = "https://api.manifold.markets/v0/bets" | |
| params: dict[str, str] = {"userId": user_id} | |
| try: | |
| response = requests.get(url, params=params) | |
| response.raise_for_status() | |
| bets = [ManifoldBet.model_validate(x) for x in response.json()] | |
| bets = [b for b in bets if b.createdTime >= start_time] | |
| if end_time: | |
| bets = [b for b in bets if b.createdTime < end_time] | |
| bets = [b for b in bets if get_manifold_market(b.contractId).isResolved] | |
| return bets | |
| except requests.exceptions.RequestException as e: | |
| # Handle or log the exception appropriately | |
| raise |
| @@ -13,7 +15,6 @@ | |||
| xDai, | |||
| Wei, | |||
| ) | |||
| from datetime import datetime | |||
There was a problem hiding this comment.
Not in this PR, but we could add https://pycqa.github.io/isort/ to the CI, to enforce some nice sorting, wdyt?
There was a problem hiding this comment.
Yes, keen for this!
| from datetime import datetime | ||
| import requests | ||
| import typing as t | ||
| from prediction_market_agent_tooling.gtypes import Mana | ||
| from prediction_market_agent_tooling.config import APIKeys | ||
| from prediction_market_agent_tooling.markets.data_models import ( | ||
| ProfitAmount, | ||
| ResolvedBet, | ||
| BetAmount, | ||
| Currency, | ||
| ManifoldBet, | ||
| ManifoldMarket, | ||
| ManifoldUser, | ||
| ManifoldContractMetric, |
There was a problem hiding this comment.
And maybe https://github.com/PyCQA/autoflake for unused imports?
|
|
||
| class DeployedAgent(BaseModel): | ||
| name: str | ||
| start_time: datetime = datetime.now().astimezone(tz=ZoneInfo("UTC")) |
There was a problem hiding this comment.
there is also datetime.utcnow() if you would like to spare some imports and characters
| bets_df = pd.DataFrame(bets_info).sort_values(by="Resolved Time") | ||
|
|
||
| st.set_page_config(layout="wide") | ||
| st.title(f"Monitoring Agent: '{agent.name}'") |
There was a problem hiding this comment.
set_page_config and title needs to be called as the first thing in streamlit and should be present only once, if someones wants to use monitor_agent(agent) after doing some other streamlit stuff before it, this will fail.
could you move it to examples/monitor/monitor.py?
There was a problem hiding this comment.
Do you mean the whole thing, or just those lines?
Not sure how much this will matter down the line, but for now I think it's good to be able to run monitor_agent from a repo where PMAT has been pip-installed. If the whole thing is moved to examples then they won't be able to do this.
Looks like it's just for st.set_page_config(layout="wide") that ordering matters. If I move this to the example, then you can do
if __name__ == "__main__":
...
st.set_page_config(layout="wide")
st.title(f"A titile")
st.subheader("A subheader")
monitor_agent(agent)
and it will look like:
so looks fine to keep the title in there.
There was a problem hiding this comment.
Having monitor_agent(agent) in the library is great, I meant to move only these two lines.
In docs it's recommended to call title only once, although it won't fail as with the page config.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
poetry.lockis excluded by:!**/*.lock
Files selected for processing (3)
- examples/monitor/monitor.py (1 hunks)
- prediction_market_agent_tooling/markets/manifold.py (2 hunks)
- prediction_market_agent_tooling/monitor/monitor.py (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- examples/monitor/monitor.py
- prediction_market_agent_tooling/markets/manifold.py
- prediction_market_agent_tooling/monitor/monitor.py
Summary
Streamlit example
Summary by CodeRabbit
New Features
Improvements
ManifoldMarketdata model with new fields for better market resolution tracking.Documentation