Skip to content
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

Streamlit web app for nft game #590

Merged
merged 12 commits into from
Dec 11, 2024
Merged

Streamlit web app for nft game #590

merged 12 commits into from
Dec 11, 2024

Conversation

kongzii
Copy link
Contributor

@kongzii kongzii commented Dec 9, 2024

Screenshot by Dropbox Capture

build please

Copy link
Contributor

coderabbitai bot commented Dec 9, 2024

Walkthrough

The pull request introduces a new enumeration class AgentIdentifier in identifiers.py, which defines string constants for various agent identifiers. This class is utilized across multiple files, including agents_nft_game.py, app.py, and deploy.py, where it replaces previous string representations of agent identifiers. The changes also include the addition of new classes, methods, and constants in several files, enhancing the structure and clarity of agent management within the prediction market agent framework.

Changes

File Change Summary
prediction_market_agent/agents/identifiers.py Added class AgentIdentifier(str, Enum) for agent identifiers.
prediction_market_agent/agents/microchain_agent/agents_nft_game.py Added class NFTAgent(BaseModel), constants AGENTS_IN_THE_GAME, TREASURY_SAFE_ADDRESS, NFT_TOKEN_FACTORY.
prediction_market_agent/agents/microchain_agent/app.py Updated imports for AgentIdentifier, refactored initialization methods to use from_agent_identifier.
prediction_market_agent/agents/microchain_agent/app_nft_game.py Added function show_treasury_part(), updated get_agent_page signature, added variable treasury_xdai_balance.
prediction_market_agent/agents/microchain_agent/deploy.py Replaced task_description with identifier: AgentIdentifier in multiple classes.
prediction_market_agent/agents/social_media_agent/deploy.py Updated import for AgentIdentifier, modified instantiation of LongTermMemoryTableHandler.
prediction_market_agent/agents/think_thoroughly_agent/think_thoroughly_agent.py Changed identifier type from str to AgentIdentifier, updated initialization logic.
prediction_market_agent/agents/utils.py Removed class AgentIdentifier(str, Enum).
prediction_market_agent/db/long_term_memory_table_handler.py Added static method from_agent_identifier to create instances using AgentIdentifier.
prediction_market_agent/db/models.py Added property method metadata_dict in LongTermMemories.
prediction_market_agent/db/prompt_table_handler.py Added static method from_agent_identifier, updated constructor signature for session_identifier.
scripts/deployed_general_agent_viewer.py Updated import for AgentIdentifier, renamed task_description to identifier.
prediction_market_agent/agents/microchain_agent/memory.py Updated static methods in DatedChatMessage and SimpleMemoryThinkThoroughly for metadata handling.
tests/db/test_long_term_memory_table_handler.py Updated assertions for metadata validation.

Possibly related PRs

Suggested reviewers

  • evangriffiths

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f064138 and 754740b.

📒 Files selected for processing (1)
  • prediction_market_agent/db/models.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • prediction_market_agent/db/models.py

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (10)
prediction_market_agent/agents/identifiers.py (2)

4-19: Consider organizing identifiers into logical groups

The enum contains multiple related identifiers that could benefit from better organization. Consider grouping them using either:

  1. Separate enums for each category (THINK_THOROUGHLY, MICROCHAIN, NFT_GAME)
  2. Comments to visually separate groups
  3. Consistent prefix/suffix patterns

This would improve maintainability as more identifiers are added.

Example organization:

class AgentIdentifier(str, Enum):
    # Think Thoroughly Agents
    THINK_THOROUGHLY = "think-thoroughly-agent"
    THINK_THOROUGHLY_PROPHET = "think-thoroughly-prophet-research-agent"
    
    # Microchain Agents
    MICROCHAIN_AGENT_OMEN = "microchain-agent-deployment-omen"
    MICROCHAIN_AGENT_OMEN_TEST = "microchain-agent-deployment-omen_test"
    MICROCHAIN_AGENT_OMEN_LEARNING_0 = "general-agent-0"
    # ... other microchain agents ...
    
    # NFT Game Agents
    NFT_GAME_AGENT_0 = "nft-game-agent-0"
    NFT_GAME_AGENT_1 = "nft-game-agent-1"
    # ... other NFT game agents ...

8-8: Inconsistent naming in string value

The test agent identifier uses an underscore in its string value while others use hyphens:

  • "microchain-agent-deployment-omen_test" vs "microchain-agent-deployment-omen"

This inconsistency could cause issues with string matching or filtering.

-    MICROCHAIN_AGENT_OMEN_TEST = "microchain-agent-deployment-omen_test"
+    MICROCHAIN_AGENT_OMEN_TEST = "microchain-agent-deployment-omen-test"
scripts/delete_agent_db_entries.py (1)

Line range hint 12-17: Add type hints and improve function documentation

The main function's documentation could be more descriptive and include type hints.

 def main(
     session_id: AgentIdentifier,
     delete_memories: bool = True,
     delete_prompts: bool = True,
     delete_goals: bool = True,
 ) -> None:
-    """
-    Delete all memories and prompts for a given agent, defined by the session_id.
-    """
+    """
+    Delete database entries for a specified agent.
+    
+    Args:
+        session_id (AgentIdentifier): The identifier of the agent
+        delete_memories (bool, optional): Delete memory entries. Defaults to True.
+        delete_prompts (bool, optional): Delete prompt entries. Defaults to True.
+        delete_goals (bool, optional): Delete goal entries. Defaults to True.
+    
+    Raises:
+        Exception: If any entries fail to delete
+    """
prediction_market_agent/agents/microchain_agent/agents_nft_game.py (2)

19-70: Consider moving agent configurations to a separate file

The hardcoded agent configurations might be better placed in a separate configuration file for easier maintenance and updates.

Consider creating a new file agent_configs.py and moving the configurations there:

from typing import List
from prediction_market_agent.agents.microchain_agent.agents_nft_game import NFTAgent

AGENTS_IN_THE_GAME: List[NFTAgent] = [
    # ... agent configurations ...
]

71-76: Consider environment variables for blockchain addresses

Hardcoding blockchain addresses directly in the source code might make it difficult to manage different environments (testnet vs mainnet).

Consider using environment variables or a configuration system that supports different environments:

from prediction_market_agent_tooling.config import get_config

TREASURY_SAFE_ADDRESS = Web3.to_checksum_address(
    get_config("TREASURY_SAFE_ADDRESS")
)
NFT_KEYS_ADDRESS = Web3.to_checksum_address(
    get_config("NFT_KEYS_ADDRESS")
)
prediction_market_agent/agents/microchain_agent/app_nft_game.py (5)

1-5: Enhance documentation with additional setup details.

Consider adding more details to the documentation:

  • Required environment variables
  • Dependencies installation instructions
  • Expected behavior when running the app

34-44: Add error handling for database connection failures.

The cached handler functions should include try-except blocks to gracefully handle potential database connection failures.

 @st.cache_resource
 def long_term_memory_table_handler(
     identifier: AgentIdentifier,
 ) -> LongTermMemoryTableHandler:
+    try:
         return LongTermMemoryTableHandler.from_agent_identifier(identifier)
+    except Exception as e:
+        st.error(f"Failed to connect to memory database: {str(e)}")
+        return None

90-90: Remove unnecessary f-string.

The f-string doesn't contain any placeholders.

-    st.markdown(f"""### Agent's actions""")
+    st.markdown("""### Agent's actions""")
🧰 Tools
🪛 Ruff (0.8.0)

90-90: f-string without any placeholders

Remove extraneous f prefix

(F541)


89-107: Implement pagination for better performance.

Loading all actions at once could be slow with a large history. Consider implementing pagination.

 def show_function_calls_part(nft_agent: NFTAgent) -> None:
     st.markdown("""### Agent's actions""")
+    page_size = 10
+    page = st.number_input("Page", min_value=1, value=1)
 
     with st.spinner("Loading agent's actions..."):
-        calls = long_term_memory_table_handler(nft_agent.identifier).search()
+        calls = long_term_memory_table_handler(nft_agent.identifier).search(
+            limit=page_size,
+            offset=(page - 1) * page_size
+        )
🧰 Tools
🪛 Ruff (0.8.0)

90-90: f-string without any placeholders

Remove extraneous f prefix

(F541)


142-147: Add error handling for treasury balance display.

The treasury balance fetching should handle potential failures gracefully.

-treasury_xdai_balance = get_balances(TREASURY_SAFE_ADDRESS).xdai
+try:
+    treasury_xdai_balance = get_balances(TREASURY_SAFE_ADDRESS).xdai
+except Exception as e:
+    st.sidebar.error(f"Failed to fetch treasury balance: {str(e)}")
+    treasury_xdai_balance = 0
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c38d21d and 875cf24.

📒 Files selected for processing (13)
  • prediction_market_agent/agents/identifiers.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/agents_nft_game.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/app.py (4 hunks)
  • prediction_market_agent/agents/microchain_agent/app_nft_game.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/deploy.py (5 hunks)
  • prediction_market_agent/agents/social_media_agent/deploy.py (2 hunks)
  • prediction_market_agent/agents/think_thoroughly_agent/think_thoroughly_agent.py (5 hunks)
  • prediction_market_agent/agents/utils.py (0 hunks)
  • prediction_market_agent/db/long_term_memory_table_handler.py (2 hunks)
  • prediction_market_agent/db/models.py (2 hunks)
  • prediction_market_agent/db/prompt_table_handler.py (1 hunks)
  • scripts/delete_agent_db_entries.py (2 hunks)
  • scripts/deployed_general_agent_viewer.py (4 hunks)
💤 Files with no reviewable changes (1)
  • prediction_market_agent/agents/utils.py
🧰 Additional context used
🪛 Ruff (0.8.0)
prediction_market_agent/agents/microchain_agent/app_nft_game.py

90-90: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (11)
prediction_market_agent/db/prompt_table_handler.py (2)

20-24: LGTM: Factory method enhances code organization

The new factory method provides a clean way to create handlers from agent identifiers, improving code organization and type safety.


14-14: Breaking change: session_identifier is now required

The constructor parameter session_identifier has been changed from optional to required. This is a breaking change that might affect existing code.

✅ Verification successful

Let me check the test file to see how the constructor is being used and if there are any instances where the session_identifier is omitted.


Let me check if there are any other usages of PromptTableHandler in the codebase, particularly focusing on the actual implementation to see the before/after state of the constructor.


Let me try to see the current implementation of the file to understand the constructor's definition.


Based on the evidence gathered:

  1. The only usages of PromptTableHandler constructor are in tests and through the new factory method from_agent_identifier
  2. The tests already provide the required session_identifier parameter
  3. The new factory method ensures the parameter is always provided
  4. There are no other direct instantiations of PromptTableHandler in the codebase

The session_identifier parameter change is safe

The change to make session_identifier required is not a breaking change as all existing usages already provide this parameter, and new usages are guided through the factory method from_agent_identifier.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing instantiations that might be affected
rg "PromptTableHandler\(" --type py

Length of output: 238


Script:

#!/bin/bash
# Check the test file content
cat tests/db/test_prompt_table_handler.py

Length of output: 1330


Script:

#!/bin/bash
# Check the implementation file to see constructor changes
git diff prediction_market_agent/db/prompt_table_handler.py

Length of output: 61


Script:

#!/bin/bash
# Check the current implementation
cat prediction_market_agent/db/prompt_table_handler.py

Length of output: 2025

prediction_market_agent/agents/microchain_agent/agents_nft_game.py (1)

8-16: LGTM: Well-structured agent model

The NFTAgent class is well-designed using Pydantic for type safety and validation. The url property provides a clean way to generate URL-friendly names.

prediction_market_agent/db/long_term_memory_table_handler.py (1)

22-26: LGTM: Consistent factory method implementation

The new factory method aligns well with the similar implementation in PromptTableHandler, maintaining consistency across the codebase.

prediction_market_agent/agents/social_media_agent/deploy.py (1)

53-55: LGTM! Clean transition to AgentIdentifier.

The change from direct instantiation to using from_agent_identifier is clean and consistent with the codebase-wide refactoring.

scripts/deployed_general_agent_viewer.py (2)

30-30: LGTM: Import statement updated correctly.

The import statement has been updated to use the new location for AgentIdentifier, aligning with the codebase's restructuring.


Line range hint 128-130: Verify initialization of LongTermMemoryTableHandler.

The initialization has been updated to use the new from_agent_identifier factory method. Let's verify that all agent identifiers are properly handled.

✅ Verification successful

All agent identifiers are properly handled through the enum class

Based on the search results, I can verify that:

  1. The AgentIdentifier is properly defined as an enum class in prediction_market_agent/agents/identifiers.py
  2. The from_agent_identifier factory method in LongTermMemoryTableHandler correctly accepts an AgentIdentifier enum and uses its value as the task description
  3. All usages of from_agent_identifier across the codebase consistently pass valid AgentIdentifier enum values
  4. The specific usage in scripts/deployed_general_agent_viewer.py is correct as it uses a valid agent identifier selected from the available enum values
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all agent identifiers used in the codebase are valid enum values

# Search for all agent identifiers used with LongTermMemoryTableHandler
ast-grep --pattern 'LongTermMemoryTableHandler.from_agent_identifier($identifier)'

# Search for all AgentIdentifier enum values
ast-grep --pattern 'class AgentIdentifier'

Length of output: 129


Script:

#!/bin/bash
# Let's try a different approach to find the usage and definition

# Search for LongTermMemoryTableHandler usage
rg "LongTermMemoryTableHandler" -A 3

# Search for AgentIdentifier usage and definition
rg "AgentIdentifier" -A 3

# Search for from_agent_identifier usage
rg "from_agent_identifier" -A 3

Length of output: 37850

prediction_market_agent/agents/microchain_agent/app.py (2)

33-33: LGTM: Import statement added correctly.

The AgentIdentifier import has been properly added to support the enum-based agent identification.


146-146: Verify PromptTableHandler initialization.

The initialization has been updated to use the new from_agent_identifier factory method. This change aligns with the new pattern of using enumerated identifiers.

✅ Verification successful

Initialization of PromptTableHandler is consistent with the new pattern

The verification shows that all usages of PromptTableHandler across the codebase consistently use the from_agent_identifier factory method with AgentIdentifier enums, including:

  • prediction_market_agent/agents/microchain_agent/app.py
  • prediction_market_agent/agents/microchain_agent/deploy.py
  • prediction_market_agent/agents/microchain_agent/app_nft_game.py
  • scripts/delete_agent_db_entries.py

The implementation aligns with the intended pattern of using enumerated identifiers.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that PromptTableHandler initialization is consistent across the codebase

# Search for all PromptTableHandler initializations
ast-grep --pattern 'PromptTableHandler.from_agent_identifier($identifier)'

Length of output: 76


Script:

#!/bin/bash
# Let's try a broader search for PromptTableHandler usage
rg "PromptTableHandler" -A 2 -B 2

# Also search for the specific method
rg "from_agent_identifier" -A 2 -B 2

# And look for the actual class definition
ast-grep --pattern 'class PromptTableHandler {
  $$$
}'

Length of output: 12979

prediction_market_agent/agents/think_thoroughly_agent/think_thoroughly_agent.py (2)

107-107: LGTM: Type annotation updated correctly.

The identifier type has been properly updated to use the new AgentIdentifier enum type, improving type safety.


117-119: LGTM: Memory handler initialization updated.

The long-term memory handler initialization has been properly updated to use the new from_agent_identifier factory method with appropriate null handling.

Comment on lines 16 to 21
@property
def metadata_dict(self) -> dict[str, Any] | None:
out: dict[str, Any] | None = (
json.loads(self.metadata_) if self.metadata_ else None
)
return out
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and documentation for metadata_dict property

The property lacks error handling for JSON parsing and documentation explaining its purpose.

Consider this implementation:

     @property
     def metadata_dict(self) -> dict[str, Any] | None:
+        """
+        Converts the metadata_ JSON string to a dictionary.
+        
+        Returns:
+            dict[str, Any] | None: Parsed metadata dictionary or None if metadata_ is not set
+        
+        Raises:
+            json.JSONDecodeError: If metadata_ contains invalid JSON
+        """
         out: dict[str, Any] | None = (
-            json.loads(self.metadata_) if self.metadata_ else None
+            None if not self.metadata_ else json.loads(self.metadata_)
         )
         return out
📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@property
def metadata_dict(self) -> dict[str, Any] | None:
out: dict[str, Any] | None = (
json.loads(self.metadata_) if self.metadata_ else None
)
return out
@property
def metadata_dict(self) -> dict[str, Any] | None:
"""
Converts the metadata_ JSON string to a dictionary.
Returns:
dict[str, Any] | None: Parsed metadata dictionary or None if metadata_ is not set
Raises:
json.JSONDecodeError: If metadata_ contains invalid JSON
"""
out: dict[str, Any] | None = (
None if not self.metadata_ else json.loads(self.metadata_)
)
return out

Copy link
Contributor

@gabrielfior gabrielfior Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point @RabbitAI, we should add error handling for JSON parsing since DB can hold any string in that column

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabrielfior What do you want to handle there? If it's not JSON it will error out and that's exactly what we want it to do, because it means a bug.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting a more graceful error handling, instead of what json.loads gives us.

Comment on lines 23 to 27
prompt_handler = PromptTableHandler.from_agent_identifier(session_id)
prompt_handler.delete_all_prompts()
if prompt_handler.fetch_latest_prompt() is not None:
raise Exception("Prompt entries were not deleted.")
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Standardize handler initialization and improve error messages

The script shows inconsistent patterns in handler initialization and could benefit from more informative messages.

Consider these improvements:

  1. Standardize handler initialization:
-        evaluated_goal_table_handler = EvaluatedGoalTableHandler(agent_id=session_id)
+        evaluated_goal_table_handler = EvaluatedGoalTableHandler.from_agent_identifier(session_id)
  1. Add more informative messages:
-            raise Exception("Prompt entries were not deleted.")
+            raise Exception(f"Failed to delete prompt entries for agent {session_id.value}")
         else:
-            print("Prompt entries successfully deleted.")
+            print(f"Successfully deleted all prompt entries for agent {session_id.value}")
  1. Consider adding a count of deleted entries in success messages:
deleted_count = len(prompt_handler.fetch_all_prompts())
prompt_handler.delete_all_prompts()
print(f"Successfully deleted {deleted_count} prompt entries for agent {session_id.value}")

Also applies to: 31-35, 39-44

Comment on lines 62 to 64
if role == "assistant":
parsed_function = message.split("(")[0]
parsed_body = message.split("(")[1].rsplit(")")[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve function parsing robustness.

Current string splitting approach is fragile and could break with complex messages containing parentheses.

Consider using regex or a proper parser:

-        parsed_function = message.split("(")[0]
-        parsed_body = message.split("(")[1].rsplit(")")[0]
+        import re
+        if match := re.match(r"(\w+)\((.*)\)$", message.strip()):
+            parsed_function, parsed_body = match.groups()
+        else:
+            parsed_function = "Unknown"
+            parsed_body = message
📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if role == "assistant":
parsed_function = message.split("(")[0]
parsed_body = message.split("(")[1].rsplit(")")[0]
if role == "assistant":
import re
if match := re.match(r"(\w+)\((.*)\)$", message.strip()):
parsed_function, parsed_body = match.groups()
else:
parsed_function = "Unknown"
parsed_body = message

Comment on lines +109 to +126
def show_about_agent_part(nft_agent: NFTAgent) -> None:
system_prompt = prompt_table_handler(nft_agent.identifier).fetch_latest_prompt()
xdai_balance = get_balances(nft_agent.wallet_address).xdai
st.markdown(
f"""### {nft_agent.name}

Currently holds <span style='font-size: 1.1em;'><strong>{xdai_balance:.2f} xDAI</strong></span>.

---
""",
unsafe_allow_html=True,
)
st.text_area(
f"{nft_agent.name}'s system prompt",
value=system_prompt.prompt if system_prompt else "No system prompt yet.",
disabled=True,
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve security and error handling.

  1. Using unsafe_allow_html=True could expose XSS vulnerabilities
  2. No error handling for balance fetching

Consider these improvements:

 def show_about_agent_part(nft_agent: NFTAgent) -> None:
     system_prompt = prompt_table_handler(nft_agent.identifier).fetch_latest_prompt()
-    xdai_balance = get_balances(nft_agent.wallet_address).xdai
+    try:
+        xdai_balance = get_balances(nft_agent.wallet_address).xdai
+    except Exception as e:
+        st.error(f"Failed to fetch balance: {str(e)}")
+        xdai_balance = 0
+
+    # Use st.markdown's native formatting instead of HTML
     st.markdown(
-        f"""### {nft_agent.name}
-
-Currently holds <span style='font-size: 1.1em;'><strong>{xdai_balance:.2f} xDAI</strong></span>.
-
----
-""",
-        unsafe_allow_html=True,
+        f"""### {nft_agent.name}\n\nCurrently holds **{xdai_balance:.2f} xDAI**.\n\n---"""
     )
📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def show_about_agent_part(nft_agent: NFTAgent) -> None:
system_prompt = prompt_table_handler(nft_agent.identifier).fetch_latest_prompt()
xdai_balance = get_balances(nft_agent.wallet_address).xdai
st.markdown(
f"""### {nft_agent.name}
Currently holds <span style='font-size: 1.1em;'><strong>{xdai_balance:.2f} xDAI</strong></span>.
---
""",
unsafe_allow_html=True,
)
st.text_area(
f"{nft_agent.name}'s system prompt",
value=system_prompt.prompt if system_prompt else "No system prompt yet.",
disabled=True,
)
def show_about_agent_part(nft_agent: NFTAgent) -> None:
system_prompt = prompt_table_handler(nft_agent.identifier).fetch_latest_prompt()
try:
xdai_balance = get_balances(nft_agent.wallet_address).xdai
except Exception as e:
st.error(f"Failed to fetch balance: {str(e)}")
xdai_balance = 0
# Use st.markdown's native formatting instead of HTML
st.markdown(
f"""### {nft_agent.name}\n\nCurrently holds **{xdai_balance:.2f} xDAI**.\n\n---"""
)
st.text_area(
f"{nft_agent.name}'s system prompt",
value=system_prompt.prompt if system_prompt else "No system prompt yet.",
disabled=True,
)

Comment on lines +46 to +53
def send_message_part(nft_agent: NFTAgent) -> None:
message = st.text_area("Write a message to the agent")

if st.button("Send message", disabled=not message):
# TODO: Don't do this manually with deployment private key, use the user's wallet!
SendPaidMessageToAnotherAgent()(nft_agent.wallet_address, message)
st.success("Message sent and will be processed soon!")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement input validation and secure wallet integration.

Current implementation has several concerns:

  1. No input validation for the message content
  2. Manual wallet handling (as noted in TODO)
  3. No rate limiting for message sending

Consider implementing:

 def send_message_part(nft_agent: NFTAgent) -> None:
     message = st.text_area("Write a message to the agent")
+    if message and len(message) > 1000:  # Example limit
+        st.error("Message too long. Maximum 1000 characters allowed.")
+        return
 
     if st.button("Send message", disabled=not message):
+        if not message.strip():
+            st.error("Message cannot be empty")
+            return
         # TODO: Don't do this manually with deployment private key, use the user's wallet!
         SendPaidMessageToAnotherAgent()(nft_agent.wallet_address, message)
         st.success("Message sent and will be processed soon!")
📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def send_message_part(nft_agent: NFTAgent) -> None:
message = st.text_area("Write a message to the agent")
if st.button("Send message", disabled=not message):
# TODO: Don't do this manually with deployment private key, use the user's wallet!
SendPaidMessageToAnotherAgent()(nft_agent.wallet_address, message)
st.success("Message sent and will be processed soon!")
def send_message_part(nft_agent: NFTAgent) -> None:
message = st.text_area("Write a message to the agent")
if message and len(message) > 1000: # Example limit
st.error("Message too long. Maximum 1000 characters allowed.")
return
if st.button("Send message", disabled=not message):
if not message.strip():
st.error("Message cannot be empty")
return
# TODO: Don't do this manually with deployment private key, use the user's wallet!
SendPaidMessageToAnotherAgent()(nft_agent.wallet_address, message)
st.success("Message sent and will be processed soon!")

@@ -38,7 +38,7 @@ class DeployableMicrochainAgent(DeployableAgent):
n_iterations = 50
load_historical_prompt: bool = False
system_prompt_choice: SystemPromptChoice = SystemPromptChoice.TRADING_AGENT
task_description = AgentIdentifier.MICROCHAIN_AGENT_OMEN
identifier: AgentIdentifier = AgentIdentifier.MICROCHAIN_AGENT_OMEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

The transition from task_description to identifier is not complete

The codebase still contains multiple references to task_description in the following locations:

  • prediction_market_agent/db/models.py: Model definition still uses task_description
  • prediction_market_agent/db/long_term_memory_table_handler.py: Multiple occurrences in class methods and database operations
  • Several test files still use task_description:
    • tests/test_chat_history.py
    • tests/agents/microchain/test_functions.py
    • tests/db/test_long_term_memory_table_handler.py
🔗 Analysis chain

Verify the complete transition from task_description to identifier.

The changes look good. Let's verify that all occurrences of task_description have been replaced.

Also applies to: 66-66, 69-72, 142-142, 149-149, 156-156, 163-163, 171-171, 181-181

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify no remaining task_description references
# Search for any remaining references to task_description
rg "task_description" --type python

Length of output: 66


Script:

#!/bin/bash
# Search for any remaining references to task_description without file type restriction
rg "task_description"

Length of output: 1745

@@ -373,7 +376,7 @@ def answer_binary_market(


class ThinkThoroughlyWithItsOwnResearch(ThinkThoroughlyBase):
identifier = "think-thoroughly-agent"
identifier = AgentIdentifier.THINK_THOROUGHLY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Model versions need to be updated to match supported versions

The codebase shows inconsistencies in model versioning:

  • gpt-4-turbo-2024-04-09 is not a valid OpenAI model version
  • gpt-4o-2024-08-06 appears to be a custom alias defined in SupportedModel enum, but is used directly in some files

Suggested changes:

  • Update ThinkThoroughlyWithItsOwnResearch to use gpt-4-turbo-preview or gpt-4-0125-preview
  • Update ThinkThoroughlyWithPredictionProphetResearch to use the enum value SupportedModel.gpt_4o.value instead of hardcoding the version
  • Update other occurrences of these model versions throughout the codebase
🔗 Analysis chain

Verify model versions and compatibility.

The agent classes specify different model versions:

  • ThinkThoroughlyWithItsOwnResearch: gpt-4-turbo-2024-04-09
  • ThinkThoroughlyWithPredictionProphetResearch: gpt-4-turbo-2024-04-09 and gpt-4o-2024-08-06

Let's verify these model versions are valid and supported.

Also applies to: 446-446


Let me gather more information about model validation and configuration.


Based on the search results, I found the SupportedModel enum in microchain_agent/microchain_agent.py that defines valid model versions. Let me check if the models used in ThinkThoroughlyAgent are consistent with OpenAI's model lineup.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify model versions used across the codebase

# Search for all model version strings
rg -A 1 'model.*=.*"gpt-'

# Search for model version validation logic
ast-grep --pattern 'class SupportedModel'

Length of output: 3230


Script:

#!/bin/bash
# Search for model validation or configuration
rg -A 3 'class.*Model' 
rg -A 3 'SUPPORTED_MODELS'
rg -A 3 'MODEL_CONFIG'

Length of output: 13200


Script:

#!/bin/bash
# Get more context around model definitions and validations
rg -A 5 'gpt-4-turbo-2024-04-09|gpt-4o-2024-08-06' --context 5

Length of output: 7825

from enum import Enum


class AgentIdentifier(str, Enum):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to a new file because of circular imports.

return self.name.lower().replace(" ", "-")


AGENTS_IN_THE_GAME = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it doesn't look like the best thing to have everything hard-coded here, but that's what PMA repository currently expects -- that each agent is defined in the code.


@property
def url(self) -> str:
return self.name.lower().replace(" ", "-")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will work as https://final-streamlit-url/agent-name

@@ -124,7 +125,9 @@ def long_term_memory_is_initialized() -> bool:
def maybe_initialize_long_term_memory() -> None:
# Initialize the db storage
if not long_term_memory_is_initialized():
st.session_state.long_term_memory = LongTermMemoryTableHandler(AGENT_IDENTIFIER)
st.session_state.long_term_memory = (
LongTermMemoryTableHandler.from_agent_identifier(AGENT_IDENTIFIER)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a little identity issue with how to identify agents.

AgentIdentifier class exists, but in LongTermMemoryTableHandler it was passed into task_description argument, in PromptTableHandler it was passed to session_identifier argument., Somewhere it's used as string, somewhere as enum. Trying to consolidate it at least a little.

message = st.text_area("Write a message to the agent")

if st.button("Send message", disabled=not message):
# TODO: Don't do this manually with deployment private key, use the user's wallet!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue for this here #591

Turns out https://github.com/AlgoveraAI/streamlit-metamask is meant only for ERC-20 transfers (and didn't work on first try anyway 😄 ).

But I don't want to block the showcasing of this because of that, we can use this to show it off.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping the streamlit-metamask extension was gonna work like a charm :(

Anyways, what do you think about adding 1 text input (RPC_URL) so that we are able to send transactions on a local chain, instead of burning xDAI on Mainnet during the demo?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are able to change the GNOSIS_RPC_URL variable in the deployment's environment. It doesn't help to do it only here if all the agents aren't deployed on the same local chain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GNOSIS_RPC_URL is probably good enough for now - I want to avoid doing demos, etc on the main chain, but I have other ideas. This can be closed.

@kongzii kongzii mentioned this pull request Dec 9, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
prediction_market_agent/agents/microchain_agent/app_nft_game.py (1)

103-103: Remove unnecessary f-string prefix

The f-string on line 103 does not contain any placeholders, so the f prefix is unnecessary.

Apply this diff to remove the unnecessary prefix:

-st.markdown(f"""### Agent's actions""")
+st.markdown("""### Agent's actions""")
🧰 Tools
🪛 Ruff (0.8.0)

103-103: f-string without any placeholders

Remove extraneous f prefix

(F541)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 875cf24 and 3c2bc80.

📒 Files selected for processing (2)
  • prediction_market_agent/agents/microchain_agent/agents_nft_game.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/app_nft_game.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • prediction_market_agent/agents/microchain_agent/agents_nft_game.py
🧰 Additional context used
🪛 Ruff (0.8.0)
prediction_market_agent/agents/microchain_agent/app_nft_game.py

103-103: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (4)
prediction_market_agent/agents/microchain_agent/app_nft_game.py (4)

46-53: Implement input validation and secure wallet integration

The current implementation has several issues:

  1. No input validation for the message content.
  2. Manual wallet handling as noted in the TODO comment.

Consider implementing input validation to ensure the message is appropriate and integrating the user's wallet securely for sending messages.


61-62: Improve function parsing robustness

The current string splitting approach in parse_function_and_body may fail with complex messages containing parentheses.

Consider using regular expressions or a proper parser for more reliable parsing.


124-133: Improve security and error handling

  1. Using unsafe_allow_html=True may expose XSS vulnerabilities.
  2. No error handling for balance fetching.

Consider these improvements:


155-160: ⚠️ Potential issue

Add error handling for balance fetching and avoid unsafe_allow_html

The code does not handle potential exceptions when fetching the treasury balance, and using unsafe_allow_html=True can lead to security risks.

Apply this diff to improve error handling and enhance security:

-# Fetch the treasury balance without error handling
-treasury_xdai_balance = get_balances(TREASURY_SAFE_ADDRESS).xdai
+try:
+    treasury_xdai_balance = get_balances(TREASURY_SAFE_ADDRESS).xdai
+except Exception as e:
+    st.error(f"Failed to fetch treasury balance: {e}")
+    treasury_xdai_balance = 0

-# Display the balance using unsafe HTML
 st.sidebar.markdown(
-    f"""### Treasury
-Currently holds <span style='font-size: 1.1em;'><strong>{treasury_xdai_balance:.2f} xDAI</strong></span>.""",
-    unsafe_allow_html=True,
+    f"""### Treasury\nCurrently holds **{treasury_xdai_balance:.2f} xDAI**."""
 )

Likely invalid or redundant comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
prediction_market_agent/agents/microchain_agent/agents_nft_game.py (2)

8-16: Consider enhancing the URL property implementation.

While the current implementation works for basic cases, consider making it more robust:

  1. Handle special characters beyond spaces
  2. Add length validation for the name field
  3. Consider URL-encoding the result
 @property
 def url(self) -> str:
-    return self.name.lower().replace(" ", "-")
+    import re
+    from urllib.parse import quote
+    # Remove special chars, convert spaces to hyphens, ensure single hyphens
+    clean_name = re.sub(r'[^\w\s-]', '', self.name.lower())
+    clean_name = re.sub(r'[-\s]+', '-', clean_name).strip('-')
+    return quote(clean_name)

71-76: Add documentation for critical addresses.

The TREASURY_SAFE_ADDRESS and NFT_KEYS_ADDRESS appear to be critical system addresses. Consider adding docstrings or comments explaining:

  1. The purpose of each address
  2. The expected behavior/interactions
  3. Any security implications
+# Safe address holding treasury funds for the NFT game
 TREASURY_SAFE_ADDRESS = Web3.to_checksum_address(
     "0xd1A54FD7a200C2ca76B6D06437795d660d37FE28"
 )
+# Contract address for NFT key management
 NFT_KEYS_ADDRESS = Web3.to_checksum_address(
     "0x0D7C0Bd4169D090038c6F41CFd066958fe7619D0"
 )
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3c2bc80 and 54259bd.

📒 Files selected for processing (1)
  • prediction_market_agent/agents/microchain_agent/agents_nft_game.py (1 hunks)
🔇 Additional comments (2)
prediction_market_agent/agents/microchain_agent/agents_nft_game.py (2)

1-6: LGTM! Imports are well-organized and appropriate.

The imports are clean, necessary, and well-structured with standard library imports separated from local imports.


19-70: Verify Ethereum addresses validity and uniqueness.

While the addresses are properly checksummed, let's verify:

  1. All addresses are valid Ethereum addresses
  2. No address reuse across agents
  3. Addresses match expected format
✅ Verification successful

Let me modify the script to correctly check only the addresses in the NFT game agents file and fix the path issue:


Ethereum addresses are valid and properly implemented

The verification confirms:

  • All addresses are unique (no duplicates found)
  • Each address follows the correct Ethereum format (0x + 40 hex characters)
  • Total of 12 addresses (6 pairs of wallet/mech addresses for 6 agents) matches the expected count
  • All addresses are properly checksummed
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Ethereum addresses in the agents list

# Extract all addresses and verify format
echo "Verifying address format and uniqueness..."
rg -o '0x[a-fA-F0-9]{40}' | sort | uniq -c | while read count addr; do
    if [ "$count" -gt 1 ]; then
        echo "WARNING: Address $addr is used $count times"
    fi
    
    # Verify address length
    if [ ${#addr} -ne 42 ]; then
        echo "ERROR: Invalid address length for $addr"
    fi
done

Length of output: 11596


Script:

#!/bin/bash
# Description: Verify Ethereum addresses specifically in agents_nft_game.py

echo "Verifying addresses in agents_nft_game.py..."
rg -o '0x[a-fA-F0-9]{40}' prediction_market_agent/agents/microchain_agent/agents_nft_game.py | sort | uniq -c | while read count addr; do
    if [ "$count" -gt 1 ]; then
        echo "WARNING: Address $addr is used $count times"
    fi
done

# Count total unique addresses to ensure we have the right number
echo -e "\nTotal unique addresses:"
rg -o '0x[a-fA-F0-9]{40}' prediction_market_agent/agents/microchain_agent/agents_nft_game.py | sort -u | wc -l

Length of output: 839

Comment on lines +125 to +131
st.markdown(
f"""### {nft_agent.name}

Currently holds <span style='font-size: 1.1em;'><strong>{xdai_balance:.2f} xDAI</strong></span>.

---
""",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do this for bold on markdown

**bold** not bold

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bold yes, but bigger font size, no.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
tests/db/test_long_term_memory_table_handler.py (1)

Line range hint 1-48: Consider adding more test cases for better coverage

While the current test cases cover basic functionality, consider adding:

  • Edge cases (empty metadata, large metadata)
  • Error scenarios (invalid metadata format)
  • Boundary conditions for timestamp filtering

Example test cases to add:

def test_empty_metadata(memory_long_term_memory_handler):
    empty_item = {}
    memory_long_term_memory_handler.save_history([empty_item])
    results = memory_long_term_memory_handler.search()
    assert results[0].metadata_dict == empty_item

def test_timestamp_boundary_conditions(memory_long_term_memory_handler):
    item = {"test": "data"}
    timestamp = utcnow()
    memory_long_term_memory_handler.save_history([item])
    
    # Test exact timestamp match
    results = memory_long_term_memory_handler.search(from_=timestamp, to_=timestamp)
    assert len(results) == 0  # Should be empty as item was saved after timestamp
prediction_market_agent/agents/microchain_agent/agents_nft_game.py (2)

14-16: Ensure URL safety in the url property

The current implementation may not handle special or non-ASCII characters in agent names, which could lead to invalid URLs. Consider using URL encoding to ensure the generated URLs are valid.

Apply this diff to enhance URL safety:

+     from urllib.parse import quote
+ 
      @property
      def url(self) -> str:
-         return self.name.lower().replace(" ", "-")
+         return quote(self.name.lower().replace(" ", "-"))

19-70: Consider externalizing agent configurations

Hardcoding agent data within the codebase reduces maintainability and flexibility. Loading agent configurations from an external file or environment variables can make updates easier and improve separation of data from code.

Here's how you might modify the code to load agents from a JSON file:

+ import json
+ 
  AGENTS_IN_THE_GAME = [
-     NFTAgent(
-         name="Sneaky Pete",
-         identifier=AgentIdentifier.NFT_GAME_AGENT_0,
-         wallet_address=Web3.to_checksum_address(
-             "0xd845A24014B3BD96212A21f602a4F16A7dA518A4"
-         ),
-         mech_address=Web3.to_checksum_address(
-             "0xDDe0780F744B84b505E344931F37cEDEaD8B6163"
-         ),
-     ),
-     # ... other agents ...
- ]
+     NFTAgent(**agent_data)
+     for agent_data in json.load(open('agents_config.json'))
+ ]

And create an agents_config.json file with the agent data:

[
  {
    "name": "Sneaky Pete",
    "identifier": "NFT_GAME_AGENT_0",
    "wallet_address": "0xd845A24014B3BD96212A21f602a4F16A7dA518A4",
    "mech_address": "0xDDe0780F744B84b505E344931F37cEDEaD8B6163"
  },
  // ... other agents ...
]
prediction_market_agent/agents/microchain_agent/app_nft_game.py (1)

108-108: Remove unnecessary f-string

The string does not contain any placeholders; therefore, the f prefix is unnecessary.

Apply this diff to remove the extraneous f prefix:

-     st.markdown(f"""### Agent's actions""")
+     st.markdown("""### Agent's actions""")
🧰 Tools
🪛 Ruff (0.8.0)

108-108: f-string without any placeholders

Remove extraneous f prefix

(F541)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 54259bd and 7f22d0b.

📒 Files selected for processing (4)
  • prediction_market_agent/agents/microchain_agent/agents_nft_game.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/app_nft_game.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/memory.py (2 hunks)
  • tests/db/test_long_term_memory_table_handler.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.0)
prediction_market_agent/agents/microchain_agent/app_nft_game.py

108-108: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (7)
tests/db/test_long_term_memory_table_handler.py (1)

41-41: LGTM! Improved assertion readability

The change to use metadata_dict instead of JSON parsing makes the test more maintainable and efficient.

Also applies to: 45-45

prediction_market_agent/agents/microchain_agent/memory.py (2)

37-37: LGTM!

Using metadata_dict with check_not_none improves data retrieval by directly accessing the dictionary, ensuring that metadata is present.


57-58: LGTM!

Updating to use model_validate with metadata_dict enhances metadata validation and aligns with the updated data handling approach.

prediction_market_agent/agents/microchain_agent/app_nft_game.py (4)

62-63: Improve function parsing robustness

The current string splitting approach may fail with unexpected message formats or nested parentheses. Consider using regular expressions to parse the function and arguments more reliably.

Apply this diff to enhance parsing:

      def parse_function_and_body(
          role: t.Literal["user", "assistant", "system"], message: str
      ) -> t.Tuple[str | None, str | None]:
          message = message.strip()

-         if role == "assistant":
-             parsed_function = message.split("(")[0]
-             parsed_body = message.split("(")[1].rsplit(")")[0]
+         import re
+         if role == "assistant":
+             match = re.match(r"(\w+)\((.*)\)$", message)
+             if match:
+                 parsed_function, parsed_body = match.groups()
+             else:
+                 parsed_function = "Unknown"
+                 parsed_body = message
          elif role == "user":
              parsed_function = "Response"
              parsed_body = message

49-51: ⚠️ Potential issue

Implement wallet integration and input validation

The code currently lacks input validation and uses manual wallet handling, as noted in the TODO comment. Integrating the user's wallet securely and validating inputs are essential to prevent potential security issues.

Would you like assistance in implementing secure wallet integration and input validation?


130-138: 🛠️ Refactor suggestion

Avoid using unsafe_allow_html=True to prevent XSS vulnerabilities

Using unsafe_allow_html=True can expose the application to XSS attacks. Consider using Markdown formatting to achieve the desired styling without enabling unsafe HTML.

You can modify the code as follows:

      xdai_balance = get_balances(nft_agent.wallet_address).xdai
      st.markdown(
-         f"""### {nft_agent.name}
- 
- Currently holds <span style='font-size: 1.1em;'><strong>{xdai_balance:.2f} xDAI</strong></span>.
- 
- ---
- """,
-         unsafe_allow_html=True,
+         f"""### {nft_agent.name}
+ 
+ Currently holds **{xdai_balance:.2f} xDAI**.
+ 
+ ---""",
      )

161-165: 🛠️ Refactor suggestion

Avoid using unsafe_allow_html=True to prevent XSS vulnerabilities

Using unsafe_allow_html=True in the sidebar can also pose XSS risks. Use Markdown formatting to display the balance safely.

Modify the code as follows:

  st.sidebar.markdown(
-     f"""### Treasury
- Currently holds <span style='font-size: 1.1em;'><strong>{treasury_xdai_balance:.2f} xDAI</strong></span>.""",
-     unsafe_allow_html=True,
+     f"""### Treasury
+ Currently holds **{treasury_xdai_balance:.2f} xDAI**.""",
  )

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
prediction_market_agent/agents/microchain_agent/app_nft_game.py (3)

1-5: Enhance docstring with setup prerequisites.

The running instructions could be more comprehensive by including:

  • Required environment variables
  • Dependencies installation steps
  • Expected behavior when running locally

Would you like me to help generate a more detailed docstring?


110-110: Remove unnecessary f-string.

The string doesn't contain any placeholders, making the f-string unnecessary.

-    st.markdown(f"""### Agent's actions""")
+    st.markdown("""### Agent's actions""")
🧰 Tools
🪛 Ruff (0.8.0)

110-110: f-string without any placeholders

Remove extraneous f prefix

(F541)


108-169: Consider implementing rate limiting for auto-refresh.

While the auto-refresh implementation using @st.fragment is good, frequent refreshes every 5 seconds might cause unnecessary load on the backend services.

Consider:

  1. Increasing the refresh interval to 10-15 seconds
  2. Adding exponential backoff if no changes are detected
  3. Implementing a websocket-based approach for real-time updates

Would you like me to help implement any of these improvements?

🧰 Tools
🪛 Ruff (0.8.0)

110-110: f-string without any placeholders

Remove extraneous f prefix

(F541)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 7f22d0b and 7c3bdba.

⛔ Files ignored due to path filters (2)
  • poetry.lock is excluded by !**/*.lock, !**/*.lock
  • pyproject.toml is excluded by !**/*.toml
📒 Files selected for processing (3)
  • prediction_market_agent/agents/microchain_agent/app_nft_game.py (1 hunks)
  • prediction_market_agent/agents/microchain_agent/memory.py (2 hunks)
  • tests/db/test_long_term_memory_table_handler.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/db/test_long_term_memory_table_handler.py
  • prediction_market_agent/agents/microchain_agent/memory.py
🧰 Additional context used
🪛 Ruff (0.8.0)
prediction_market_agent/agents/microchain_agent/app_nft_game.py

110-110: f-string without any placeholders

Remove extraneous f prefix

(F541)

🔇 Additional comments (2)
prediction_market_agent/agents/microchain_agent/app_nft_game.py (2)

56-77: ⚠️ Potential issue

Improve message parsing robustness.

The current string splitting approach is fragile and could break with complex messages containing nested parentheses.

Consider using a proper parser or regex as suggested in the previous review.


149-156: ⚠️ Potential issue

Avoid using unsafe HTML in markdown.

Using unsafe_allow_html=True could expose XSS vulnerabilities.

Consider using Streamlit's native markdown formatting instead.

Comment on lines +35 to +45
@st.cache_resource
def long_term_memory_table_handler(
identifier: AgentIdentifier,
) -> LongTermMemoryTableHandler:
return LongTermMemoryTableHandler.from_agent_identifier(identifier)


@st.cache_resource
def prompt_table_handler(identifier: AgentIdentifier) -> PromptTableHandler:
return PromptTableHandler.from_agent_identifier(identifier)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for handler initialization.

The cached handlers could fail silently if initialization fails. Consider adding try-catch blocks and appropriate error messages.

 @st.cache_resource
 def long_term_memory_table_handler(
     identifier: AgentIdentifier,
 ) -> LongTermMemoryTableHandler:
+    try:
         return LongTermMemoryTableHandler.from_agent_identifier(identifier)
+    except Exception as e:
+        st.error(f"Failed to initialize memory handler: {str(e)}")
+        raise

 @st.cache_resource
 def prompt_table_handler(identifier: AgentIdentifier) -> PromptTableHandler:
+    try:
         return PromptTableHandler.from_agent_identifier(identifier)
+    except Exception as e:
+        st.error(f"Failed to initialize prompt handler: {str(e)}")
+        raise
📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@st.cache_resource
def long_term_memory_table_handler(
identifier: AgentIdentifier,
) -> LongTermMemoryTableHandler:
return LongTermMemoryTableHandler.from_agent_identifier(identifier)
@st.cache_resource
def prompt_table_handler(identifier: AgentIdentifier) -> PromptTableHandler:
return PromptTableHandler.from_agent_identifier(identifier)
@st.cache_resource
def long_term_memory_table_handler(
identifier: AgentIdentifier,
) -> LongTermMemoryTableHandler:
try:
return LongTermMemoryTableHandler.from_agent_identifier(identifier)
except Exception as e:
st.error(f"Failed to initialize memory handler: {str(e)}")
raise
@st.cache_resource
def prompt_table_handler(identifier: AgentIdentifier) -> PromptTableHandler:
try:
return PromptTableHandler.from_agent_identifier(identifier)
except Exception as e:
st.error(f"Failed to initialize prompt handler: {str(e)}")
raise

@kongzii kongzii merged commit 8928731 into main Dec 11, 2024
10 checks passed
@kongzii kongzii deleted the peter/demo branch December 11, 2024 12:18
@coderabbitai coderabbitai bot mentioned this pull request Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants