Skip to content

Commit e47d2ae

Browse files
keitajkeitaj
authored
fix: promote _STRATEGY_PARAMS / _COMMON_PARAMS to module level (#150)
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).
1 parent 2edad25 commit e47d2ae

2 files changed

Lines changed: 181 additions & 109 deletions

File tree

‎bot.py‎

Lines changed: 121 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,121 @@
5252
'max_open_positions', 'cooldown_after_stop',
5353
]
5454

55+
# Common-strategy parameter names (apply to most / all strategies).
56+
# Module-level so ``validation.strategy_validator.known_market_making_keys()``
57+
# can introspect them for typo detection without circular-import gymnastics.
58+
_COMMON_PARAMS = [
59+
'position_size_usd', 'max_positions', 'take_profit_percent',
60+
'stop_loss_percent', 'candle_interval', 'account_cap_pct',
61+
]
62+
63+
# Per-strategy parameter names. Each list contains the ``config_key``
64+
# (i.e. the name produced by argparse ``dest=``); for renames where the
65+
# CLI flag and config key differ, ``_collect_params`` accepts an
66+
# ``(arg_name, config_key)`` tuple. Module-level for the same reason as
67+
# ``_COMMON_PARAMS`` above.
68+
_STRATEGY_PARAMS = {
69+
'simple_ma': ['fast_ma_period', 'slow_ma_period'],
70+
'rsi': [
71+
'rsi_period', 'oversold_threshold', 'overbought_threshold',
72+
'rsi_extreme_low', 'rsi_moderate_low',
73+
'size_multiplier_extreme', 'size_multiplier_moderate',
74+
],
75+
'bollinger_bands': [
76+
'bb_period', 'std_dev', 'squeeze_threshold',
77+
'volatility_expansion_threshold',
78+
'high_band_width_threshold', 'high_band_width_multiplier',
79+
'low_band_width_threshold', 'low_band_width_multiplier',
80+
],
81+
'macd': [
82+
'fast_ema', 'slow_ema', 'signal_ema', 'divergence_lookback',
83+
'histogram_strength_high', 'histogram_strength_low',
84+
'histogram_multiplier_high', 'histogram_multiplier_low',
85+
],
86+
'grid_trading': [
87+
'grid_levels', 'grid_spacing_pct', 'position_size_per_grid',
88+
'range_period', 'range_pct_threshold', 'volatility_threshold',
89+
'grid_recalc_bars', 'grid_saturation_threshold',
90+
'grid_boundary_margin_low', 'grid_boundary_margin_high',
91+
],
92+
'breakout': [
93+
'lookback_period', 'volume_multiplier',
94+
'breakout_confirmation_bars', 'atr_period',
95+
'pivot_window', 'avg_volume_lookback',
96+
'stop_loss_atr_multiplier', 'position_stop_loss_atr_multiplier',
97+
'strong_breakout_multiplier',
98+
'high_atr_threshold', 'low_atr_threshold',
99+
'high_atr_multiplier', 'low_atr_multiplier',
100+
],
101+
'market_making': [
102+
'spread_bps', 'order_size_usd', 'max_open_orders',
103+
'refresh_interval_seconds',
104+
'refresh_tolerance_bp',
105+
'refresh_max_age_seconds',
106+
'max_position_age_seconds',
107+
'taker_fallback_age_seconds',
108+
'aggressive_loss_bps',
109+
'force_close_max_loss_bps',
110+
'close_spread_bps',
111+
'close_breakeven_pct',
112+
'close_aggressive_pct',
113+
'unrealized_loss_close_bps',
114+
'bbo_mode',
115+
'bbo_offset_bps',
116+
'inventory_skew_bps',
117+
'imbalance_threshold',
118+
'loss_streak_limit',
119+
'loss_streak_cooldown',
120+
'bbo_guard_threshold_bps',
121+
'imbalance_guard_threshold',
122+
'imbalance_guard_depth',
123+
'vol_adjust_enabled',
124+
'vol_adjust_multiplier',
125+
'vol_lookback',
126+
'vol_adjust_max_offset',
127+
'adverse_selection_log_interval',
128+
'coin_offset_overrides',
129+
'coin_spread_overrides',
130+
'coin_size_overrides',
131+
'coin_unrealized_loss_overrides',
132+
'close_refresh_threshold_bps',
133+
'spread_schedule',
134+
'quiet_hours_utc',
135+
'quiet_hours_spread_multiplier',
136+
'microprice_skew_enabled',
137+
'microprice_skew_multiplier',
138+
'microprice_max_skew_bps',
139+
'velocity_guard_enabled',
140+
'velocity_consecutive',
141+
'velocity_min_move_bps',
142+
'dynamic_offset_enabled',
143+
'dynamic_offset_sensitivity',
144+
'dynamic_offset_tighten_rate',
145+
'dynamic_offset_max_addition',
146+
'dynamic_offset_max_reduction',
147+
'dynamic_offset_floor',
148+
'dynamic_offset_min_fills',
149+
'dynamic_age_enabled',
150+
'dynamic_age_baseline_vol',
151+
'dynamic_age_min',
152+
'dynamic_age_max',
153+
'auto_exclude_enabled',
154+
'auto_exclude_threshold_bps',
155+
'auto_exclude_consecutive',
156+
'auto_exclude_min_fills',
157+
'auto_exclude_cooldown',
158+
'auto_exclude_window_label',
159+
'forager_enabled',
160+
'forager_score_threshold',
161+
'forager_consecutive',
162+
'forager_cooldown_seconds',
163+
'forager_weight_activity',
164+
'forager_weight_quality',
165+
'forager_weight_cost',
166+
'drain_flag_file',
167+
],
168+
}
169+
55170

56171
def _apply_json_risk_overrides(json_overrides: Optional[Dict], args) -> None:
57172
"""Wire risk parameters from JSON config into the ``Config`` class.
@@ -1270,114 +1385,12 @@ def stop(self):
12701385

12711386
args = parser.parse_args()
12721387

1273-
# Build strategy config from command line arguments.
1274-
# Each list maps CLI arg names to config keys (same name when identical).
1275-
# For renamed keys use a (arg_name, config_key) tuple.
1276-
_COMMON_PARAMS = [
1277-
'position_size_usd', 'max_positions', 'take_profit_percent',
1278-
'stop_loss_percent', 'candle_interval', 'account_cap_pct',
1279-
]
1280-
_STRATEGY_PARAMS = {
1281-
'simple_ma': ['fast_ma_period', 'slow_ma_period'],
1282-
'rsi': [
1283-
'rsi_period', 'oversold_threshold', 'overbought_threshold',
1284-
'rsi_extreme_low', 'rsi_moderate_low',
1285-
'size_multiplier_extreme', 'size_multiplier_moderate',
1286-
],
1287-
'bollinger_bands': [
1288-
'bb_period', 'std_dev', 'squeeze_threshold',
1289-
'volatility_expansion_threshold',
1290-
'high_band_width_threshold', 'high_band_width_multiplier',
1291-
'low_band_width_threshold', 'low_band_width_multiplier',
1292-
],
1293-
'macd': [
1294-
'fast_ema', 'slow_ema', 'signal_ema', 'divergence_lookback',
1295-
'histogram_strength_high', 'histogram_strength_low',
1296-
'histogram_multiplier_high', 'histogram_multiplier_low',
1297-
],
1298-
'grid_trading': [
1299-
'grid_levels', 'grid_spacing_pct', 'position_size_per_grid',
1300-
'range_period', 'range_pct_threshold', 'volatility_threshold',
1301-
'grid_recalc_bars', 'grid_saturation_threshold',
1302-
'grid_boundary_margin_low', 'grid_boundary_margin_high',
1303-
],
1304-
'breakout': [
1305-
'lookback_period', 'volume_multiplier',
1306-
'breakout_confirmation_bars', 'atr_period',
1307-
'pivot_window', 'avg_volume_lookback',
1308-
'stop_loss_atr_multiplier', 'position_stop_loss_atr_multiplier',
1309-
'strong_breakout_multiplier',
1310-
'high_atr_threshold', 'low_atr_threshold',
1311-
'high_atr_multiplier', 'low_atr_multiplier',
1312-
],
1313-
'market_making': [
1314-
'spread_bps', 'order_size_usd', 'max_open_orders',
1315-
'refresh_interval_seconds',
1316-
'refresh_tolerance_bp',
1317-
'refresh_max_age_seconds',
1318-
'max_position_age_seconds',
1319-
'taker_fallback_age_seconds',
1320-
'aggressive_loss_bps',
1321-
'force_close_max_loss_bps',
1322-
'close_spread_bps',
1323-
'close_breakeven_pct',
1324-
'close_aggressive_pct',
1325-
'unrealized_loss_close_bps',
1326-
'bbo_mode',
1327-
'bbo_offset_bps',
1328-
'inventory_skew_bps',
1329-
'imbalance_threshold',
1330-
'loss_streak_limit',
1331-
'loss_streak_cooldown',
1332-
'bbo_guard_threshold_bps',
1333-
'imbalance_guard_threshold',
1334-
'imbalance_guard_depth',
1335-
'vol_adjust_enabled',
1336-
'vol_adjust_multiplier',
1337-
'vol_lookback',
1338-
'vol_adjust_max_offset',
1339-
'adverse_selection_log_interval',
1340-
'coin_offset_overrides',
1341-
'coin_spread_overrides',
1342-
'coin_size_overrides',
1343-
'coin_unrealized_loss_overrides',
1344-
'close_refresh_threshold_bps',
1345-
'spread_schedule',
1346-
'quiet_hours_utc',
1347-
'quiet_hours_spread_multiplier',
1348-
'microprice_skew_enabled',
1349-
'microprice_skew_multiplier',
1350-
'microprice_max_skew_bps',
1351-
'velocity_guard_enabled',
1352-
'velocity_consecutive',
1353-
'velocity_min_move_bps',
1354-
'dynamic_offset_enabled',
1355-
'dynamic_offset_sensitivity',
1356-
'dynamic_offset_tighten_rate',
1357-
'dynamic_offset_max_addition',
1358-
'dynamic_offset_max_reduction',
1359-
'dynamic_offset_floor',
1360-
'dynamic_offset_min_fills',
1361-
'dynamic_age_enabled',
1362-
'dynamic_age_baseline_vol',
1363-
'dynamic_age_min',
1364-
'dynamic_age_max',
1365-
'auto_exclude_enabled',
1366-
'auto_exclude_threshold_bps',
1367-
'auto_exclude_consecutive',
1368-
'auto_exclude_min_fills',
1369-
'auto_exclude_cooldown',
1370-
'auto_exclude_window_label',
1371-
'forager_enabled',
1372-
'forager_score_threshold',
1373-
'forager_consecutive',
1374-
'forager_cooldown_seconds',
1375-
'forager_weight_activity',
1376-
'forager_weight_quality',
1377-
'forager_weight_cost',
1378-
'drain_flag_file',
1379-
],
1380-
}
1388+
# ``_COMMON_PARAMS`` and ``_STRATEGY_PARAMS`` are defined at module
1389+
# level (top of this file) so the JSON config layer's typo detector
1390+
# (``validation.strategy_validator.known_market_making_keys``) can
1391+
# introspect them without circular-import gymnastics. The local
1392+
# references below preserve the existing call-site shape of
1393+
# ``_collect_params``.
13811394

13821395
def _collect_params(params, source, dest):
13831396
"""Copy non-None CLI args into *dest* dict."""

‎tests/test_strategy_validator.py‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
"""Tests for strategy parameter validation."""
22

3-
from validation.strategy_validator import validate_strategy_config, VALID_CANDLE_INTERVALS
3+
from validation.strategy_validator import (
4+
validate_strategy_config,
5+
known_market_making_keys,
6+
VALID_CANDLE_INTERVALS,
7+
)
48

59

610
class TestCommonValidation:
@@ -239,3 +243,58 @@ def test_unknown_strategy_valid_common(self):
239243
assert validate_strategy_config('unknown_strategy', {
240244
'position_size_usd': 100,
241245
}) is None
246+
247+
248+
class TestKnownMarketMakingKeys:
249+
"""Regression tests for ``known_market_making_keys``.
250+
251+
The function imports ``_STRATEGY_PARAMS``, ``_COMMON_PARAMS``, and
252+
``_RISK_PARAMS`` lazily from ``bot``. They must all be **module
253+
level** in ``bot.py`` — if any are demoted back inside ``main()``,
254+
the JSON config loader's typo detector will crash at runtime when a
255+
user invokes ``--config``. These tests pin that contract.
256+
"""
257+
258+
def test_returns_non_empty_set(self):
259+
keys = known_market_making_keys()
260+
assert isinstance(keys, set)
261+
assert len(keys) > 0
262+
263+
def test_includes_market_making_params(self):
264+
"""Spot-check a handful of representative MM keys."""
265+
keys = known_market_making_keys()
266+
for k in ('spread_bps', 'order_size_usd', 'refresh_interval_seconds',
267+
'forager_enabled', 'dynamic_age_enabled'):
268+
assert k in keys, f'{k} missing from known_market_making_keys'
269+
270+
def test_includes_common_params(self):
271+
keys = known_market_making_keys()
272+
for k in ('position_size_usd', 'max_positions', 'candle_interval'):
273+
assert k in keys, f'common param {k} missing'
274+
275+
def test_includes_risk_params(self):
276+
"""Risk-guardrail keys must be recognised so JSON ``risk:`` blocks
277+
don't trigger spurious typo warnings."""
278+
keys = known_market_making_keys()
279+
for k in ('max_position_pct', 'daily_loss_limit', 'per_trade_stop_loss'):
280+
assert k in keys, f'risk param {k} missing'
281+
282+
def test_includes_derived_runtime_keys(self):
283+
"""Keys read via ``config.get`` in MarketMakingStrategy but not in
284+
``_STRATEGY_PARAMS`` (e.g. ``maker_only``, ``close_immediately``).
285+
"""
286+
keys = known_market_making_keys()
287+
for k in ('maker_only', 'close_immediately', 'enable_ws'):
288+
assert k in keys
289+
290+
def test_bot_module_exports_param_lists(self):
291+
"""Direct import — pins that the three lists are at module scope.
292+
293+
If ``_STRATEGY_PARAMS`` or ``_COMMON_PARAMS`` ever get demoted
294+
back inside ``main()``, this import will raise ``ImportError``.
295+
"""
296+
from bot import _STRATEGY_PARAMS, _COMMON_PARAMS, _RISK_PARAMS
297+
assert isinstance(_STRATEGY_PARAMS, dict)
298+
assert 'market_making' in _STRATEGY_PARAMS
299+
assert isinstance(_COMMON_PARAMS, list)
300+
assert isinstance(_RISK_PARAMS, list)

0 commit comments

Comments
 (0)