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.

Sign up for GitHub

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

Jump to bottom

469 missing information in ipfsagentresult #521

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

469 missing information in ipfsagentresult #521

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10af4a8
Adding agent model to IPFS upload
gabrielfior Oct 21, 2024
1ed4389
Added agent name as property to DeployableTraderAgent
gabrielfior Oct 21, 2024
dda8993
Merge remote-tracking branch 'refs/remotes/origin/main' into 469-miss…
gabrielfior Oct 21, 2024
bcd2682
Added agent name as property to DeployableTraderAgent
gabrielfior Oct 21, 2024
f353ff3
Update prediction_market_agent_tooling/markets/omen/data_models.py
gabrielfior Oct 22, 2024
75228f6
Passing agent_name
gabrielfior Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion prediction_market_agent_tooling/deploy/agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
MarketType,
have_bet_on_market_since,
)
from prediction_market_agent_tooling.markets.omen.data_models import IPFSAgentResult
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid importing Omen-specific classes in a generic agent module

Importing IPFSAgentResult from markets.omen.data_models introduces Omen-specific dependencies into the generic DeployableAgent class hierarchy. This reduces modularity and reusability, potentially causing issues for other markets like Metaculus that do not require IPFS uploads.

Consider abstracting the result handling to be market-agnostic. You could define a generic interface or base class for agent results and let market-specific implementations extend it. This way, DeployableTraderAgent remains decoupled from any specific market logic.

from prediction_market_agent_tooling.markets.omen.omen import (
withdraw_wxdai_to_xdai_to_keep_balance,
)
Expand Down Expand Up @@ -334,6 +335,10 @@ def update_langfuse_trace_by_processed_market(
]
)

@property
def agent_name(self) -> str:
return self.__class__.__name__

def check_min_required_balance_to_operate(self, market_type: MarketType) -> None:
api_keys = APIKeys()

Expand Down Expand Up @@ -492,7 +497,15 @@ def after_process_market(
processed_market: ProcessedMarket,
) -> None:
keys = APIKeys()
market.store_prediction(processed_market=processed_market, keys=keys)
reasoning = (
processed_market.answer.reasoning
if processed_market.answer.reasoning
else ""
)
agent_result = IPFSAgentResult(reasoning=reasoning, agent_name=self.agent_name)
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 moved this logic from OmenAgentMarket -> DeployableTraderAgent because the property agent_name belongs to DeployableTraderAgent.
One could simply pass the property as arg, but I would argue that the responsibility to build a IPFSAgentResult should be handled by the agent itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

IPFSAgentResult is specific to Omen; for example, Metaculus wants only a comment without any IPFS upload.

That refactoring was made based on Evan's comment because there were a lot of Omen-uniquess in the DeployableTraderAgent.

Maybe it could be named IPFSOmenAgentResult or something to make it clearer in the code. 🤔

Wdyt just passing agent_name into store_prediction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No strong preference here - passing agent_name is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it could be named IPFSOmenAgentResult or something to make it clearer in the code. 🤔

I would prefer to keep it as IPFSAgentResult - we can rename it once we have other agents uploading results as well.

market.store_prediction(
processed_market=processed_market, keys=keys, agent_result=agent_result
)

def before_process_markets(self, market_type: MarketType) -> None:
"""
Expand Down
6 changes: 5 additions & 1 deletion prediction_market_agent_tooling/markets/agent_market.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
TokenAmount,
)
from prediction_market_agent_tooling.markets.market_fees import MarketFees
from prediction_market_agent_tooling.markets.omen.data_models import IPFSAgentResult
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing IPFSAgentResult Parameter in Subclasses and Method Call

Several areas in the codebase have not been updated to include the IPFSAgentResult parameter:

  • Subclasses missing IPFSAgentResult in store_prediction:

    • prediction_market_agent_tooling/markets/omen/omen.py

  • Method call missing IPFSAgentResult argument:

    • prediction_market_agent_tooling/deploy/agent.py

Please update the store_prediction methods in the affected subclasses and ensure all calls to store_prediction include the IPFSAgentResult parameter.

🔗 Analysis chain

Verify implementation in subclasses and related components.

The changes to the AgentMarket class, while minimal, have a significant impact on the codebase:

  1. All subclasses implementing store_prediction will need to be updated to handle the new agent_result parameter.
  2. Any code calling store_prediction will need to provide the IPFSAgentResult argument.

Please run the following script to identify potential areas that need updates:

Ensure that all identified areas are updated to accommodate the new agent_result parameter.

Also applies to: 232-235

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Identify classes that inherit from AgentMarket and places where store_prediction is called.

# Find subclasses of AgentMarket
echo "Subclasses of AgentMarket:"
rg "class \w+\(.*AgentMarket.*\):" -g '*.py'

# Find calls to store_prediction
echo "\nCalls to store_prediction:"
rg "\.store_prediction\(" -g '*.py'

Length of output: 902

Script:

#!/bin/bash
# Description: Verify that each subclass of AgentMarket has an updated store_prediction method with the IPFSAgentResult parameter.

# List of subclass files
subclasses=(
    "prediction_market_agent_tooling/markets/polymarket/polymarket.py"
    "prediction_market_agent_tooling/markets/omen/omen.py"
    "prediction_market_agent_tooling/markets/metaculus/metaculus.py"
    "prediction_market_agent_tooling/jobs/jobs_models.py"
    "prediction_market_agent_tooling/jobs/omen/omen_jobs.py"
    "prediction_market_agent_tooling/markets/manifold/manifold.py"
)

for file in "${subclasses[@]}"; do
    echo "Checking store_prediction method in $file:"
    rg "def store_prediction\s*\(" "$file" | while read -r line; do
        # Check if the method includes 'IPFSAgentResult' as a parameter
        if [[ "$line" =~ store_prediction\([^\)]*IPFSAgentResult[^\)]*\) ]]; then
            echo "  ✅ Updated store_prediction method found."
        else
            echo "  ❌ store_prediction method missing IPFSAgentResult parameter."
        fi
    done
done

# Check the call in deploy/agent.py
echo -e "\nChecking store_prediction call in deploy/agent.py:"
rg "\.store_prediction\s*\(" prediction_market_agent_tooling/deploy/agent.py | while read -r line; do
    # Check if the call includes 'IPFSAgentResult' argument
    if [[ "$line" =~ store_prediction\([^\)]*IPFSAgentResult[^\)]*\) ]]; then
        echo "  ✅ store_prediction call includes IPFSAgentResult argument."
    else
        echo "  ❌ store_prediction call is missing IPFSAgentResult argument."
    fi
done

Length of output: 3078

from prediction_market_agent_tooling.tools.utils import (
DatetimeUTC,
check_not_none,
Expand Down Expand Up @@ -228,7 +229,10 @@ def verify_operational_balance(api_keys: APIKeys) -> bool:
raise NotImplementedError("Subclasses must implement this method")

def store_prediction(
self, processed_market: ProcessedMarket, keys: APIKeys
self,
processed_market: ProcessedMarket,
keys: APIKeys,
agent_result: IPFSAgentResult,
) -> None:
"""
If market allows to upload predictions somewhere, implement it in this method.
Expand Down
2 changes: 1 addition & 1 deletion prediction_market_agent_tooling/markets/omen/data_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ def from_tuple(values: tuple[t.Any]) -> "ContractPrediction":

class IPFSAgentResult(BaseModel):
reasoning: str

agent_name: str | None
gabrielfior marked this conversation as resolved.
Show resolved Hide resolved
model_config = ConfigDict(
extra="forbid",
)
15 changes: 5 additions & 10 deletions prediction_market_agent_tooling/markets/omen/omen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,20 +417,15 @@ def verify_operational_balance(api_keys: APIKeys) -> bool:
) > xdai_type(0.001)

def store_prediction(
self, processed_market: ProcessedMarket, keys: APIKeys
self,
processed_market: ProcessedMarket,
keys: APIKeys,
agent_result: IPFSAgentResult,
) -> None:
reasoning = (
processed_market.answer.reasoning
if processed_market.answer.reasoning
else ""
)

ipfs_hash_decoded = HexBytes(HASH_ZERO)
if keys.enable_ipfs_upload:
logger.info("Storing prediction on IPFS.")
ipfs_hash = IPFSHandler(keys).store_agent_result(
IPFSAgentResult(reasoning=reasoning)
)
ipfs_hash = IPFSHandler(keys).store_agent_result(agent_result)
ipfs_hash_decoded = ipfscidv0_to_byte32(ipfs_hash)

tx_hashes = [
Expand Down
Loading