Skip to content

Commit 2edad25

Browse files
keitajclaude
andauthored
fix: wire JSON risk namespace through to Config (close PR #148 gap) (#149)
The previous JSON config layer (#148) flattened ``{"risk": {...}}`` into flat keys correctly, but those values only flowed into ``strategy_config`` — the strategy-level merged dict — not into the ``Config`` class that ``RiskManager`` actually reads from. Operators putting ``"daily_loss_limit": 200`` in their JSON would see no effect on the risk guardrails. This change closes that gap: * Promote ``_RISK_PARAMS`` from ``main()`` to module level so the JSON helper and the CLI override loop share the list. * Add ``_apply_json_risk_overrides(json_overrides, args)``: for each risk param present in JSON and *not* already supplied via CLI args, write the value to ``Config.{KEY.upper()}``. Pop the key from ``json_overrides`` so it does not leak into ``strategy_config`` (which the strategy would not know what to do with) or trigger the typo detector. * Call the helper right after ``load_json_configs`` and before the existing CLI risk loop. CLI > JSON precedence is preserved because the CLI loop runs last and unconditionally writes ``args.{param}`` when present. * ``known_market_making_keys()`` now includes ``_RISK_PARAMS`` so the JSON loader's typo detector treats risk keys as known when they appear at flat-top-level (e.g., a user mixing flat + nested forms). * README clarifies that the ``risk`` namespace targets ``Config``, not ``strategy_config``, and that CLI flags still beat JSON. 5 new test cases pin the behaviour: JSON sets Config when CLI is unset; CLI beats JSON; multiple risk params apply together; empty / None inputs are no-ops; unknown risk-shaped keys pass through untouched. Tests snapshot and restore the relevant ``Config`` class attributes so they stay isolated from each other and from the wider suite. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c5813c4 commit 2edad25

4 files changed

Lines changed: 167 additions & 8 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,8 @@ becomes:
336336

337337
Unknown keys produce a warning (typo detection) but are still passed through to the validator, which decides whether to abort. Missing files are warned and skipped (so a typo in `--config /missing.json` does not block startup); malformed JSON aborts with exit code 2. See `examples/config.example.json` for a full template.
338338

339+
The `risk` namespace flows to the bot's `Config` class (the same path the existing `--max-position-pct` / `--daily-loss-limit` / etc. CLI flags target), not to `strategy_config`. CLI flags still beat JSON for any individual risk parameter, so an operator can leave defaults in JSON and override per-run from the command line.
340+
339341
The `market_making` strategy uses **progressive close pricing**: as a position ages, the take-profit price is tightened from full spread → breakeven (at 50% of max age) → small loss (at 75%), reducing costly taker force-closes. The loss tolerance is configurable via `--aggressive-loss-bps` (default: 1 bps). During the force-close phase, `--force-close-max-loss-bps` enables progressive loss acceptance that scales from `aggressive-loss-bps` to the configured maximum as the position approaches the taker deadline. **Unrealized loss early close** (`--unrealized-loss-close-bps`): When a position's unrealized loss exceeds this threshold (in bps), it is immediately closed via taker order regardless of position age. This caps large adverse moves before the age-based close triggers. Default: 0 (disabled).
340342

341343
**BBO mode** (`--bbo-mode`): Places orders at the best bid/ask instead of `mid ± spread_bps`. On Hyperliquid, market spreads are typically 0.1–2 bps, so even `SPREAD_BPS=5` places orders 4–5 bps away from BBO, resulting in low fill rates. BBO mode improves fill rates by tracking the current best prices. Use `--bbo-offset-bps N` to place orders N bps behind BBO (default: 0 = at BBO). Falls back to `mid ± spread_bps` when BBO is unavailable.

bot.py

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,46 @@
4141
logger = logging.getLogger(__name__)
4242

4343

44+
# Risk-guardrail parameter names. Promoted to module-level so the JSON
45+
# config layer (``_apply_json_risk_overrides``) and the existing CLI
46+
# override loop in ``main()`` share the same list. Each name maps to:
47+
# - argparse dest: ``args.{name}``
48+
# - Config class attribute: ``Config.{name.upper()}``
49+
_RISK_PARAMS = [
50+
'max_position_pct', 'max_margin_usage', 'force_close_margin',
51+
'force_close_leverage', 'daily_loss_limit', 'per_trade_stop_loss',
52+
'max_open_positions', 'cooldown_after_stop',
53+
]
54+
55+
56+
def _apply_json_risk_overrides(json_overrides: Optional[Dict], args) -> None:
57+
"""Wire risk parameters from JSON config into the ``Config`` class.
58+
59+
JSON values flow to ``Config.{KEY.upper()}`` only when the same key
60+
is **not** also present on ``args`` (CLI > JSON precedence). After
61+
application the keys are popped from ``json_overrides`` so they do
62+
not leak into the strategy_config dict (where the strategy would
63+
not know what to do with them and the typo detector would false-warn).
64+
65+
No-op when ``json_overrides`` is empty or None. Existing CLI / env
66+
behaviour is unaffected.
67+
"""
68+
if not json_overrides:
69+
return
70+
for param in _RISK_PARAMS:
71+
if param not in json_overrides:
72+
continue
73+
cli_val = getattr(args, param, None)
74+
if cli_val is not None:
75+
# CLI already specified — let the existing CLI loop handle it
76+
# and just pop the JSON value so it does not pollute strategy_config.
77+
json_overrides.pop(param, None)
78+
continue
79+
value = json_overrides.pop(param)
80+
setattr(Config, param.upper(), value)
81+
logger.info(f"[config] Risk param from JSON: {param.upper()}={value}")
82+
83+
4484
class HyperliquidBot:
4585
def __init__(self, strategy_name: str = "simple_ma", coins: Optional[List[str]] = None,
4686
strategy_config: Optional[Dict] = None,
@@ -1371,12 +1411,9 @@ def _collect_params(params, source, dest):
13711411
if args.no_hl:
13721412
Config.ENABLE_STANDARD_HL = False
13731413

1374-
# Apply CLI overrides for risk guardrails (CLI > env > default)
1375-
_RISK_PARAMS = [
1376-
'max_position_pct', 'max_margin_usage', 'force_close_margin',
1377-
'force_close_leverage', 'daily_loss_limit', 'per_trade_stop_loss',
1378-
'max_open_positions', 'cooldown_after_stop',
1379-
]
1414+
# Apply CLI overrides for risk guardrails (CLI > env > default).
1415+
# ``_RISK_PARAMS`` is defined at module level so the JSON layer
1416+
# (``_apply_json_risk_overrides``) and this loop stay in sync.
13801417
for param in _RISK_PARAMS:
13811418
val = getattr(args, param, None)
13821419
if val is not None:
@@ -1421,6 +1458,11 @@ def _collect_params(params, source, dest):
14211458
logger.error(f"{e}")
14221459
raise SystemExit(2)
14231460

1461+
# Risk parameters from JSON flow to ``Config`` (separate from
1462+
# ``strategy_config``). CLI args still beat JSON because the CLI
1463+
# override loop further down checks ``args.{param}`` first.
1464+
_apply_json_risk_overrides(json_overrides, args)
1465+
14241466
bot = HyperliquidBot(
14251467
strategy_name=args.strategy,
14261468
coins=args.coins if Config.ENABLE_STANDARD_HL else [],

tests/test_bot_config_layering.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,115 @@ def test_loader_output_layers_correctly_under_cli(tmp_path):
169169
"forager_enabled": True, # JSON > default
170170
"max_open_orders": 4, # default
171171
}
172+
173+
174+
# --------------------------------------------------------------------------- #
175+
# Risk namespace: JSON values must reach Config, not just strategy_config.
176+
# --------------------------------------------------------------------------- #
177+
178+
179+
class TestJsonRiskOverrides:
180+
"""``_apply_json_risk_overrides`` wires JSON ``risk:`` keys to Config.
181+
182+
Saves and restores the relevant ``Config`` attributes so tests stay
183+
isolated from each other and from the wider suite.
184+
"""
185+
186+
_SAVED_ATTRS = (
187+
'MAX_POSITION_PCT', 'MAX_MARGIN_USAGE', 'DAILY_LOSS_LIMIT',
188+
'PER_TRADE_STOP_LOSS', 'FORCE_CLOSE_MARGIN', 'FORCE_CLOSE_LEVERAGE',
189+
)
190+
191+
def setup_method(self):
192+
from config import Config
193+
self._snapshot = {a: getattr(Config, a) for a in self._SAVED_ATTRS}
194+
195+
def teardown_method(self):
196+
from config import Config
197+
for a, v in self._snapshot.items():
198+
setattr(Config, a, v)
199+
200+
def _args(self, **overrides):
201+
"""Build a Namespace mimicking argparse output (None where unset)."""
202+
import argparse
203+
attrs = {p: None for p in (
204+
'max_position_pct', 'max_margin_usage', 'daily_loss_limit',
205+
'per_trade_stop_loss', 'force_close_margin', 'force_close_leverage',
206+
'max_open_positions', 'cooldown_after_stop',
207+
)}
208+
attrs.update(overrides)
209+
return argparse.Namespace(**attrs)
210+
211+
def test_json_risk_value_sets_config_when_cli_unset(self):
212+
from bot import _apply_json_risk_overrides
213+
from config import Config
214+
215+
json_overrides = {"daily_loss_limit": 250.0, "spread_bps": 10}
216+
args = self._args(daily_loss_limit=None)
217+
218+
_apply_json_risk_overrides(json_overrides, args)
219+
220+
assert Config.DAILY_LOSS_LIMIT == 250.0
221+
# Risk key popped from the dict — it should not pollute strategy_config.
222+
assert "daily_loss_limit" not in json_overrides
223+
# Non-risk key is left alone.
224+
assert json_overrides == {"spread_bps": 10}
225+
226+
def test_cli_value_beats_json_for_risk(self):
227+
"""CLI > JSON: when args has a risk param, JSON is ignored & popped."""
228+
from bot import _apply_json_risk_overrides
229+
from config import Config
230+
231+
# Simulate the existing CLI loop having already applied 999 to Config.
232+
Config.DAILY_LOSS_LIMIT = 999.0
233+
234+
json_overrides = {"daily_loss_limit": 250.0}
235+
args = self._args(daily_loss_limit=999.0)
236+
237+
_apply_json_risk_overrides(json_overrides, args)
238+
239+
# Config still holds the CLI value.
240+
assert Config.DAILY_LOSS_LIMIT == 999.0
241+
# JSON value was popped (so it doesn't leak into strategy_config)
242+
# but Config was *not* touched by the helper.
243+
assert "daily_loss_limit" not in json_overrides
244+
245+
def test_multiple_risk_params(self):
246+
from bot import _apply_json_risk_overrides
247+
from config import Config
248+
249+
json_overrides = {
250+
"max_position_pct": 0.5,
251+
"max_margin_usage": 0.6,
252+
"daily_loss_limit": 100.0,
253+
"per_trade_stop_loss": 0.03,
254+
"spread_bps": 7,
255+
}
256+
args = self._args()
257+
258+
_apply_json_risk_overrides(json_overrides, args)
259+
260+
assert Config.MAX_POSITION_PCT == 0.5
261+
assert Config.MAX_MARGIN_USAGE == 0.6
262+
assert Config.DAILY_LOSS_LIMIT == 100.0
263+
assert Config.PER_TRADE_STOP_LOSS == 0.03
264+
# Only the non-risk key remains in json_overrides.
265+
assert json_overrides == {"spread_bps": 7}
266+
267+
def test_no_op_when_json_overrides_empty_or_none(self):
268+
from bot import _apply_json_risk_overrides
269+
from config import Config
270+
271+
before = Config.DAILY_LOSS_LIMIT
272+
_apply_json_risk_overrides(None, self._args())
273+
_apply_json_risk_overrides({}, self._args())
274+
assert Config.DAILY_LOSS_LIMIT == before
275+
276+
def test_unknown_risk_key_in_json_ignored(self):
277+
"""Keys not in _RISK_PARAMS pass through untouched."""
278+
from bot import _apply_json_risk_overrides
279+
280+
json_overrides = {"spread_bps": 10, "made_up_risk_param": 0.5}
281+
_apply_json_risk_overrides(json_overrides, self._args())
282+
# No risk keys present — both pass through untouched.
283+
assert json_overrides == {"spread_bps": 10, "made_up_risk_param": 0.5}

validation/strategy_validator.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,18 +319,21 @@ def known_market_making_keys() -> set:
319319
(e.g. ``maker_only``, ``close_immediately``, ``max_positions``).
320320
"""
321321
# Defer the import to avoid a cycle (bot.py imports validators).
322-
from bot import _STRATEGY_PARAMS, _COMMON_PARAMS
322+
from bot import _STRATEGY_PARAMS, _COMMON_PARAMS, _RISK_PARAMS
323323

324324
keys = set()
325325
keys.update(_extract_param_names(_STRATEGY_PARAMS.get('market_making', [])))
326326
keys.update(_extract_param_names(_COMMON_PARAMS))
327+
# Risk-guardrail names — applied via ``Config.{KEY.upper()}`` in
328+
# ``_apply_json_risk_overrides``; included so the JSON typo detector
329+
# treats them as known.
330+
keys.update(_RISK_PARAMS)
327331
# A few derived keys not in _STRATEGY_PARAMS but read via config.get
328332
# in MarketMakingStrategy:
329333
keys.update({
330334
'close_immediately',
331335
'maker_only',
332336
'max_positions',
333-
'max_open_positions',
334337
'enable_adverse_selection_log',
335338
'enable_ws',
336339
'main_loop_interval',

0 commit comments

Comments
 (0)