From d972c078bc6da179a2ac50bafb85d2bf6c660a96 Mon Sep 17 00:00:00 2001 From: keitaj Date: Mon, 4 May 2026 11:44:51 +0900 Subject: [PATCH] fix: promote _STRATEGY_PARAMS / _COMMON_PARAMS to module level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #149 promoted ``_RISK_PARAMS`` to module scope so ``validation.strategy_validator.known_market_making_keys`` could introspect risk-guardrail names lazily. The other two lists (``_STRATEGY_PARAMS``, ``_COMMON_PARAMS``) were left as locals inside ``main()``, so the lazy import in ``known_market_making_keys`` would raise ``ImportError`` at runtime as soon as the JSON config typo detector tried to enumerate known MM keys. This PR moves both lists out of ``main()`` to module scope right after ``_RISK_PARAMS``. ``main()`` keeps a short comment pointing readers at the new location; the surrounding ``_collect_params`` / ``argparse.Namespace`` call-sites are unchanged. A regression test class is added to ``test_strategy_validator.py`` — it pins the contract that all three name lists remain at module level (any future demotion will fail the import-style assertions). --- bot.py | 229 ++++++++++++++++--------------- tests/test_strategy_validator.py | 61 +++++++- 2 files changed, 181 insertions(+), 109 deletions(-) diff --git a/bot.py b/bot.py index 3fe3f2a..50cd0d8 100644 --- a/bot.py +++ b/bot.py @@ -52,6 +52,121 @@ 'max_open_positions', 'cooldown_after_stop', ] +# Common-strategy parameter names (apply to most / all strategies). +# Module-level so ``validation.strategy_validator.known_market_making_keys()`` +# can introspect them for typo detection without circular-import gymnastics. +_COMMON_PARAMS = [ + 'position_size_usd', 'max_positions', 'take_profit_percent', + 'stop_loss_percent', 'candle_interval', 'account_cap_pct', +] + +# Per-strategy parameter names. Each list contains the ``config_key`` +# (i.e. the name produced by argparse ``dest=``); for renames where the +# CLI flag and config key differ, ``_collect_params`` accepts an +# ``(arg_name, config_key)`` tuple. Module-level for the same reason as +# ``_COMMON_PARAMS`` above. +_STRATEGY_PARAMS = { + 'simple_ma': ['fast_ma_period', 'slow_ma_period'], + 'rsi': [ + 'rsi_period', 'oversold_threshold', 'overbought_threshold', + 'rsi_extreme_low', 'rsi_moderate_low', + 'size_multiplier_extreme', 'size_multiplier_moderate', + ], + 'bollinger_bands': [ + 'bb_period', 'std_dev', 'squeeze_threshold', + 'volatility_expansion_threshold', + 'high_band_width_threshold', 'high_band_width_multiplier', + 'low_band_width_threshold', 'low_band_width_multiplier', + ], + 'macd': [ + 'fast_ema', 'slow_ema', 'signal_ema', 'divergence_lookback', + 'histogram_strength_high', 'histogram_strength_low', + 'histogram_multiplier_high', 'histogram_multiplier_low', + ], + 'grid_trading': [ + 'grid_levels', 'grid_spacing_pct', 'position_size_per_grid', + 'range_period', 'range_pct_threshold', 'volatility_threshold', + 'grid_recalc_bars', 'grid_saturation_threshold', + 'grid_boundary_margin_low', 'grid_boundary_margin_high', + ], + 'breakout': [ + 'lookback_period', 'volume_multiplier', + 'breakout_confirmation_bars', 'atr_period', + 'pivot_window', 'avg_volume_lookback', + 'stop_loss_atr_multiplier', 'position_stop_loss_atr_multiplier', + 'strong_breakout_multiplier', + 'high_atr_threshold', 'low_atr_threshold', + 'high_atr_multiplier', 'low_atr_multiplier', + ], + 'market_making': [ + 'spread_bps', 'order_size_usd', 'max_open_orders', + 'refresh_interval_seconds', + 'refresh_tolerance_bp', + 'refresh_max_age_seconds', + 'max_position_age_seconds', + 'taker_fallback_age_seconds', + 'aggressive_loss_bps', + 'force_close_max_loss_bps', + 'close_spread_bps', + 'close_breakeven_pct', + 'close_aggressive_pct', + 'unrealized_loss_close_bps', + 'bbo_mode', + 'bbo_offset_bps', + 'inventory_skew_bps', + 'imbalance_threshold', + 'loss_streak_limit', + 'loss_streak_cooldown', + 'bbo_guard_threshold_bps', + 'imbalance_guard_threshold', + 'imbalance_guard_depth', + 'vol_adjust_enabled', + 'vol_adjust_multiplier', + 'vol_lookback', + 'vol_adjust_max_offset', + 'adverse_selection_log_interval', + 'coin_offset_overrides', + 'coin_spread_overrides', + 'coin_size_overrides', + 'coin_unrealized_loss_overrides', + 'close_refresh_threshold_bps', + 'spread_schedule', + 'quiet_hours_utc', + 'quiet_hours_spread_multiplier', + 'microprice_skew_enabled', + 'microprice_skew_multiplier', + 'microprice_max_skew_bps', + 'velocity_guard_enabled', + 'velocity_consecutive', + 'velocity_min_move_bps', + 'dynamic_offset_enabled', + 'dynamic_offset_sensitivity', + 'dynamic_offset_tighten_rate', + 'dynamic_offset_max_addition', + 'dynamic_offset_max_reduction', + 'dynamic_offset_floor', + 'dynamic_offset_min_fills', + 'dynamic_age_enabled', + 'dynamic_age_baseline_vol', + 'dynamic_age_min', + 'dynamic_age_max', + 'auto_exclude_enabled', + 'auto_exclude_threshold_bps', + 'auto_exclude_consecutive', + 'auto_exclude_min_fills', + 'auto_exclude_cooldown', + 'auto_exclude_window_label', + 'forager_enabled', + 'forager_score_threshold', + 'forager_consecutive', + 'forager_cooldown_seconds', + 'forager_weight_activity', + 'forager_weight_quality', + 'forager_weight_cost', + 'drain_flag_file', + ], +} + def _apply_json_risk_overrides(json_overrides: Optional[Dict], args) -> None: """Wire risk parameters from JSON config into the ``Config`` class. @@ -1270,114 +1385,12 @@ def stop(self): args = parser.parse_args() - # Build strategy config from command line arguments. - # Each list maps CLI arg names to config keys (same name when identical). - # For renamed keys use a (arg_name, config_key) tuple. - _COMMON_PARAMS = [ - 'position_size_usd', 'max_positions', 'take_profit_percent', - 'stop_loss_percent', 'candle_interval', 'account_cap_pct', - ] - _STRATEGY_PARAMS = { - 'simple_ma': ['fast_ma_period', 'slow_ma_period'], - 'rsi': [ - 'rsi_period', 'oversold_threshold', 'overbought_threshold', - 'rsi_extreme_low', 'rsi_moderate_low', - 'size_multiplier_extreme', 'size_multiplier_moderate', - ], - 'bollinger_bands': [ - 'bb_period', 'std_dev', 'squeeze_threshold', - 'volatility_expansion_threshold', - 'high_band_width_threshold', 'high_band_width_multiplier', - 'low_band_width_threshold', 'low_band_width_multiplier', - ], - 'macd': [ - 'fast_ema', 'slow_ema', 'signal_ema', 'divergence_lookback', - 'histogram_strength_high', 'histogram_strength_low', - 'histogram_multiplier_high', 'histogram_multiplier_low', - ], - 'grid_trading': [ - 'grid_levels', 'grid_spacing_pct', 'position_size_per_grid', - 'range_period', 'range_pct_threshold', 'volatility_threshold', - 'grid_recalc_bars', 'grid_saturation_threshold', - 'grid_boundary_margin_low', 'grid_boundary_margin_high', - ], - 'breakout': [ - 'lookback_period', 'volume_multiplier', - 'breakout_confirmation_bars', 'atr_period', - 'pivot_window', 'avg_volume_lookback', - 'stop_loss_atr_multiplier', 'position_stop_loss_atr_multiplier', - 'strong_breakout_multiplier', - 'high_atr_threshold', 'low_atr_threshold', - 'high_atr_multiplier', 'low_atr_multiplier', - ], - 'market_making': [ - 'spread_bps', 'order_size_usd', 'max_open_orders', - 'refresh_interval_seconds', - 'refresh_tolerance_bp', - 'refresh_max_age_seconds', - 'max_position_age_seconds', - 'taker_fallback_age_seconds', - 'aggressive_loss_bps', - 'force_close_max_loss_bps', - 'close_spread_bps', - 'close_breakeven_pct', - 'close_aggressive_pct', - 'unrealized_loss_close_bps', - 'bbo_mode', - 'bbo_offset_bps', - 'inventory_skew_bps', - 'imbalance_threshold', - 'loss_streak_limit', - 'loss_streak_cooldown', - 'bbo_guard_threshold_bps', - 'imbalance_guard_threshold', - 'imbalance_guard_depth', - 'vol_adjust_enabled', - 'vol_adjust_multiplier', - 'vol_lookback', - 'vol_adjust_max_offset', - 'adverse_selection_log_interval', - 'coin_offset_overrides', - 'coin_spread_overrides', - 'coin_size_overrides', - 'coin_unrealized_loss_overrides', - 'close_refresh_threshold_bps', - 'spread_schedule', - 'quiet_hours_utc', - 'quiet_hours_spread_multiplier', - 'microprice_skew_enabled', - 'microprice_skew_multiplier', - 'microprice_max_skew_bps', - 'velocity_guard_enabled', - 'velocity_consecutive', - 'velocity_min_move_bps', - 'dynamic_offset_enabled', - 'dynamic_offset_sensitivity', - 'dynamic_offset_tighten_rate', - 'dynamic_offset_max_addition', - 'dynamic_offset_max_reduction', - 'dynamic_offset_floor', - 'dynamic_offset_min_fills', - 'dynamic_age_enabled', - 'dynamic_age_baseline_vol', - 'dynamic_age_min', - 'dynamic_age_max', - 'auto_exclude_enabled', - 'auto_exclude_threshold_bps', - 'auto_exclude_consecutive', - 'auto_exclude_min_fills', - 'auto_exclude_cooldown', - 'auto_exclude_window_label', - 'forager_enabled', - 'forager_score_threshold', - 'forager_consecutive', - 'forager_cooldown_seconds', - 'forager_weight_activity', - 'forager_weight_quality', - 'forager_weight_cost', - 'drain_flag_file', - ], - } + # ``_COMMON_PARAMS`` and ``_STRATEGY_PARAMS`` are defined at module + # level (top of this file) so the JSON config layer's typo detector + # (``validation.strategy_validator.known_market_making_keys``) can + # introspect them without circular-import gymnastics. The local + # references below preserve the existing call-site shape of + # ``_collect_params``. def _collect_params(params, source, dest): """Copy non-None CLI args into *dest* dict.""" diff --git a/tests/test_strategy_validator.py b/tests/test_strategy_validator.py index 66d687d..7cb3105 100644 --- a/tests/test_strategy_validator.py +++ b/tests/test_strategy_validator.py @@ -1,6 +1,10 @@ """Tests for strategy parameter validation.""" -from validation.strategy_validator import validate_strategy_config, VALID_CANDLE_INTERVALS +from validation.strategy_validator import ( + validate_strategy_config, + known_market_making_keys, + VALID_CANDLE_INTERVALS, +) class TestCommonValidation: @@ -239,3 +243,58 @@ def test_unknown_strategy_valid_common(self): assert validate_strategy_config('unknown_strategy', { 'position_size_usd': 100, }) is None + + +class TestKnownMarketMakingKeys: + """Regression tests for ``known_market_making_keys``. + + The function imports ``_STRATEGY_PARAMS``, ``_COMMON_PARAMS``, and + ``_RISK_PARAMS`` lazily from ``bot``. They must all be **module + level** in ``bot.py`` — if any are demoted back inside ``main()``, + the JSON config loader's typo detector will crash at runtime when a + user invokes ``--config``. These tests pin that contract. + """ + + def test_returns_non_empty_set(self): + keys = known_market_making_keys() + assert isinstance(keys, set) + assert len(keys) > 0 + + def test_includes_market_making_params(self): + """Spot-check a handful of representative MM keys.""" + keys = known_market_making_keys() + for k in ('spread_bps', 'order_size_usd', 'refresh_interval_seconds', + 'forager_enabled', 'dynamic_age_enabled'): + assert k in keys, f'{k} missing from known_market_making_keys' + + def test_includes_common_params(self): + keys = known_market_making_keys() + for k in ('position_size_usd', 'max_positions', 'candle_interval'): + assert k in keys, f'common param {k} missing' + + def test_includes_risk_params(self): + """Risk-guardrail keys must be recognised so JSON ``risk:`` blocks + don't trigger spurious typo warnings.""" + keys = known_market_making_keys() + for k in ('max_position_pct', 'daily_loss_limit', 'per_trade_stop_loss'): + assert k in keys, f'risk param {k} missing' + + def test_includes_derived_runtime_keys(self): + """Keys read via ``config.get`` in MarketMakingStrategy but not in + ``_STRATEGY_PARAMS`` (e.g. ``maker_only``, ``close_immediately``). + """ + keys = known_market_making_keys() + for k in ('maker_only', 'close_immediately', 'enable_ws'): + assert k in keys + + def test_bot_module_exports_param_lists(self): + """Direct import — pins that the three lists are at module scope. + + If ``_STRATEGY_PARAMS`` or ``_COMMON_PARAMS`` ever get demoted + back inside ``main()``, this import will raise ``ImportError``. + """ + from bot import _STRATEGY_PARAMS, _COMMON_PARAMS, _RISK_PARAMS + assert isinstance(_STRATEGY_PARAMS, dict) + assert 'market_making' in _STRATEGY_PARAMS + assert isinstance(_COMMON_PARAMS, list) + assert isinstance(_RISK_PARAMS, list)