Expose combo execution contract#57
Conversation
Co-Authored-By: Codex <noreply@openai.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 302bed9037
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| for symbol, payload in dict(trend_metadata.get("rotation_candidates", {})).items() | ||
| } | ||
| runtime_trend_universe = {symbol: {"base_asset": symbol[:-4]} for symbol in universe_snapshot} |
There was a problem hiding this comment.
When universe_snapshot contains symbols that need normalization, such as lowercase or padded values, the trend leg resolves candidates with normalized uppercase keys, but this new execution map keeps the raw symbols and passes them to plan_trend_buys. That helper indexes prices[symbol] and checks selected_candidates.get(symbol), so valid candidates can either be skipped or raise a KeyError even though the rotation resolver accepted them; normalize these symbols or reuse the resolved trend pool before building the buy plan.
Useful? React with 👍 / 👎.
| state, | ||
| symbol, | ||
| curr_price, | ||
| indicators_map.get(symbol), |
There was a problem hiding this comment.
When combo state marks a trend symbol as holding and the platform supplies the indicator set accepted by selection here (SMA/ROC/vol but no atr14, as in the new combo fixture), this new sell-reason pass calls get_trend_sell_reason, whose ATR stop path unconditionally reads indicators["atr14"]. That raises KeyError before returning any StrategyDecision, so either require/populate atr14 for combo execution or skip/default the ATR stop when it is missing.
Useful? React with 👍 / 👎.
| indicators_map = _require_market_data(ctx, "derived_indicators") | ||
| benchmark_snapshot = _require_market_data(ctx, "benchmark_snapshot") | ||
| portfolio = _resolve_portfolio_snapshot(ctx) | ||
| account_metrics = _resolve_account_metrics(ctx) |
There was a problem hiding this comment.
When the combo entrypoint receives a mapping-style portfolio_snapshot, which the underlying crypto_equity_combo.build_target_weights path explicitly supports, this new call routes through _resolve_account_metrics; that helper only falls back through object attributes like snapshot.total_equity when no embedded account_metrics exists. Such callers now raise before a StrategyDecision is returned, so handle Mapping snapshots in the account-metrics path or avoid requiring it for the combo budget diagnostics.
Useful? React with 👍 / 👎.
1 participant
Summary\n- expose Binance-compatible budgets for crypto_equity_combo\n- include rotation diagnostics used by Binance execution mapping\n- add regression coverage for combo execution contract\n\n## Validation\n- PYTHONPATH=src:/Users/lisiyi/Projects/QuantPlatformKit/src python3 -m pytest -q tests\n- python3 -m ruff check .\n- BinancePlatform local dry-run replay with local CryptoStrategies source for synthetic $102/$500/$1000/$2000