docs: disclose that inventory skew and position cap are inert

[keitaj]

Summary

• inventory_skew_bps and max_position_multiple are currently no-ops, but the README documents both as if setting them takes effect. This PR makes that discoverable instead of silent.
• No change to order placement — logging, docs and tests only.
• Adds a regression test that pins the single-sided invariant, so whoever implements two-sided quoting gets a failing test reminding them to update the warnings and docs.

Why they are inert

run() hands any coin that holds a position to PositionCloser and continues, so _place_orders() only ever runs while flat:

if has_position:
    ...
    self._closer.manage(...)
    continue
# No position — normal MM flow

Both features gate on a non-zero position (_calculate_inventory_skew returns 0.0 without one; the cap checks size != 0), so neither can reach a placed order. There is no mid-cycle race either: self.positions is only repopulated by update_positions() at the top of a cycle.

Two things made this hard to notice:

1. The cycle log computed and printed the skew for position-holding coins (BTC:skew+1.4bp), implying the value was affecting orders.
2. The existing unit tests call _place_orders directly with a position set, a state the live flow never produces — so they pass while the feature is inert.

Changes

File	Change
strategies/market_making_strategy.py	New _warn_inert_parameters() logs a WARNING when either param is non-zero; replaces the misleading Position cap armed INFO. Cycle log no longer prints the skew. Corrected the stale comment claiming opposite-side entries still place. Added a docstring note on _calculate_inventory_skew.
README.md	New "Inventory skew and position cap are inert" section; corrected the position-cap paragraph and both YAML reference entries.
tests/test_mm_single_sided_flow.py	New. Pins that _place_orders is skipped while a position is open and runs when flat, that the warnings fire only when non-zero, and that the cycle log omits the skew.
tests/test_mm_cycle_log.py	test_position_with_skew → test_position_never_reports_skew (asserts the new behaviour).
tests/test_mm_inventory_skew.py, tests/test_mm_position_cap.py	Docstring notes explaining these exercise a path unreachable in the live flow.

The parameters are kept rather than removed: both become meaningful under a two-sided quoting mode, where a coin keeps quoting while holding inventory. That is a deliberate strategy change (it trades faster inventory flattening for more quote uptime, and only pays off when the risk manager's net-inventory cap is large relative to the order size), so it is out of scope here.

Test plan

• Unit tests added/updated (9 new, 1 updated)
• flake8 passes
• pytest passes — 1286 passed
• No regressions in existing tests

🤖 Generated with Claude Code

[keitaj]

Review: disclose that inventory skew and position cap are inert

The core change is correct and well-scoped. The root-cause analysis holds up against the code: run() routes any coin with a position to PositionCloser and continues (market_making_strategy.py L482-495), so _place_orders() only runs while flat, and both features gate on a non-zero position. self.positions is only repopulated by update_positions() at the top of a cycle, so there is genuinely no mid-cycle race that could make them fire. No production behaviour changes, __init__ ordering is right (inventory_skew_bps L72 and _max_position_multiple L142 are both set before _warn_inert_parameters() L144), and test_place_orders_skipped_while_position_open is a good invariant pin for whoever implements two-sided quoting later.

Two issues, both in the test suite rather than the change itself.

---

1. test_market_data_none is now vacuous (should-fix)

tests/test_mm_cycle_log.py L132-144:

This test existed to cover the mid = md.mid_price if md else 0 fallback inside the position branch — the branch this PR deletes. With the skew no longer computed there, the position path never touches market data, so md.get_market_data.return_value = None has no bearing on the assertion. The test now passes trivially, and its inline comment (# skew=0 when no market data → shows :pos) describes a code path that no longer exists.

Leaving a green test that asserts nothing is worse than deleting it: it implies coverage that is not there.

Suggestion: either drop it, or re-scope it to the branch that does still read market data — the idle path with vol_adjust_enabled=True, where _get_volatility_adjusted_offset is consulted. If re-scoped, rename it accordingly (e.g. test_idle_coin_without_market_data).

---

2. test_position_with_zero_skew is now redundant (nit)

tests/test_mm_cycle_log.py L119-130:

Now that the position branch ignores the skew entirely, this test and the updated test_position_never_reports_skew (L97) exercise the same code path with the only difference being an input that is no longer read. The latter is strictly stronger (it also asserts skew is absent from the line).

Suggestion: fold it into the parametrisation of test_position_never_reports_skew (inventory_skew_bps of 0 and 2 both expected to yield :pos), or remove it.

---

Verdict: Changes requested

No blockers — nothing here affects order placement or risk guards, and lint/tests pass. But item 1 leaves a test whose name and comment claim coverage it no longer provides, which is the same class of problem this PR sets out to fix (a green test masking an inert code path). Worth correcting before merge so the suite does not repeat the pattern.

[keitaj]

Re-review: follow-up on the two test findings

Diff reviewed: ed800e4..dcda76d (tests/test_mm_cycle_log.py only, +18/-22). Both findings are addressed and the fix is scoped to the test file — no production code moved.

1. Vacuous test_market_data_none — resolved. Re-scoped to test_idle_coin_without_market_data, which targets the idle branch (the one that still consults market data now that the position branch does not). The added assert md.get_market_data.called is the right touch: it proves the None actually flowed through the quoting path, so the test cannot silently degrade into the same "passes on an unexercised branch" state it replaced.

2. Redundant test_position_with_zero_skew — resolved. Folded into @pytest.mark.parametrize("inventory_skew_bps", [0, 2]) on test_position_never_reports_skew. The stronger assertions (skew absent, 1 pos present) now cover both inputs, where previously the zero case only checked :pos. Net coverage goes up, not down.

pytest import added correctly alongside the parametrize.

Verdict: LGTM

Both should-fix/nit items closed, no new concerns. The change remains behaviour-neutral for order placement, the risk guards are untouched, and test_place_orders_skipped_while_position_open gives whoever implements two-sided quoting a failing test to prompt updating the warnings and docs. Proceeding to CI and merge.