refactor: Align argparse dest with config keys (Phase 2b)

[keitaj]

Summary

• Ten market_making CLI flags had argparse dest names that differed from the config keys consumed by MarketMakingStrategy. The mismatch required tuple-form rename mappings in _STRATEGY_PARAMS (e.g. ('refresh_interval', 'refresh_interval_seconds')).
• Set dest='<config_key>' explicitly on each affected add_argument so the parser stores the canonical name directly. _STRATEGY_PARAMS entries for these ten flags collapse to plain strings.
• CLI surface is unchanged: --refresh-interval, --vol-adjust, --dynamic-offset-max-add, etc. all continue to behave exactly as before. Only the internal Namespace attribute name is updated.

Why

After Phases 1 (#128) and 2a (#129) introduced grouped dataclasses for the MM strategy, the leftover tuple-form rename mappings in _STRATEGY_PARAMS were the most visible source of "config naming inconsistency" called out in the original design. Aligning the argparse dest with the config key is a strictly internal cleanup: end-users see no change, and the rename indirection disappears from the code.

The _collect_params tuple-handling branch is kept intact because other strategies (rsi, breakout, etc.) may still use it.

Affected flags

CLI flag	dest before	dest after
--refresh-interval	refresh_interval	refresh_interval_seconds
--max-position-age	max_position_age	max_position_age_seconds
--taker-fallback-age	taker_fallback_age	taker_fallback_age_seconds
--vol-adjust	vol_adjust	vol_adjust_enabled
--microprice-skew	microprice_skew	microprice_skew_enabled
--velocity-guard	velocity_guard	velocity_guard_enabled
--dynamic-offset	dynamic_offset	dynamic_offset_enabled
--dynamic-age	dynamic_age	dynamic_age_enabled
--dynamic-offset-max-add	dynamic_offset_max_add	dynamic_offset_max_addition
--dynamic-offset-max-reduce	dynamic_offset_max_reduce	dynamic_offset_max_reduction

Verification

A grep across the repo confirmed that no code outside the parser construction reads any of the old dest names directly (no args.refresh_interval, args.vol_adjust, etc.). All consumption flows through _collect_params into strategy_config.

A direct exercise of the parser also confirms each new dest:

• --refresh-interval 60 → args.refresh_interval_seconds == 60.0
• --vol-adjust → args.vol_adjust_enabled is True

Test plan

• flake8 passes.
• pytest passes (851 tests, unchanged from prior phase).
• Manual parser verification: each flag's dest matches the expected config key.

🤖 Generated with Claude Code

[keitaj]

Review: refactor: Align argparse dest with config keys (Phase 2b)

Tight, focused refactor. Each of the ten add_argument calls picks up an explicit dest= matching the canonical config key, and the corresponding _STRATEGY_PARAMS tuples collapse to plain strings. CLI surface is identical to main, and the existing test suite continues to pass without modification.

Verified the right ten flags are covered (the rename mappings previously in _STRATEGY_PARAMS["market_making"]) and that the special-case flags untouched here (--no-close-immediately, --maker-only, --enable-adverse-selection-log) correctly retain their auto-derived dests because their consumers reference the auto-derived names directly.

One nit on test coverage; not blocking.

---

1. No regression test for the dest mapping (nit)

bot.py argparse block (L985-1104):

The PR relies on manual verification that each dest= matches the config key consumed by _collect_params. If a future edit accidentally drops dest= from one of these flags, argparse silently falls back to the auto-derived name (refresh_interval instead of refresh_interval_seconds), and the value silently fails to land in strategy_config — the bot just runs with the default. That kind of silent degradation is exactly what a regression test catches cheaply.

Suggestion: Extract the parser construction into a small helper (e.g. _build_parser() -> ArgumentParser) and add a unit test that asserts the dest of each previously-aliased flag, e.g.:

@pytest.mark.parametrize("flag,expected_dest", [
    ("--refresh-interval", "refresh_interval_seconds"),
    ("--vol-adjust", "vol_adjust_enabled"),
    # ...the other eight
])
def test_argparse_dest_matches_config_key(flag, expected_dest):
    parser = _build_parser()
    actions = [a for a in parser._actions if flag in a.option_strings]
    assert actions and actions[0].dest == expected_dest

Could be a follow-up PR — the parser refactor is independent of this change.

---

2. _collect_params tuple branch is now dead code (nit)

bot.py L1209-1212 (existing, not modified by this PR):

After this PR, no _STRATEGY_PARAMS entry is a tuple — every strategy uses plain strings. The isinstance(entry, tuple) branch in _collect_params is therefore unreachable. Keeping it as defensive forward-compat is reasonable; just calling it out.

Suggestion: Either leave it as-is with a comment noting its for forward use, or simplify to arg_name, config_key = entry, entry. Pure stylistic, not blocking.

---

Verdict: LGTM

Phase 2b cleanly closes out the naming inconsistency that motivated the original design doc. The two notes above are both follow-up candidates rather than blockers — merging is appropriate after CI confirmation.