From b69bc142175bf883d79c50db5de052dcbc586b76 Mon Sep 17 00:00:00 2001 From: keitaj Date: Mon, 4 May 2026 11:20:47 +0900 Subject: [PATCH] fix: wire JSON risk namespace through to Config (close PR #148 gap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- README.md | 2 + bot.py | 54 ++++++++++++-- tests/test_bot_config_layering.py | 112 ++++++++++++++++++++++++++++++ validation/strategy_validator.py | 7 +- 4 files changed, 167 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0a4b928..4dd1f6a 100644 --- a/README.md +++ b/README.md @@ -336,6 +336,8 @@ becomes: 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. +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. + 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). **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. diff --git a/bot.py b/bot.py index b12da7a..3fe3f2a 100644 --- a/bot.py +++ b/bot.py @@ -41,6 +41,46 @@ logger = logging.getLogger(__name__) +# Risk-guardrail parameter names. Promoted to module-level so the JSON +# config layer (``_apply_json_risk_overrides``) and the existing CLI +# override loop in ``main()`` share the same list. Each name maps to: +# - argparse dest: ``args.{name}`` +# - Config class attribute: ``Config.{name.upper()}`` +_RISK_PARAMS = [ + 'max_position_pct', 'max_margin_usage', 'force_close_margin', + 'force_close_leverage', 'daily_loss_limit', 'per_trade_stop_loss', + 'max_open_positions', 'cooldown_after_stop', +] + + +def _apply_json_risk_overrides(json_overrides: Optional[Dict], args) -> None: + """Wire risk parameters from JSON config into the ``Config`` class. + + JSON values flow to ``Config.{KEY.upper()}`` only when the same key + is **not** also present on ``args`` (CLI > JSON precedence). After + application the keys are popped from ``json_overrides`` so they do + not leak into the strategy_config dict (where the strategy would + not know what to do with them and the typo detector would false-warn). + + No-op when ``json_overrides`` is empty or None. Existing CLI / env + behaviour is unaffected. + """ + if not json_overrides: + return + for param in _RISK_PARAMS: + if param not in json_overrides: + continue + cli_val = getattr(args, param, None) + if cli_val is not None: + # CLI already specified — let the existing CLI loop handle it + # and just pop the JSON value so it does not pollute strategy_config. + json_overrides.pop(param, None) + continue + value = json_overrides.pop(param) + setattr(Config, param.upper(), value) + logger.info(f"[config] Risk param from JSON: {param.upper()}={value}") + + class HyperliquidBot: def __init__(self, strategy_name: str = "simple_ma", coins: Optional[List[str]] = None, strategy_config: Optional[Dict] = None, @@ -1371,12 +1411,9 @@ def _collect_params(params, source, dest): if args.no_hl: Config.ENABLE_STANDARD_HL = False - # Apply CLI overrides for risk guardrails (CLI > env > default) - _RISK_PARAMS = [ - 'max_position_pct', 'max_margin_usage', 'force_close_margin', - 'force_close_leverage', 'daily_loss_limit', 'per_trade_stop_loss', - 'max_open_positions', 'cooldown_after_stop', - ] + # Apply CLI overrides for risk guardrails (CLI > env > default). + # ``_RISK_PARAMS`` is defined at module level so the JSON layer + # (``_apply_json_risk_overrides``) and this loop stay in sync. for param in _RISK_PARAMS: val = getattr(args, param, None) if val is not None: @@ -1421,6 +1458,11 @@ def _collect_params(params, source, dest): logger.error(f"{e}") raise SystemExit(2) + # Risk parameters from JSON flow to ``Config`` (separate from + # ``strategy_config``). CLI args still beat JSON because the CLI + # override loop further down checks ``args.{param}`` first. + _apply_json_risk_overrides(json_overrides, args) + bot = HyperliquidBot( strategy_name=args.strategy, coins=args.coins if Config.ENABLE_STANDARD_HL else [], diff --git a/tests/test_bot_config_layering.py b/tests/test_bot_config_layering.py index 54508e0..543870b 100644 --- a/tests/test_bot_config_layering.py +++ b/tests/test_bot_config_layering.py @@ -169,3 +169,115 @@ def test_loader_output_layers_correctly_under_cli(tmp_path): "forager_enabled": True, # JSON > default "max_open_orders": 4, # default } + + +# --------------------------------------------------------------------------- # +# Risk namespace: JSON values must reach Config, not just strategy_config. +# --------------------------------------------------------------------------- # + + +class TestJsonRiskOverrides: + """``_apply_json_risk_overrides`` wires JSON ``risk:`` keys to Config. + + Saves and restores the relevant ``Config`` attributes so tests stay + isolated from each other and from the wider suite. + """ + + _SAVED_ATTRS = ( + 'MAX_POSITION_PCT', 'MAX_MARGIN_USAGE', 'DAILY_LOSS_LIMIT', + 'PER_TRADE_STOP_LOSS', 'FORCE_CLOSE_MARGIN', 'FORCE_CLOSE_LEVERAGE', + ) + + def setup_method(self): + from config import Config + self._snapshot = {a: getattr(Config, a) for a in self._SAVED_ATTRS} + + def teardown_method(self): + from config import Config + for a, v in self._snapshot.items(): + setattr(Config, a, v) + + def _args(self, **overrides): + """Build a Namespace mimicking argparse output (None where unset).""" + import argparse + attrs = {p: None for p in ( + 'max_position_pct', 'max_margin_usage', 'daily_loss_limit', + 'per_trade_stop_loss', 'force_close_margin', 'force_close_leverage', + 'max_open_positions', 'cooldown_after_stop', + )} + attrs.update(overrides) + return argparse.Namespace(**attrs) + + def test_json_risk_value_sets_config_when_cli_unset(self): + from bot import _apply_json_risk_overrides + from config import Config + + json_overrides = {"daily_loss_limit": 250.0, "spread_bps": 10} + args = self._args(daily_loss_limit=None) + + _apply_json_risk_overrides(json_overrides, args) + + assert Config.DAILY_LOSS_LIMIT == 250.0 + # Risk key popped from the dict — it should not pollute strategy_config. + assert "daily_loss_limit" not in json_overrides + # Non-risk key is left alone. + assert json_overrides == {"spread_bps": 10} + + def test_cli_value_beats_json_for_risk(self): + """CLI > JSON: when args has a risk param, JSON is ignored & popped.""" + from bot import _apply_json_risk_overrides + from config import Config + + # Simulate the existing CLI loop having already applied 999 to Config. + Config.DAILY_LOSS_LIMIT = 999.0 + + json_overrides = {"daily_loss_limit": 250.0} + args = self._args(daily_loss_limit=999.0) + + _apply_json_risk_overrides(json_overrides, args) + + # Config still holds the CLI value. + assert Config.DAILY_LOSS_LIMIT == 999.0 + # JSON value was popped (so it doesn't leak into strategy_config) + # but Config was *not* touched by the helper. + assert "daily_loss_limit" not in json_overrides + + def test_multiple_risk_params(self): + from bot import _apply_json_risk_overrides + from config import Config + + json_overrides = { + "max_position_pct": 0.5, + "max_margin_usage": 0.6, + "daily_loss_limit": 100.0, + "per_trade_stop_loss": 0.03, + "spread_bps": 7, + } + args = self._args() + + _apply_json_risk_overrides(json_overrides, args) + + assert Config.MAX_POSITION_PCT == 0.5 + assert Config.MAX_MARGIN_USAGE == 0.6 + assert Config.DAILY_LOSS_LIMIT == 100.0 + assert Config.PER_TRADE_STOP_LOSS == 0.03 + # Only the non-risk key remains in json_overrides. + assert json_overrides == {"spread_bps": 7} + + def test_no_op_when_json_overrides_empty_or_none(self): + from bot import _apply_json_risk_overrides + from config import Config + + before = Config.DAILY_LOSS_LIMIT + _apply_json_risk_overrides(None, self._args()) + _apply_json_risk_overrides({}, self._args()) + assert Config.DAILY_LOSS_LIMIT == before + + def test_unknown_risk_key_in_json_ignored(self): + """Keys not in _RISK_PARAMS pass through untouched.""" + from bot import _apply_json_risk_overrides + + json_overrides = {"spread_bps": 10, "made_up_risk_param": 0.5} + _apply_json_risk_overrides(json_overrides, self._args()) + # No risk keys present — both pass through untouched. + assert json_overrides == {"spread_bps": 10, "made_up_risk_param": 0.5} diff --git a/validation/strategy_validator.py b/validation/strategy_validator.py index c614b66..e3c1d3f 100644 --- a/validation/strategy_validator.py +++ b/validation/strategy_validator.py @@ -319,18 +319,21 @@ def known_market_making_keys() -> set: (e.g. ``maker_only``, ``close_immediately``, ``max_positions``). """ # Defer the import to avoid a cycle (bot.py imports validators). - from bot import _STRATEGY_PARAMS, _COMMON_PARAMS + from bot import _STRATEGY_PARAMS, _COMMON_PARAMS, _RISK_PARAMS keys = set() keys.update(_extract_param_names(_STRATEGY_PARAMS.get('market_making', []))) keys.update(_extract_param_names(_COMMON_PARAMS)) + # Risk-guardrail names — applied via ``Config.{KEY.upper()}`` in + # ``_apply_json_risk_overrides``; included so the JSON typo detector + # treats them as known. + keys.update(_RISK_PARAMS) # A few derived keys not in _STRATEGY_PARAMS but read via config.get # in MarketMakingStrategy: keys.update({ 'close_immediately', 'maker_only', 'max_positions', - 'max_open_positions', 'enable_adverse_selection_log', 'enable_ws', 'main_loop_interval',