feat: Add per-coin unrealized_loss_close_bps overrides#143
Conversation
Mirrors the existing coin_offset / coin_spread / coin_size override
pattern for the unrealized-loss early-taker-close threshold so the bps
threshold can be tuned per coin. Low-vol coins can be relaxed
(e.g. INTC:25) to suppress over-eager early closes; volatile coins
can be tightened (e.g. OIL:10) to cap loss faster. An override of 0
disables the unrealized-loss check for that coin even when the global
threshold is non-zero.
PerCoinOverrides gains an `unrealized_loss` dict; PositionCloser gets
a new constructor kwarg `coin_unrealized_loss_overrides` and a
`_get_unrealized_loss_bps_for_coin` helper with the same DEX-prefixed-
or-bare lookup as `_get_spread_for_coin`. The `manage()` early-close
block reads through the helper, and the warning log now shows the
threshold actually applied (not the global value).
Backward-compatible: when the new flag is absent, behavior is
identical to before. Existing tests are unmodified except for one
assertion that confirms the new field defaults to {}.
keitaj
left a comment
There was a problem hiding this comment.
Review: per-coin unrealized_loss_close_bps overrides
Tightly mirrors the existing coin_offset / coin_spread / coin_size override pattern — same parser, same DEX-prefix-or-bare lookup helper layout. The change is small, fully covered by tests (relaxed override, tightened override, unspecified-coin fallback, DEX-prefix→bare resolution, disable-via-zero, and the log-message threshold reporting), and backward-compatible: the new ctor kwarg defaults to None, the legacy-dict key defaults to '', and the gate > 0 keeps the existing "global = 0 means disabled" semantics intact.
1. Negative override values are silently treated as disabled (nit)
strategies/mm_position_closer.py (_get_unrealized_loss_bps_for_coin + the > 0 gate in manage()):
If a user passes --coin-unrealized-loss-overrides INTC:-5, the override resolves to -5.0, and the if unrealized_loss_threshold > 0 gate quietly disables the feature for that coin without warning. This matches existing behavior of parse_coin_overrides (no validation of value ranges) and the analogous _get_spread_for_coin paths, so it's consistent with the rest of the codebase. A future hardening pass could centralize "non-negative numeric override" validation in parse_coin_overrides or in each consumer's helper. Not for this PR.
2. Log-message threshold reporting (worth noting, not a fix)
strategies/mm_position_closer.py (the warning log inside the early-close branch):
logger.warning(
f"[mm] Position {coin} unrealized loss {unrealized_bps:.1f}bps "
f"exceeds threshold {unrealized_loss_threshold}bps -- "
...
)The literal value in the message has changed from self.unrealized_loss_close_bps (global) to unrealized_loss_threshold (effective per-coin). This is the intended behaviour and is more useful for ops, but it's a log-format change — anyone with a downstream parser keyed on the global value would notice. Repo-internal parsers (the [close-reason] regex in /review) only target the structured [close-reason] line, not this warning, so we're fine. Worth keeping in mind if external dashboards exist.
Verdict: LGTM
No blockers, no should-fixes, two informational observations. CI green per author note; will re-run before merge.
Summary
coin_unrealized_loss_overridesconfig key (CLI flag--coin-unrealized-loss-overrides) so the unrealized-loss early-taker-close threshold can be tuned per coin instead of being applied uniformly.coin_offset_overrides/coin_spread_overrides/coin_size_overridespattern: same parser, same DEX-prefixed-or-bare lookup, sameDict[str, float]shape onPerCoinOverrides.0disables the unrealized-loss check for that coin even when the global threshold is non-zero — useful for stable / illiquid coins where the early-close path was firing too eagerly.Changes
strategies/mm_config.py:PerCoinOverridesgainsunrealized_loss: Dict[str, float] = {};from_legacy_dictparsescoin_unrealized_loss_overrides.strategies/mm_position_closer.py: new constructor kwargcoin_unrealized_loss_overrides; new_get_unrealized_loss_bps_for_coin(coin)helper; themanage()early-close block reads through the helper. The warning log now shows the threshold actually applied.strategies/market_making_strategy.py: passes the override dict toPositionCloser(alongside the existingcoin_spread_overrides); logs the configured overrides at startup.bot.py:--coin-unrealized-loss-overridesargparse entry;_STRATEGY_PARAMS['market_making']includescoin_unrealized_loss_overrides.README.md: per-coin overrides section and YAML reference updated.tests/test_mm_config.py:PerCoinOverridesdefaults assert the new field; full-dict round-trip checks the new key.tests/test_unrealized_loss_close.py: newTestCoinUnrealizedLossOverridescovering relaxed override, tightened override, unspecified-coin fallback, DEX-prefixed-vs-bare lookup, the disable-via-zero behavior, and the warning-log threshold reporting.Test plan
flake8passes on the full repo.pytest tests/ -qpasses (937 tests, +7 new).[mm] Position … unrealized loss …bps exceeds threshold …bps …warning text format is preserved (just reports the per-coin value when an override applies).🤖 Generated with Claude Code