Skip to content

fix: Add _open_positions guard in MarketMakingStrategy.close_position()#117

Merged
keitaj merged 1 commit into
mainfrom
fix/base-strategy-close-guard
Apr 22, 2026
Merged

fix: Add _open_positions guard in MarketMakingStrategy.close_position()#117
keitaj merged 1 commit into
mainfrom
fix/base-strategy-close-guard

Conversation

@keitaj

@keitaj keitaj commented Apr 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Override close_position() in MarketMakingStrategy to check _closer._open_positions before sending reduce_only order
  • Closes the remaining race condition gap: PR fix: Add final _open_positions guard before all reduce_only orders #116 guarded the PositionCloser paths but not the BaseStrategy close path (used by close_immediately=True and QUIET hours)
  • _open_positions is updated by FillFeed on the WS thread with zero cache delay, more reliable than the 2-second TTL position cache

Changes

  • strategies/market_making_strategy.py: Add close_position() override with _open_positions guard
  • tests/test_close_position_guard.py: 3 new tests (skip when not tracked, proceed when tracked, skip for untracked coin)
  • tests/test_reduce_only_force_close.py: Add _closer mock with _open_positions to existing test helper

Test plan

  • Unit tests added (3 new)
  • flake8 passes
  • pytest passes (712 tests)
  • No regressions

🤖 Generated with Claude Code

…() (#117)

Co-Authored-By: Claude Opus 4.6 (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: close_position() _open_positions guard

Clean 11-line override that closes the last remaining reduce_only race condition gap. PR #116 covered PositionCloser paths; this covers the BaseStrategy close path used by close_immediately and QUIET hours.

The guard is consistent with the pattern established in PR #116: check _open_positions (updated instantly by FillFeed) rather than relying on the 2-second TTL cache.

Verdict: LGTM

Minimal change, correct logic, good test coverage. Ship it.

@keitaj
keitaj merged commit 8a655c6 into main Apr 22, 2026
5 checks passed
@keitaj
keitaj deleted the fix/base-strategy-close-guard branch April 22, 2026 23:23
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