Skip to content

refactor: Group MM config into dataclasses (Phase 3)#131

Merged
keitaj merged 1 commit into
mainfrom
refactor/mm-config-phase-3
Apr 28, 2026
Merged

refactor: Group MM config into dataclasses (Phase 3)#131
keitaj merged 1 commit into
mainfrom
refactor/mm-config-phase-3

Conversation

@keitaj

@keitaj keitaj commented Apr 28, 2026

Copy link
Copy Markdown
Owner

Summary

Why

After this PR, every CLI/env-driven MM parameter that has a stable home flows through MMConfig. The remaining config.get(...) calls in MarketMakingStrategy.__init__ are limited to the core MM primitives (spread_bps, order_size_usd, bbo_offset_bps, etc.) and the parameters passed straight through to PositionCloser (max_position_age_seconds, taker_fallback_age_seconds, aggressive_loss_bps).

This closes out the dataclass-grouping work that #128 began. CLI surface, env vars, and existing test suite are unchanged.

Changes

  • strategies/mm_config.py: add DynamicOffsetConfig and DynamicAgeConfig dataclasses; extend MMConfig and from_legacy_dict() with the two groups; refresh MMConfig docstring.
  • strategies/market_making_strategy.py: read dynamic-offset and dynamic-age parameters from self.cfg.*. Existing flat instance attributes preserved as aliases.
  • tests/test_mm_config.py: add unit tests for the two new dataclasses and extend from_legacy_dict end-to-end coverage to all the new keys.

Backward compatibility

  • CLI flags and environment variables: unchanged.
  • Flat instance attributes (self._dynamic_offset_*, self._dynamic_age_*) preserved as aliases of the new cfg fields.
  • No new validation introduced; defaults match the legacy config.get() defaults exactly.

Test plan

  • Unit tests added (2 new dataclass tests + extended from_legacy_dict coverage).
  • flake8 passes.
  • pytest passes (853 tests total).
  • No regressions in existing tests.

🤖 Generated with Claude Code

Add the final two grouped config dataclasses for the MM strategy:

- DynamicOffsetConfig: enabled, sensitivity, tighten_rate, max_addition,
  max_reduction, floor, min_fills (7 keys → 1 dataclass).
- DynamicAgeConfig: enabled, baseline_vol_bps, min_seconds, max_seconds
  (4 keys → 1 dataclass).

MarketMakingStrategy.__init__ now reads these groups from self.cfg.* with
existing flat instance attributes preserved as aliases (Phase 1 pattern).
After this PR every CLI/env-driven MM parameter that has a stable home
flows through MMConfig; the only direct config.get() calls left in
__init__ are those tied to PositionCloser construction or core MM
plumbing (spread_bps, order_size_usd, etc.).

Also refresh the MMConfig docstring — it had been stale since Phase 1
saying close/imbalance/schedule were "future phases".

@keitaj keitaj left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: refactor: Group MM config into dataclasses (Phase 3)

Closes out the dataclass-grouping work cleanly. Two more groups (DynamicOffsetConfig, DynamicAgeConfig), the alias pattern stays consistent with #128/#129, and the previously stale MMConfig docstring is now accurate.

Verified:

  • All 7 dynamic_offset_* legacy keys map to the matching dataclass field with identical defaults.
  • All 4 dynamic_age_* legacy keys map correctly. The dataclass uses more descriptive field names (baseline_vol_bps, min_seconds, max_seconds) while from_legacy_dict reads the original key names — no env / CLI breakage.
  • _dynamic_age_log_interval and _base_max_position_age are intentionally left outside the new dataclass: the former is a constified pseudo-config (Phase 1 scope), the latter is core MM plumbing shared with PositionCloser.
  • 853 tests pass; flat instance aliases in MarketMakingStrategy keep call-sites and tests untouched.

No blockers, no should-fix.


1. No invariant check between min_seconds and max_seconds (nit)

strategies/mm_config.py DynamicAgeConfig:

The dataclass accepts any pair of positive numbers, including min_seconds > max_seconds, which would silently produce a never-satisfied range. The original code didnt validate this either, so this matches existing behaviour — calling it out only because new dataclasses are a natural place to add such guards if you ever want them.

Suggestion: Optional. If desired, add a __post_init__ that raises when min_seconds > max_seconds or when either is non-positive. Defer to a follow-up if youd rather keep this PR purely structural.


Verdict: LGTM

After Phase 3 the MM strategys __init__ only retains direct config.get(...) calls for core MM parameters (spread_bps, order_size_usd, etc.) and a few values still flowing straight to PositionCloser. The dataclass-grouping arc started in #128 is complete. Merging on CI confirmation.

@keitaj
keitaj merged commit 181323b into main Apr 28, 2026
6 checks passed
@keitaj
keitaj deleted the refactor/mm-config-phase-3 branch April 28, 2026 00:22
keitaj added a commit that referenced this pull request Apr 29, 2026
)

Mirror the file layout that emerged from the MM config refactor
(PRs #128#131) so the architecture table stays accurate.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
keitaj added a commit that referenced this pull request Apr 29, 2026
The module-level docstring still described mm_config as Phase 1 with
four sub-configs, but Phases 2–3 (PRs #129, #130, #131) and the
auto-exclude work (PR #132) added six more groups. Rewrite the opening
paragraph so it reflects the current set without naming a specific
phase that will keep going stale.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant