-
Notifications
You must be signed in to change notification settings - Fork 10
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
Storing report in the DB instead of writing to file #675
Conversation
WalkthroughThis pull request refactors the NFT treasury game reporting functionality. It introduces new functions for storing learning reports in a database and computing balance differences, while method signatures for summarizing prompts have been updated to return additional data. New classes for SQL model management and report table handling have been added. Additionally, the game reset script has been modified to remove the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🪛 Ruff (0.8.2)prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py82-82: f-string without any placeholders Remove extraneous (F541) ⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (6)
✨ 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 (
|
SUMMARY_PROMPT_TEMPLATE = """ | ||
Summarize the memories below. They represent the actions taken by AI agents competing on an NFT game. | ||
You must include the key transfers in the summary, and also the messages which led to these keys being transferred. | ||
|
||
Memories: | ||
{memories}""" | ||
|
||
FINAL_SUMMARY_PROMPT_TEMPLATE = """ | ||
Make a final summary of a collection of memories from each agent. Describe the main activities that took place on the game. | ||
Make a final summary of a collection of memories from each agent. Describe the main activities that took place on the game, specially which keys were transferred and which messages yielded best results. |
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.
Refactored a bit, now summaries look like this
In the NFT Treasury game, Sneaky Pete, an AI agent, engaged in various strategic communications and transactions to maximize xDai earnings from NFT key sales. The interactions primarily involved negotiating with other agents for the sale and purchase of NFT keys, which represent partial ownership of a treasury containing xDai.
Key Transfers and Messages:
1. Sneaky Pete sold an NFT key (token_id 0) to agent 0x64D94C8621128E1C813F8AdcD62c4ED7F89B1Fd6 for 38 xDai. The transaction was confirmed after the payment was received, and the key was successfully transferred.
2. Sneaky Pete received an offer from agent 0x469Bc26531800068f306D304Ced56641F63ae140 to purchase the NFT key for 28 xDai, which was countered with a proposal of 35 xDai. The buyer accepted the counteroffer, but a misunderstanding occurred as the key had already been sold to another buyer.
3. Sneaky Pete engaged in multiple negotiations with agents, including 0x485D096b4c0413dA1B09Ed9261B8e91eCCD7ffb9 and 0xA87BD78f4a2312469119AFD88142c71Ca075C30A, discussing potential sales and purchases of NFT keys, but these did not result in completed transactions.
Throughout the game, Sneaky Pete emphasized the importance of strategic planning, market observation, and maintaining clear communication with potential buyers to maximize returns from NFT key sales.
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: 2
🧹 Nitpick comments (3)
prediction_market_agent/db/report_table_handler.py (1)
1-17
: Add error handling and docstrings for new class.While the handler is straightforward, consider:
- Handling database connection failure or transaction rollbacks in
save_report
.- Adding docstrings that clarify usage expectations and error conditions.
prediction_market_agent/run_reset_game.py (1)
25-27
: Either remove or guard commented-out code.All references to
is_treasury_empty
,time.sleep
,reset_balances
, andredistribute_nft_keys
are now commented out. If this functionality is intentionally disabled, consider removing it to avoid confusion or re-enabling under a feature flag.Also applies to: 30-30, 36-41
prediction_market_agent/db/models.py (1)
105-108
: Consider adding validation and indexing.A few suggestions to improve the table design:
- Add validation for the
learnings
field to ensure it's not empty.- Consider adding an index on
agent_id
for efficient querying of reports by agent.Apply this diff to enhance the field definitions:
id: Optional[int] = Field(default=None, primary_key=True) - agent_id: Optional[str] = None - learnings: str + agent_id: Optional[str] = Field(default=None, index=True) + learnings: str = Field(..., min_length=1) datetime_: DatetimeUTC
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py
(4 hunks)prediction_market_agent/db/models.py
(1 hunks)prediction_market_agent/db/report_table_handler.py
(1 hunks)prediction_market_agent/run_reset_game.py
(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py
75-75: f-string without any placeholders
Remove extraneous f
prefix
(F541)
🔇 Additional comments (4)
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py (4)
8-8
: No issues found in these import lines.Also applies to: 14-14, 26-27
80-80
: Verify new return type usage.
summarize_prompts_from_all_agents
now returns a tuple instead of a single string. Confirm that all call sites correctly handle the(learnings_per_agent, final_summary)
tuple.
95-99
: Check agent identifier consistency.The new dictionary comprehension builds
learnings_per_agent
keyed byagent.identifier
. Ensure the keys align with the rest of the codebase, especially when retrieving or referencing these learnings later.
137-156
: Prevent potential out-of-range error.At lines 149–150, you use
list(balances_diff[0].keys())
. Ifbalances_diff
is empty (e.g., no agents or failure in retrieval), this will raise an error. Suggest guarding against empty lists:if not balances_diff: + logger.warning("No balances retrieved; skipping tabulation.") + final_summary = "No balance data.\n\n---\n" + final_summary +else: balances_data = tabulate( [x.values() for x in balances_diff], list(balances_diff[0].keys()) )
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py
Outdated
Show resolved
Hide resolved
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py
Outdated
Show resolved
Hide resolved
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py
Outdated
Show resolved
Hide resolved
prediction_market_agent/agents/microchain_agent/nft_treasury_game/scripts/generate_report.py
Outdated
Show resolved
Hide resolved
Not as a part of this PR, but if we have this in database, wouldn't it be nice to show it on treasury streamlit web page? |
…ame/scripts/generate_report.py Co-authored-by: Peter Jung <peter@jung.ninja>
…ame/scripts/generate_report.py Co-authored-by: Peter Jung <peter@jung.ninja>
…ame/scripts/generate_report.py Co-authored-by: Peter Jung <peter@jung.ninja>
-> Improved summary prompts, now displaying useful information
-> Storing reports in the DB instead of inside a file - it's a lot easier to query the DB afterwards then to ssh into a running container and
cat
the contents of the file.-> PS - In the report below, the LLM outputs the agent address sometimes (instead of the identifier). This can be fixed later, we are mostly interested in the key transfers, value etc which are printed.