Skip to content

feat: Add drain mode for graceful pre-shutdown (--drain-flag-file)#127

Merged
keitaj merged 1 commit into
mainfrom
feat/session-switch-drain-mode
Apr 26, 2026
Merged

feat: Add drain mode for graceful pre-shutdown (--drain-flag-file)#127
keitaj merged 1 commit into
mainfrom
feat/session-switch-drain-mode

Conversation

@keitaj

@keitaj keitaj commented Apr 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a flag-file based drain mode for the market-making strategy. When the configured file exists, the strategy stops placing new entry orders and only manages existing positions via the normal maker-first PositionCloser flow.
  • Designed to be triggered by an external script (e.g., a session-switch helper) before sending SIGTERM, so positions can unwind via maker close instead of taker IOC close at the boundary.
  • Behavior mirrors the existing quiet_hours full-stop mode; drain takes priority when both apply.

Changes

  • strategies/market_making_strategy.py
    • New _drain_flag_file and _was_drain state initialised from config.get('drain_flag_file', '').
    • New _is_drain_mode() helper that returns True when the flag file exists (cheap os.path.exists per cycle).
    • In run(), a drain branch executes before the quiet-hours branch: cancels open orders on entry, manages existing positions through PositionCloser, skips new entry quotes, logs [DRAIN] cycle suffix, and resets state when the flag disappears.
  • bot.py
    • New CLI flag --drain-flag-file (default empty = disabled).
    • Wired into _STRATEGY_PARAMS['market_making'] as drain_flag_file.
  • tests/test_drain_mode.py (new)
    • _is_drain_mode() truthiness in all configurations (empty / missing / present / None).
    • Drain cancels all orders on entry and does not repeat-cancel on subsequent cycles.
    • Drain skips bulk_place_orders when no positions are open.
    • Drain still calls PositionCloser.manage() for open positions.
    • Removing the flag file resets _was_drain.
    • Drain takes precedence over quiet_hours when both apply.
    • When unconfigured, normal quoting proceeds.
  • tests/test_mm_*.py (3 files): test fixtures that bypass __init__ extended with _drain_flag_file = '' and _was_drain = False to keep the existing assertions intact.
  • README.md: human description in ## Strategies plus YAML reference in ## Parameter Reference for AI Agents.

Test plan

  • Unit tests added (tests/test_drain_mode.py, 8 tests)
  • flake8 . --max-line-length=120 passes (no new findings)
  • pytest tests/ passes (810 / 810)
  • No regressions in test_quiet_hours.py (drain takes priority but quiet-hours behaviour preserved when drain inactive)

🤖 Generated with Claude Code

Introduce a flag-file based drain mode for the market-making strategy.
When the configured file exists, the strategy stops placing new entry
orders and only manages existing positions via the normal maker-first
PositionCloser flow. Designed to be triggered by an external script
before SIGTERM so positions can unwind via maker close instead of taker
IOC close at the boundary.

- New CLI flag --drain-flag-file (default: empty = disabled)
- Behavior mirrors quiet_hours full-stop mode; drain takes priority
  when both apply
- _is_drain_mode() polls the flag file each cycle (cheap stat call)
- New tests/test_drain_mode.py covering enable/disable, no-repeat
  cancel, position management, exit reset, and precedence over
  quiet_hours
- Update test fixtures that bypass __init__ to set the new attributes

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@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: Drain mode for graceful pre-shutdown

Clean implementation that reuses the established quiet_hours full-stop pattern. Backward compatible (empty default disables), risk guards untouched, well covered by the new test module. One nit and one optional test gap below.


1. Redundant _is_drain_mode() evaluation in run() (nit)

strategies/market_making_strategy.py L279–L313:

The drain branch calls self._is_drain_mode() twice per cycle in the not-active path: once to gate the early-return block, then again in if self._was_drain and not self._is_drain_mode(): after it. Each call performs an os.path.exists syscall. Cheap, but easy to dedupe and matches the pattern already used for is_quiet.

Suggestion:

is_drain = self._is_drain_mode()
if is_drain:
    # ... existing drain branch ...
    return

if self._was_drain and not is_drain:
    logger.info("[mm] Exiting drain mode, resuming quotes")
    self._was_drain = False

2. Test coverage for close_immediately=True in drain (nit)

tests/test_drain_mode.py:

TestDrainModeBehavior only exercises the close_immediately=False branch (the default in tests). The if self.close_immediately: self.close_position(coin) path inside the drain branch is structurally identical to the quiet-hours equivalent, but is currently uncovered for drain. Optional — adding a single parametrized assertion would close the gap.

Suggestion: Add a test that sets close_immediately=True and asserts strategy.close_position is called when a position is open under drain.


3. test_exit_drain_resets_flag could verify quotes resume (nit)

tests/test_drain_mode.py L173–L196:

After removing the flag file, the test asserts _was_drain is False but does not check that normal quoting resumes (the more useful behavioural guarantee). Asserting bulk_place_orders.assert_called_once() after the second run() would catch a regression where the early-return is left in place by mistake.


Verdict: LGTM

No blockers. Two nits + one optional test gap, all non-blocking. Behaviour mirrors quiet_hours precisely so the risk surface is well understood. Ready to merge after CI confirmation.

@keitaj
keitaj merged commit a543104 into main Apr 26, 2026
6 checks passed
@keitaj
keitaj deleted the feat/session-switch-drain-mode branch April 26, 2026 23:50
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