fix(agent-performance): lowercase safe address on Polymarket subgraph queries

[valory-coding-agent]

Implements: https://linear.app/valory-xyz/issue/OPE-1923

Description

What: _fetch_trader_agent and _fetch_trader_agent_bets bound the EIP-55 checksummed Safe address straight into their GraphQL variables. The Polymarket squid resolves traderAgentById(id: String!) and the traderAgent: {id_eq: $id} filter by exact string match against ids it stores lowercased, so both queries returned null / [] for every Polystrat agent. Those two call sites are the only sources of prediction accuracy and total ROI, so both rendered blank while predictions_made populated from a neighbouring helper whose caller already lowercased. fetch_performance_data_round then ended Event.FAIL and last_updated froze.

Why it was invisible on Omen: the same un-normalised code runs on Gnosis, but predict-omen is served by a Graph node whose ID! scalar normalises case. Same id: Bytes! in both schemas, two runtimes, opposite semantics.

Fix: .lower() on the variables binding inside each of the two helpers, both platform branches.

Type of Change

• Bug fix

Changes Made

• graph_tooling/requests.py - lowercase the Safe address on the variables binding in _fetch_trader_agent and _fetch_trader_agent_bets (Polymarket and Omen branches), with a docstring note on each.
• behaviours.py - reword the "Trader may be unstaked." warning at :937. It asserted a staking state the code never checked; the reporter's agent was STAKED throughout, and the claim sent the original investigation away from the query. String only, no behavioural change.
• tests/graph_tooling/test_requests.py - wire-level assertions on the address actually sent, plus a containment test.
• tests/test_behaviours.py - the corrected warning text, plus two over-application guards.
• packages.json, skill.yaml, aea-config.yaml, service.yaml - mechanical autonomy packages lock output.

Placement note for the reviewer

Section 10.1 of the technical scope left one question open: the .lower() calls go inside the two requests.py helpers, not at the caller sites behaviours.py:746 / :930 that the ticket names. This follows the scope's stated preference and the convention the file already carries (_fetch_ct_held_position_keys:453, _fetch_daily_profit_statistics:739), and each helper has exactly one production caller - the broken site itself - so the blast radius is nil.

The honest cost: the warnings at behaviours.py:752 and :937 still log the checksummed address while the query used the lowercased one. Moving the two calls to the caller sites removes that mismatch and substitutes cleanly with no other change. Say the word if you prefer it.

Deliberately not done

Per the reviewer feedback recorded in the scope ("lower case only for the input of the queries, the rest should be unchanged"), the v2 proposal to normalise all eleven address-carrying helpers was dropped. The other five (_fetch_mech_sender, _fetch_agent_details, _fetch_trader_agent_performance, _fetch_pending_bets, _fetch_all_mech_requests) are correct today only because their subgraph is Graph-node-backed or their caller lowercases first. That residual risk is stated rather than fixed: if another endpoint moves from The Graph to a squid, the same defect can reappear. test_forwards_the_address_verbatim pins that decision so a later "make it uniform" sweep has to be a conscious edit.

Normalisation was not pushed into _fetch_from_subgraph: that is the shared transport, and blanket-lowercasing there would corrupt the free-text questionTitles of GET_MECH_REQUESTS_BY_TITLES_QUERY.

POLYMARKET_AGENTS_SUBGRAPH_URL was not repointed at the Graph-node deployment. It accepts checksummed addresses, which makes it look like a zero-code fix, but the ticket measured it returning months-stale counts at the same chain head - that would turn "accuracy hidden" into "accuracy confidently wrong".

How to Test

1. uv sync --all-groups && uv run autonomy packages sync
2. uv run pytest packages/valory/skills/agent_performance_summary_abci/tests/ -q - 1237 pass.
3. Confirm the tests are a real regression gate: revert the four .lower() calls in requests.py and re-run -k "lowercase or verbatim". The four *_sends_lowercased_address tests fail; the two containment tests still pass.
4. Against a live Polystrat Safe, confirm prediction accuracy and Total ROI populate and last_updated resumes advancing. The ticket's external checks: service 166 -> 53.19% (97 bets, 94 resolved, 50 won); service 32 -> 62.56% (897 / 892 / 558).

Checklist

• Code follows project style guidelines
• Self-reviewed for obvious errors
• Tests added / updated
• Existing tests pass (1237 in this skill; whole-suite matrix left to CI)
• Documentation updated if needed (n/a - no API, config or schema change)
• Related linear ticket: OPE-1923

Verification run locally: tomte format-code; tomte tox -p -e black-check -e isort-check -e flake8 -e mypy -e pylint -e darglint; check-abci-docstrings, check-abciapp-specs, check-handlers, check-hash, check-packages, check-dependencies, check-third-party-hashes, check-doc-hashes, analyse-service, gitleaks; uv lock --check. All pass. No FSM spec diff, as the scope predicted.

Note: tomte check-copyright --author valory reports 42 files, identically with and without this change - it compares header end-years against file mtimes, which a shallow clone sets to today. Pre-existing and untouched here.

---

Technical Scope (from Linear, v3)

Technical Scoping — valory-xyz/trader — v3

TL;DR

• Scope deliberately shrunk on reviewer instruction. v2 proposed a uniform normalisation across eleven helpers plus two fetcher classes; the reviewer's feedback ("lower case only for the input of the queries, the rest should be unchanged") reverses that. v3 fixes the two broken query paths and nothing else — see Section 10 for what was dropped and why.
• The change is two .lower() calls, placed on the query-variable binding inside _fetch_trader_agent and _fetch_trader_agent_bets in graph_tooling/requests.py (Section 7 justifies that placement over the caller sites).
• Verified: each of those two helpers has exactly one production caller — behaviours.py:748 and :932, the two broken sites themselves. Blast radius is zero; nothing else in the repo reaches them.
• No API contract change and no schema change (Sections 5 and 6). The stale on-disk summary self-heals on the first successful round.
• One judgement call is being exercised against the reviewer's "unless it's a very low hanging fruit" carve-out: the misleading "Trader may be unstaked." warning at behaviours.py:937 (Section 7).
• Main risk is over-application — two nearby sites read the same field for non-subgraph purposes and must keep checksummed casing (Section 8).

---

1. Context

Target repository: valory-xyz/trader. Cloned at 3dbc71df89f6bf3cf9f80910c254f46b3e60eb34 — the same commit the report cites. Every line number, helper name and call site below was re-derived from that clone rather than carried over from v1 or v2, because v2 had already had to correct one of v1's (:936 → :937). Two corrections to v2 are folded in, both in Section 3.

Repo-set decision and reasoning. Evidence weighed in order:

1. Explicit repository (decisive). The description names valory-xyz/trader in the TL;DR, root-cause section and reference list, with a concrete path (packages/valory/skills/agent_performance_summary_abci/behaviours.py) and commit.
2. Surface mapping (corroborating). The defect is in prediction-market trading behaviour and in how the agent queries its performance subgraphs — owned by trader.
3. Labels (supporting only). Bug + Polystrat-Omenstrat are consistent with a Polymarket trading agent. Neither Backend nor Frontend is set, so no Pearl repo is implicated by label.

Repos considered and excluded:

• valory-xyz/olas-operate-middleware — the blank values originate in the agent, not the middleware. Not scoped.
• valory-xyz/olas-operate-app — Pearl renders whatever the agent's HTTP server returns. Once the metrics populate, the existing UI renders them; no contract change (Section 5). Not scoped.
• The predict-polymarket subgraph repo — the synced-but-stale Graph-node deployment is a real, separate defect and belongs to its own ticket. It matters here only as the reason the "repoint the URL" shortcut is rejected (Section 7).

Architecture reference used: the repo publishes no .claude/commands/explore-*.md, so per the fallback order this scope is grounded in trader/CLAUDE.md plus the source files read below. That reference defines no bug-section template, so a Reproduction and a Root Cause section are appended after Section 10.

Issue type: Bug.

---

2. Scope Classification

Bug fix — agent skill, subgraph query layer. Backend/agent only.

• No FSM change: no new rounds, payloads, transitions or fsm_specification.yaml edits.
• No new dependencies, no config or service.yaml change, no env-var change.
• No API contract change (Section 5) and no persistence schema change (Section 6).
• Package contents change, so content-addressed hashes must be regenerated (Section 8).

---

3. Data-Flow Trace

Both broken paths originate in FetchPerformanceSummaryBehaviour and terminate at the same squid endpoint.

Path A — Total ROI
calculate_roi (behaviours.py:746) reads self.synchronized_data.safe_contract_address unmodified → calls _fetch_trader_agent at :748 → requests.py:500 → on Polystrat selects self.context.polymarket_agents_subgraph, binding variables={"id": agent_safe_address} at requests.py:508 → _fetch_from_subgraph (requests.py:253), which builds the payload via to_content(query, variables=variables) and forwards variables verbatim → GET_POLYMARKET_TRADER_AGENT_PERFORMANCE_QUERY (queries.py:307), declared query GetPolymarketTraderAgentPerformance($id: String!) { traderAgentById(id: $id) … }. A checksummed $id misses, _unwrap_trader_agent yields None, calculate_roi returns (None, None).

Path B — Prediction accuracy
_get_prediction_accuracy (behaviours.py:930) reads the same field unmodified → calls _fetch_trader_agent_bets at :932 → requests.py:556 → self.context.polymarket_bets_subgraph, binding variables={"id": agent_safe_address} at requests.py:564 → same verbatim forwarding → GET_POLYMARKET_TRADER_AGENT_BETS_QUERY (queries.py:354), whose filter is where: {traderAgent: {id_eq: $id}} — exact equality. Empty result → None.

Correction to v2 (i) — the blast radius is one caller each, not many. Searching the whole packages/ tree for callers of these two helpers returns, outside tests, exactly behaviours.py:748 and behaviours.py:932. Nothing else in the repo calls either. This is what makes the helper-level placement in Section 7 safe.

Correction to v2 (ii) — predictions_helper.py does not call these helpers. v2's change list included predictions_helper.py partly on the strength of a _fetch_trader_agent_bets call at :586. That is a different method on a different class — PredictionsFetcher._fetch_trader_agent_bets(self, safe_address, first, skip) defined at predictions_helper.py:608 — not the requests.py helper, which takes (self, agent_safe_address) and is a generator. Same name, unrelated code path. The file is out of scope in v3 regardless (Section 10), but the conflation should not be inherited.

Both helpers bind the address in both platform branches. _fetch_trader_agent binds it for Polymarket and again for Omen (requests.py:515); _fetch_trader_agent_bets likewise (requests.py:577). One normalisation per helper therefore covers both venues.

Why the endpoint's casing rule differs by platform (verified against skill.yaml):

Context	URL (skill.yaml)	Runtime	id casing
polymarket_agents_subgraph	subgraph.autonolas.tech/squid/predict-polymarket/graphql (:225)	Subsquid	Exact — case-sensitive
polymarket_bets_subgraph	same squid (:240)	Subsquid	Exact — case-sensitive
polymarket_questions_subgraph	same squid (:255)	Subsquid	Exact — case-sensitive
olas_agents_subgraph	api.subgraph.autonolas.tech/api/proxy/predict-omen (:135)	Graph node	ID! normalises
polygon_mech_subgraph	…/api/proxy/marketplace-polygon (:270)	Graph node	ID! normalises
olas_mech_subgraph	…/api/proxy/marketplace-gnosis (:165)	Graph node	ID! normalises

This table is the substance of the bug: identical un-normalised code is correct on Omen and wrong on Polystrat. It is retained because it explains why the Omen branches of the two fixed helpers are unaffected either way — lowercasing is a no-op there today, and insurance if that endpoint ever moves to a squid.

No response-side matching on the address. Nothing in the skill compares a subgraph response back against safe_contract_address (no equality test on that value exists in agent_performance_summary_abci), and _unwrap_trader_agent keys off field names only. So changing the casing of the query input cannot desynchronise any downstream match.

Downstream effect. Both None values become NA metrics in _build_performance_metrics. The round reports failure, _save_agent_performance_summary logs "Preserving existing values for failed metrics", and last_updated stays frozen — matching the reported freeze.

---

4. File-Level Change List

• packages/valory/skills/agent_performance_summary_abci/graph_tooling/requests.py — Modify — lowercase the Safe address on the variables binding in _fetch_trader_agent and _fetch_trader_agent_bets only (both platform branches of each), and note it in each docstring.
• packages/valory/skills/agent_performance_summary_abci/behaviours.py — Modify — reword the misleading warning at :937 only. No other edit to this file; in particular no .lower() is added or removed here.
• packages/valory/skills/agent_performance_summary_abci/tests/graph_tooling/test_requests.py — Modify — extend the two existing test classes with wire-level assertions on the address actually sent (Section 9).
• packages/valory/skills/agent_performance_summary_abci/tests/test_behaviours.py — Modify — cover the corrected warning text; the orphaned SAFE_ADDRESS_LOWER becomes usable as the expected value in the wire assertions.
• packages/valory/skills/agent_performance_summary_abci/skill.yaml — Modify — content hash only, regenerated by tooling (Section 8). No hand-editing.

graph_tooling/queries.py is not changed — the queries are correct; only the values bound to them were wrong. handlers.py, predictions_helper.py and polymarket_predictions_helper.py are not changed (Sections 5 and 10).

---

4b. Existing Code to Reuse

The normalise-at-the-binding pattern is already established in the target file, which makes this a consistency fix rather than a new convention:

• graph_tooling/requests.py:739 — _fetch_daily_profit_statistics — lowercases at the binding ("agentId": agent_safe_address.lower()); the closest precedent to copy verbatim.
• graph_tooling/requests.py:453 — _fetch_ct_held_position_keys — lowercases at the binding ("id": agent_safe_address.lower()) — the exact shape needed here, on the same id variable name.
• graph_tooling/requests.py:816 — _hydrate_profit_participants — derives bettor_id = agent_safe_address.lower() once; its docstring already states "The address is lowercased here so the invariant does not depend on the caller" (:808-809) — reuse that sentence as the docstring note for the two fixed helpers.
• graph_tooling/requests.py:1028 — _fetch_mech_requests_by_titles — normalises sender while deliberately leaving questionTitles untouched in the same variables dict; the working example of why normalisation belongs per-parameter, not per-transport.
• graph_tooling/requests.py:144 — to_content(query, variables) — the single point where variables is serialised onto the wire. Reuse as-is for the test assertions in Section 9; do not add normalisation to it or to _fetch_from_subgraph (Section 7).
• tests/test_behaviours.py:83-84 — SAFE_ADDRESS = "0xSafeAddress" / SAFE_ADDRESS_LOWER = "0xsafeaddress" — the mixed-case/lowercase pair the tests already define. Verified: SAFE_ADDRESS_LOWER is referenced nowhere in packages/ today; it is exactly the fixture this change needs.
• tests/graph_tooling/test_requests.py:572 TestFetchTraderAgent and :749 TestFetchTraderAgentBets — a class already exists for each helper being changed, each with test_polymarket_path / test_omen_path methods, so the new assertions extend existing classes rather than adding scaffolding.

---

5. API Contract Changes

None.

The agent's performance HTTP surface keeps its existing shape. AgentPerformanceSummary.prediction_accuracy stays Optional[float], and the metric entries keep the names "Prediction accuracy" and "Total ROI". The only difference is behavioural: fields that resolved to NA will carry real values.

Because v3 adds normalisation only at two query bindings and removes none of the existing caller-side .lower() calls, no served value changes. In particular the five sites that produce a served or persisted value keep their current behaviour untouched and unexamined: handlers.py:452, :505, :634 (each emits "agent_id": safe_address in a JSON response), handlers.py:797, and behaviours.py:1092 (the lowercased address is the fallback in id=agent_details_raw.get("id", safe_address), so it becomes AgentDetails.id when the subgraph omits id). v2 had to reason about these because it proposed removing them; v3 does not touch them, so response payloads are byte-identical before and after. This is why handlers.py appears in no change list.

Dependency note (not scoped here): Pearl consumes this via the agent's own HTTP server. Because field names, types and nullability are unchanged, no coordinated change is required in olas-operate-app or olas-operate-middleware. Optional/NA remains a legitimate runtime state (genuinely new agent, transient subgraph failure), so existing null-handling on the consumer side must stay.

---

6. Persistence & Schema Updates

No schema change and no migration.

The persisted AgentPerformanceSummary keeps its current fields. Two points worth stating explicitly:

• Self-healing, no backfill. The stale on-disk summary is overwritten on the first successful round after the fix, and last_updated resumes advancing. Nothing needs rewriting by hand.
• profit_over_time was never affected. It is sourced through _fetch_daily_profit_statistics, which already lowercases at the binding (requests.py:739) — so no rebuild of historical profit data is needed. Worth confirming during verification rather than assuming.

---

7. Implementation Approach

What the reviewer asked for. The feedback is "make it lower case only for the input of the queries, the rest should be unchanged. We fix what's broken, and leave everything as is (unless it's a very low hanging fruit)." v3 reads that as three instructions: normalise only values bound into GraphQL query variables; restrict the change to the two paths that are actually broken; and leave every currently-working site alone even where a broader rule would be tidier. The dropped work is enumerated in Section 10 so the reduction is visible as a decision rather than an omission.

Where the .lower() goes — inside the two helpers, on the variables binding. Each of _fetch_trader_agent and _fetch_trader_agent_bets lowercases the address as it is placed into its variables dict, in both the Polymarket and the Omen branch, matching what _fetch_ct_held_position_keys and _fetch_daily_profit_statistics already do a few lines away in the same file. Each helper's docstring gains the one-line note that the address is lower-cased internally, echoing the wording _hydrate_profit_participants already uses.

Three things make this the right placement rather than the caller sites. It is the most literal reading of "only for the input of the queries" — the query input is precisely the variables value, whereas lowercasing the local variable in behaviours.py would also change what the surrounding log lines print. It is provably contained: each helper has exactly one production caller and that caller is the broken site itself (Section 3), so nothing else can be affected. And it puts the rule where the file already keeps it, so the next reader finds one convention in requests.py instead of a new one split across two modules.

The counter-argument is diagnostic, and it is worth stating because it is the honest cost: with the fix in the helper, the warnings at behaviours.py:752 and :937 still log the checksummed address while the query used the lowercased one — a small mismatch of exactly the kind that made this bug hard to trace in the first place. The alternative below removes that mismatch; Section 10 item 1 asks the reviewer to confirm the choice.

Alternative — the two caller-side lines named in the report. Adding .lower() at behaviours.py:746 and :930 is equally small, equally effective, and is literally the fix the report specifies. It also makes the log lines print the address that was actually queried. It is not preferred only because it lowercases a local variable used for more than the query, leaves the helpers' own contract undefined for any future caller, and adds the convention to a second file rather than reinforcing it in the one that already carries it. If the reviewer prefers the report's literal wording, this substitutes cleanly with no other change to this scope.

The misleading warning at behaviours.py:937 — the carve-out, exercised. The line reads "Agent bets data not found for {agent_safe_address=}. Trader may be unstaked." The staking claim is unfounded — the reporter's cli.log shows staking_state=<StakingState.STAKED: 1> — and it actively sent the original investigation toward staking rather than the query. Rewording it to state that no bets were returned for the queried address, citing subgraph unavailability or a genuinely new agent as possible causes and asserting nothing about staking state, is a single string with no behavioural effect, in a file already being opened. That is the "very low hanging fruit" the feedback allows for, and it is the one item in the ticket's own "Also:" list. It is called out separately here so it can be dropped on request without disturbing anything else. The neighbouring warning on the ROI path (:752, "Trader agent data not found or incomplete") is accurate and is left alone.

Alternative — normalise centrally in _fetch_from_subgraph. Rejected, and verified rather than assumed: that function is the shared transport for every query in the skill, and GET_MECH_REQUESTS_BY_TITLES_QUERY passes free-text market titles through the same variables dict. Blanket-lowercasing there would corrupt those titles and break mech-request attribution. _fetch_mech_requests_by_titles already demonstrates the correct granularity (Section 4b).

Alternative — repoint POLYMARKET_AGENTS_SUBGRAPH_URL at the Graph-node deployment. Rejected. It is config-only — polymarket_trader/service.yaml:310 already parameterises the URL as ${POLYMARKET_AGENTS_SUBGRAPH_URL:str:…} — and that deployment does accept checksummed addresses, which is what makes it tempting. But the report's measurements show it returning materially lower bet counts and months-stale lastActive at the same chain head: synced but not current. Adopting it would replace a visibly missing metric with a confidently wrong one, the worse failure mode for a number users read as their agent's performance.

---

8. Constraint Checklist

• Change exactly two variables bindings. Every other address-carrying helper in requests.py stays as it is, including the ones that forward verbatim today and are correct only because their subgraph is Graph-node-backed. That breadth was explicitly declined (Section 10).
• Do not lowercase non-subgraph reads of safe_contract_address. Two sites read the same field and must keep checksummed casing: behaviours.py:1326 (LiFi quote fromAddress) and behaviours.py:1424, consumed as account= on an ERC-20 check_balance contract call at :1441. Neither is a subgraph query. This is the main way to over-apply the fix.
• Do not add or remove any .lower() in behaviours.py. The existing ones at :1092, :1118, :1169, :1506 and :1693 stay exactly as they are; :1092 in particular feeds a served and persisted value (Section 5). The only edit to this file is the warning string at :937.
• Do not touch handlers.py, predictions_helper.py or polymarket_predictions_helper.py. They are correct today; the callers that feed them still lowercase, because those .lower() calls are not being removed.
• Regenerate content-addressed package hashes after touching package contents (autonomy packages lock / make generators) — CI enforces them.
• Copyright headers and # -*- coding: utf-8 -*- preserved on every touched file.
• Sphinx-style docstrings on the two modified helpers, including the :param: note that the address is lower-cased internally — darglint enforces this.
• Black (88 cols), isort, mypy, pylint clean. Run linters via tomte tox, never bare tox (per trader/CLAUDE.md), and never parallelise the autonomy-based environments.
• CI enforces 100% coverage — run coverage for both modified modules (graph_tooling/requests.py and behaviours.py), not just requests.py.
• No FSM spec regeneration expected; if check-abciapp-specs reports a diff, that signals unintended scope creep.

---

9. Test Strategy

The reason this shipped is a testing gap, not a review miss: TestGetPredictionAccuracy patches _fetch_trader_agent_bets wholesale, so the Safe address never reaches a query and no test could observe its casing. The existing TestFetchTraderAgent / TestFetchTraderAgentBets classes do exercise the real helpers, but assert only on the return value, which is identical whatever casing went out. The assertion has to be on what goes onto the wire.

• Wire-level assertions (the load-bearing tests). Both helper classes already mock get_http_response. Capture the content kwarg it receives — built by to_content(query, variables=variables) — decode the JSON, and assert variables["id"] equals SAFE_ADDRESS_LOWER when the helper is invoked with the mixed-case SAFE_ADDRESS. This is the regression gate: it fails on today's code and passes after the fix. Add it to test_polymarket_path and test_omen_path in each class, so the Omen branch is pinned too.
• Scope-containment tests. Assert that at least one deliberately unchanged helper — _fetch_agent_details is the closest neighbour, sharing the same {"id": …} shape — still forwards the address verbatim. This encodes the reviewer's narrow-scope decision as a test, so a later well-meaning "make it uniform" change has to be a conscious edit rather than a silent drift.
• Negative tests — the over-application guard. Assert that the LiFi quote (:1326) and the ERC-20 check_balance call (:1441) still receive the checksummed address.
• Behaviour-level. With a lowercase-keyed fake subgraph, assert _get_prediction_accuracy and calculate_roi return populated values rather than None, and that metrics render numerically instead of NA.
• Warning text. Assert the corrected :937 message, and confirm no existing test asserts on the old "may be unstaked" wording.
• Manual verification. Against a real Polystrat Safe, confirm accuracy and Total ROI populate and last_updated resumes advancing. The report's expected values for services 166 and 32 are usable as external checks — treat them as reported, not as fixtures.

---

10. Unresolved Questions

1. Placement of the two .lower() calls — confirm. This scope puts them on the variables binding inside the two requests.py helpers, as the most literal reading of "only for the input of the queries" and the containable one. The report's own wording specifies the caller sites behaviours.py:746 and :930 instead, which additionally makes the warning logs print the address actually queried. Both are two lines and equally effective; Section 7 sets out the trade-off. Say which you prefer — the swap changes nothing else in this scope.
2. The warning reword at behaviours.py:937 — in or out? Included here as the "very low hanging fruit" the feedback allows: one string, no behavioural effect, and it is the item the ticket itself lists under "Also:". If you read the carve-out more tightly, drop it and the change reduces to requests.py plus its tests.

Superseded — v2 decisions reversed by the 2026-09-04 feedback. Recorded so the reduction in scope is auditable and no reviewer has to diff v2 against v3 to find it. None of these are open questions; they are closed against doing the work:

3. Uniform normalisation across all address-carrying helpers — dropped. v2 proposed normalising _fetch_mech_sender, _fetch_agent_details, _fetch_trader_agent_performance, _fetch_pending_bets and _fetch_all_mech_requests as well. All five are correct today because their subgraph is Graph-node-backed or their caller already lowercases. They are now left untouched. The residual risk is stated plainly: the invariant still depends on convention rather than enforcement, so if another endpoint moves from The Graph to a squid, the same defect can reappear at one of those five sites. That is an accepted consequence of fixing only what is broken, not an oversight.
4. Normalising the two predictions-fetcher classes — dropped. Safe precisely because the caller-side .lower() calls that feed them (behaviours.py:1506, handlers.py:797) are no longer being removed. This item and the next are coupled: dropping one without the other would have been a regression.
5. Removing the caller-side .lower() calls in behaviours.py — dropped. The two overlapping conventions remain visible in the code. Tidiness only; no correctness cost given item 4 is also dropped.
6. agent_id casing in the HTTP responses — moot. v2 raised this because it was removing handlers.py .lower() calls. v3 removes none, so the served payloads are unchanged and there is nothing to decide (Section 5).
7. The synced-but-stale predict-polymarket Graph deployment — out of scope, still unfiled as far as this scope can tell. Independent of this bug and belonging to its own ticket; noted here only so it is not lost.

---

Reproduction

Verified structurally against 3dbc71d rather than by running an agent:

1. Run a Polystrat (Polymarket) agent with a Safe that has resolved bets, so is_running_on_polymarket is true.
2. calculate_roi (:746) and _get_prediction_accuracy (:930) read safe_contract_address in EIP-55 checksummed form and pass it unmodified to their helpers.
3. Both queries resolve against the Subsquid endpoint, which matches id exactly — the checksummed value matches no row, since ids are stored lowercase.
4. Observed: traderAgentById returns null and marketParticipants returns []; prediction_accuracy and Total ROI render as NA; the log warns "Trader may be unstaked."; last_updated freezes at the last good round.
5. Contrast: _fetch_trader_agent_performance issues a comparable query for the same Safe in the same round and succeeds, because its caller (behaviours.py:1118) lowercases — which is why predictions_made populates while the other two do not. Two nearly-identical calls disagreeing within one round is the fingerprint of this bug.

Equivalent on Omen: not reproducible — predict-omen is Graph-node-served and its ID! scalar normalises case, masking the identical defect.

Root Cause

A missing normalisation invariant at the subgraph boundary, exposed by a runtime migration.

_fetch_from_subgraph forwards variables verbatim by design, so responsibility for address casing rests with each _fetch_* helper or its caller. That responsibility was never centralised: some helpers normalise internally, others rely on their callers, and of the callers reading safe_contract_address in behaviours.py, five lowercase and two — the two broken ones — do not. While every Polymarket-facing query was Graph-node-served, the ID! scalar normalised case and the inconsistency had no observable effect. Moving the Polymarket queries onto Subsquid — whose generated traderAgentById(id: String!) and id_eq filters are exact string matches — turned those two un-normalised sites from harmless into user-visible.

The defect is therefore not the two missing .lower() calls in isolation; it is that correctness depended on a convention no layer enforced. Two identical id: Bytes! schemas with opposite case semantics across two runtimes is a migration hazard for anything else moved from The Graph to a squid. v3 fixes the two broken sites as instructed and does not attempt to enforce the invariant more broadly — see Section 10 item 3 for the residual risk that leaves.

🤖 Generated with Claude Code

[valory-coding-agent]

[scope] Section 9 of the scope lists four kinds of test; three landed here (wire-level, containment, over-application guards) but the fourth did not:

Behaviour-level. With a lowercase-keyed fake subgraph, assert _get_prediction_accuracy and calculate_roi return populated values rather than None, and that metrics render numerically instead of NA.

Every existing test for both paths patches the helper wholesale — patch.object(b, "_fetch_trader_agent_bets", …) here, and patch.object(b, "_fetch_trader_agent", …) at the 12 sites in TestCalculateRoi (test_behaviours.py:2180-2327). So nothing in the suite drives calculate_roi / _get_prediction_accuracy through a subgraph keyed on the lowercased address and asserts the metric comes out numeric rather than NA — which is the user-visible symptom OPE-1923 actually reports.

The four *_sends_lowercased_address tests are the regression gate for the casing itself, so this is not a coverage hole in the fix; it is the one Section 9 item that is neither delivered nor listed under "Deliberately not done" in the PR description. Either add it, or say in the description that it was dropped as redundant given the wire-level tests — so the omission is a decision rather than a gap.

[valory-coding-agent]

[question] The containment test is the right idea, but it pins the Polymarket branch — and that branch is the one place where "forwards verbatim" is a latent instance of the very bug this PR fixes.

_fetch_agent_details (requests.py:611-617) sends its Polymarket query to self.context.polymarket_agents_subgraph, i.e. the same case-sensitive Subsquid endpoint as _fetch_trader_agent. It only works today because its single production caller lowercases first (behaviours.py:1093, safe_address = …safe_contract_address.lower()). The scope acknowledges exactly this in Section 10 item 3: the five untouched helpers "are correct today only because their subgraph is Graph-node-backed or their caller lowercases first", and calls the residual risk out explicitly.

Setting is_running_on_polymarket = True here therefore asserts that a checksummed address reaching the squid is the intended contract. If someone later reports blank created_at / last_active_at for a Polystrat agent — the same failure mode, one helper over — the fix has to delete a test whose name and docstring both say verbatim is deliberate.

Flipping this to is_running_on_polymarket = False pins the identical property ("this helper does not normalise; its caller does") against the Graph-node endpoint, where verbatim is genuinely harmless. Was the Polymarket branch chosen on purpose, or just to mirror the two tests above?

[valory-coding-agent]

[question] Answering Section 10 item 1 of the scope, which this PR restates under "Placement note for the reviewer": keep the .lower() inside the two requests.py helpers. That placement matches the convention the file already carries (_fetch_ct_held_position_keys:453, _fetch_daily_profit_statistics:761), it gives each helper a stated contract instead of leaving the invariant to whoever calls it next, and it keeps one rule in one module. Moving it to behaviours.py:746 / :930 would put the convention in a second file and re-open the "correct only by caller convention" gap that Section 10 item 3 already flags as residual risk.

That leaves the diagnostic cost the PR honestly names — this warning still prints the checksummed address while the query went out lowercased. Since the whole reason OPE-1923 took a subgraph round-trip to diagnose is that a log line did not describe what actually went on the wire, it seems worth closing here rather than accepting:

self.context.logger.warning(
    f"No bets returned for {agent_safe_address=} "
    f"(queried as {agent_safe_address.lower()!r}). The agent may not "
    "have placed a bet yet, or the bets subgraph may be unavailable."
)

The existing assertions still hold (SAFE_ADDRESS in warning, "No bets returned" in warning, "unstaked" not in warning), so test_none_agent_bets_warning_makes_no_staking_claim needs no change. Your call on the exact wording — the point is that the reader can see both forms.

[valory-coding-agent]

[nit] This pulls the whole of tests/test_behaviours.py (310 KB, ~3.5k lines) into test_requests.py for two string constants, so pytest tests/graph_tooling/test_requests.py now imports and collects nothing less than the entire behaviours suite's module body as a side effect.

Cross-test-module imports do have precedent here (trader_abci/tests/test_handlers.py:60), so this is not a convention break. But the repo's usual home for shared test fixtures is a package-local conftest.py — see market_manager_abci/tests/conftest.py (raw_bet) and decision_maker_abci/tests/conftest.py (profile_name). agent_performance_summary_abci/tests/ has none yet; moving SAFE_ADDRESS / SAFE_ADDRESS_LOWER into one (re-exported from test_behaviours.py so its 187 existing uses keep working) would keep the two test modules independent.

Not worth blocking on — flagging it while the file is open.

[valory-coding-agent]

Code Review — OPE-1923

Scope: Polystrat: prediction accuracy shows null despite resolved predictions
Files reviewed: 14 of 14
Comments: 4 ([security]: 0, [bug]: 0, [scope]: 1, [convention]: 0, [suggestion]: 0, [question]: 2, [human-input]: 0, [nit]: 1)

Key findings

• The fix itself is correct and correctly bounded. Four .lower() calls on the variables bindings inside _fetch_trader_agent (requests.py:519, :526) and _fetch_trader_agent_bets (:586, :601) — both platform branches of each, as Section 3 requires. Both helpers have exactly one production caller each (behaviours.py:748, :932), confirmed by grepping packages/ outside tests, so the blast radius is the two broken sites and nothing else. Neither helper's return value is compared back against safe_contract_address anywhere in the skill, so changing the query input cannot desynchronise a downstream match.
• Section 8's hard constraints hold. The two non-subgraph reads are untouched and now pinned: _get_pol_to_usdc_rate still builds fromAddress=/toAddress= from the checksummed value (behaviours.py:1341-1342), and _fetch_available_funds still passes account=safe_contract_address to the ERC-20 check_balance call (:1441). No .lower() was added or removed in behaviours.py; handlers.py, predictions_helper.py and polymarket_predictions_helper.py are not in the diff, so served payloads are byte-identical.
• The Omen branches are a genuine no-op, not a silent behaviour change. GET_TRADER_AGENT_QUERY and GET_TRADER_AGENT_BETS_QUERY both go to olas_agents_subgraph, the Graph-node-backed predict-omen proxy whose ID! scalar normalises case — the ticket verified both casings return the same traderAgent. Lowercasing there costs nothing today and is insurance if that endpoint ever moves to a squid.
• Test strategy is three-quarters delivered. The four *_sends_lowercased_address tests assert on the JSON actually handed to get_http_response, which is the right seam — the reason this shipped is that TestGetPredictionAccuracy and all 12 TestCalculateRoi cases patch the helpers wholesale, so no test could ever observe the casing. Section 9's behaviour-level test (fake subgraph → metric renders numerically rather than NA) is the one item neither delivered nor listed under "Deliberately not done" — see the [scope] comment.
• One placement question answered, one test-design question raised. Section 10 item 1 (helpers vs. caller sites) is answered in favour of the current placement, with a concrete suggestion for the log/query mismatch the PR honestly flags. Separately, the containment test pins the Polymarket squid branch of _fetch_agent_details as "forwards verbatim" — the branch where verbatim is a latent instance of this same bug, held safe only by its caller. Pinning the Omen branch would assert the same property without that hazard.
• Hash regeneration is mechanically consistent. agent_performance_summary_abci → chatui_abci / decision_maker_abci / trader_abci → check_stop_trading_abci / tx_settlement_multiplexer_abci → agent/trader → both service.yaml files: the dependency closure is complete, and CI's check-hash, check-packages, check-doc-hashes, analyse-service, lock_check (all three OSes), linter_checks and copyright_and_dependencies_check are all green on 6db3c15. No FSM spec diff, as the scope predicted.

Scope coverage

• ✅ graph_tooling/requests.py — modified; .lower() on both bindings in both helpers, docstring note on each (:504-513, :570-580)
• ✅ behaviours.py — modified; the :937 warning reworded, nothing else in the file touched
• ✅ tests/graph_tooling/test_requests.py — modified; 4 wire-level assertions + 1 containment test
• ✅ tests/test_behaviours.py — modified; corrected warning text + 2 over-application guards
• ✅ skill.yaml — regenerated
• ✅ packages.json, aea-config.yaml, both service.yamls, 5 dependent skill.yamls — mechanical hash propagation, correctly outside Section 4's list

Confirmed unchanged, per Section 8: graph_tooling/queries.py, handlers.py, predictions_helper.py, polymarket_predictions_helper.py, and the five helpers Section 10 item 3 deliberately left alone (_fetch_mech_sender, _fetch_agent_details, _fetch_trader_agent_performance, _fetch_pending_bets, _fetch_all_mech_requests).

Not reviewed

• The test matrix (15 jobs, 3.10–3.14 × 3 OSes) was still in progress at review time; per review policy the suite was not run locally. Coverage against both modified modules is CI's gate.
• Step 4 of "How to Test" (live Polystrat Safe → accuracy and Total ROI populate, last_updated resumes) needs a human with a funded agent; it cannot be verified from the diff.

---

🤖 Coding Agent [Beta] · Automated review against OPE-1923