Skip to content

feat: Add unrealized loss based early taker close#122

Merged
keitaj merged 1 commit into
mainfrom
feat/unrealized-loss-close
Apr 25, 2026
Merged

feat: Add unrealized loss based early taker close#122
keitaj merged 1 commit into
mainfrom
feat/unrealized-loss-close

Conversation

@keitaj

@keitaj keitaj commented Apr 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add unrealized loss threshold (--unrealized-loss-close-bps) to PositionCloser: when a position's unrealized loss exceeds the configured bps, it is immediately closed via taker order regardless of position age
  • Targets large-loss trades that accumulate while waiting for maker close or age-based force close
  • Default 0 (disabled) preserves full backward compatibility

Changes

File Change
strategies/mm_position_closer.py Add CLOSE_REASON_UNREALIZED_LOSS constant, unrealized_loss_close_bps parameter, and unrealized loss check in manage() before age-based force close
strategies/market_making_strategy.py Pass unrealized_loss_close_bps from config to PositionCloser
bot.py Add --unrealized-loss-close-bps CLI argument and wire it into strategy params
README.md Document the new parameter in strategy description and YAML reference
tests/test_unrealized_loss_close.py 9 test cases covering long/short, below/above threshold, profit, disabled, no market data, close reason logging, and existing close order cancellation

Test plan

  • All 9 new tests pass
  • Full test suite passes (753 tests)
  • flake8 lint clean
  • No regressions in existing tests

🤖 Generated with Claude Code

When a position's unrealized loss exceeds a configurable threshold (in bps),
PositionCloser triggers an immediate taker close instead of waiting for the
age-based force-close. This caps large adverse moves that currently account
for 55% of total losses. Default: 0 (disabled, backward compatible).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@keitaj keitaj changed the title feat: Add unrealized loss based early taker close (#122) feat: Add unrealized loss based early taker close Apr 25, 2026

@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: Unrealized loss early taker close

Clean, well-scoped feature with solid test coverage. The implementation follows existing patterns in PositionCloser correctly, backward compatibility is preserved via default=0, and the bps calculation is correct for both long and short positions.


1. Missing _consecutive_close_failures cleanup (nit)

strategies/mm_position_closer.py L253-256:

When the unrealized loss path closes a position, it pops _open_positions but does not clean up _consecutive_close_failures — unlike the force-close path (L342, L372) and cleanup_closed() (L161) which both do.

self._record_close(coin, CLOSE_REASON_UNREALIZED_LOSS, age, current_tier)
close_position_fn(coin)
self._open_positions.pop(coin, None)
return

The stale entry gets cleaned up on the next successful close placement for that coin, so this is not a functional bug — just an inconsistency with the other close paths.

Suggestion:

self._record_close(coin, CLOSE_REASON_UNREALIZED_LOSS, age, current_tier)
close_position_fn(coin)
self._open_positions.pop(coin, None)
self._consecutive_close_failures.pop(coin, None)
return

Verdict: LGTM

Single nit (non-blocking). Logic is correct, risk guards untouched, tests are thorough (9 cases covering all edge cases), and CLI/config wiring is complete. Ready to merge after CI passes.

@keitaj
keitaj merged commit 238d1a1 into main Apr 25, 2026
6 checks passed
@keitaj
keitaj deleted the feat/unrealized-loss-close branch April 25, 2026 08:37
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