feat: Add Stop-Loss and Risk Management System (v0.3.1)#8
Conversation
- Implement RiskManager with real-time position monitoring and multiple stop-loss types - Add AutoExecutor for automated risk response and emergency stops - Integrate risk monitoring into WebSocket client for live price updates - Enhance trading client and portfolio with risk-aware order placement - Add risk simulation to backtesting engine - Include comprehensive documentation, examples, and test suite - Bump version to 0.3.1
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on November 1. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds a comprehensive risk management system (v0.3.1) to the Neural SDK, introducing real-time stop-loss monitoring, automated position exits, and configurable risk limits. The implementation centers on three new classes: RiskManager (position tracking, risk limit enforcement), StopLossEngine (stop price calculation strategies), and AutoExecutor (automated order execution in response to risk events). The system integrates into the existing architecture through optional parameters in TradingClient, PaperPortfolio, KalshiWebSocketClient, and Backtester, ensuring backward compatibility. The WebSocket client now processes incoming price updates through the risk manager, triggering automated responses when thresholds are breached. The change is designed as an opt-in layer—existing code continues to work unchanged, while users who instantiate components with a risk_manager parameter gain real-time risk control. Supporting documentation, a working example, and a test suite demonstrate the feature's intended usage patterns.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
neural/analysis/risk/risk_manager.py |
2/5 | Adds core risk management classes with critical bugs in trailing stop logic for short positions and incomplete portfolio value calculation |
neural/analysis/execution/auto_executor.py |
2/5 | Introduces automated order execution with inverted stop-loss side logic and memory leak in rate limiter |
neural/trading/websocket.py |
3/5 | Integrates real-time risk monitoring into WebSocket message handler with type-safety and error-handling gaps |
neural/trading/client.py |
2/5 | Adds risk-aware order placement method with unverified API calls and silent failure handling |
tests/test_risk_management.py |
2/5 | Adds test suite with direct state mutation, conflicting mocks, and missing assertions |
docs/analysis/risk-management.mdx |
2/5 | Creates comprehensive risk management guide with code examples containing undefined classes and missing parameters |
examples/risk_management_example.py |
4/5 | Demonstrates risk system with minor API boundary violations and calls to private methods |
neural/analysis/backtesting/engine.py |
3/5 | Adds risk_manager parameter but does not integrate it into backtest execution logic |
docs/analysis/overview.mdx |
3.5/5 | Updates module descriptions with minor inaccuracy attributing automated execution to wrong module |
neural/analysis/risk/__init__.py |
5/5 | Exports new risk management classes following standard Python packaging patterns |
neural/analysis/execution/__init__.py |
5/5 | Exposes AutoExecutor and ExecutionConfig in public API |
neural/trading/paper_portfolio.py |
4/5 | Adds optional risk_manager parameter with weak typing and no internal integration |
neural/__init__.py |
5/5 | Bumps version to 0.3.1 in sync with package metadata |
pyproject.toml |
5/5 | Updates version number to 0.3.1 for new risk management release |
CHANGELOG.md |
5/5 | Documents v0.3.1release with comprehensive feature descriptions |
Confidence score: 2/5
- This PR has critical bugs that will cause incorrect behavior in production trading environments and should not be merged without fixes
- Score reflects fundamental logic errors in stop-loss execution (inverted order sides), trailing stop calculations (only tracking upward movement for all positions), incomplete portfolio value calculation, undefined references in documentation (
ExecutionConfig), unverified trading client API assumptions, and test suite issues with missing assertions and state mutation - Pay close attention to
neural/analysis/risk/risk_manager.py(lines 145-148, 215-222,269-276, 349-360),neural/analysis/execution/auto_executor.py(lines 131-142, 198-205, 240-252),neural/trading/client.py(lines 159-161, 169-179),docs/analysis/risk-management.mdx(lines35, 156, 214), andtests/test_risk_management.py(lines 113-130, 137-145, 176-179)
Sequence Diagram
sequenceDiagram
participant User
participant TradingClient
participant RiskManager
participant Position
participant WebSocketClient
participant AutoExecutor
participant KalshiAPI
User->>TradingClient: "place_order_with_risk(market_id, side, quantity, stop_loss_config)"
TradingClient->>KalshiAPI: "create_order(market_id, side, quantity)"
KalshiAPI-->>TradingClient: "order_result"
TradingClient->>Position: "new Position(market_id, side, quantity, entry_price, stop_loss_config)"
TradingClient->>RiskManager: "add_position(position)"
RiskManager-->>TradingClient: "position registered"
TradingClient-->>User: "order_result"
User->>WebSocketClient: "connect()"
WebSocketClient->>KalshiAPI: "WebSocket handshake with signed headers"
KalshiAPI-->>WebSocketClient: "connection established"
WebSocketClient->>WebSocketClient: "subscribe(['market_price'])"
loop Real-time Price Updates
KalshiAPI->>WebSocketClient: "market_price message"
WebSocketClient->>WebSocketClient: "_process_risk_monitoring(payload)"
WebSocketClient->>RiskManager: "update_position_price(market_id, new_price)"
RiskManager->>Position: "update current_price"
RiskManager->>RiskManager: "_check_stop_loss(position)"
alt Stop-Loss Triggered
RiskManager->>AutoExecutor: "on_risk_event(STOP_LOSS_TRIGGERED, position, data)"
AutoExecutor->>AutoExecutor: "_check_rate_limit()"
AutoExecutor->>AutoExecutor: "_execute_market_order(market_id, side, quantity)"
AutoExecutor->>TradingClient: "place_order (close position)"
TradingClient->>KalshiAPI: "create_order (opposite side)"
KalshiAPI-->>TradingClient: "close_order_result"
AutoExecutor->>RiskManager: "remove_position(market_id)"
end
RiskManager->>RiskManager: "_check_drawdown_limit()"
alt Max Drawdown Exceeded
RiskManager->>AutoExecutor: "on_risk_event(MAX_DRAWDOWN_EXCEEDED, position, data)"
end
RiskManager->>RiskManager: "_check_daily_loss_limit()"
alt Daily Loss Limit Exceeded
RiskManager->>AutoExecutor: "on_risk_event(DAILY_LOSS_LIMIT_EXCEEDED, 'all', data)"
AutoExecutor->>AutoExecutor: "_emergency_stop(data)"
AutoExecutor->>AutoExecutor: "disable auto_execution"
loop Cancel All Orders
AutoExecutor->>TradingClient: "cancel_order(order_id)"
end
end
end
User->>RiskManager: "get_risk_metrics()"
RiskManager-->>User: "portfolio_value, drawdown_pct, daily_pnl, open_positions"
Additional Comments (2)
-
neural/analysis/backtesting/engine.py, line 181-208 (link)logic: Position exits are only checked via
strategy.should_close_position(). If a risk_manager is provided, it should also be consulted here to trigger stop-loss exits based on risk rules. -
neural/analysis/backtesting/engine.py, line 213-252 (link)logic: New positions are opened without consulting the risk_manager. If risk_manager is set, it should validate position size, check max drawdown limits, and verify position count before allowing entry.
15 files reviewed, 29 comments
| commission: float = 0.0, # Additional commission if any | ||
| initial_capital: float = 1000.0, | ||
| max_workers: int = 4, | ||
| risk_manager: Any = None, # Optional risk manager for stop-loss simulation |
There was a problem hiding this comment.
logic: Risk manager parameter added but never used in the backtesting logic. Should either integrate risk checks during backtesting (e.g., simulate stop-losses, position limits) or remove if not needed. Should the risk_manager simulate stop-losses and position limits during the backtest, or is this parameter intended for future use?
Prompt To Fix With AI
This is a comment left during a code review.
Path: neural/analysis/backtesting/engine.py
Line: 88:88
Comment:
**logic:** Risk manager parameter added but never used in the backtesting logic. Should either integrate risk checks during backtesting (e.g., simulate stop-losses, position limits) or remove if not needed. Should the risk_manager simulate stop-losses and position limits during the backtest, or is this parameter intended for future use?
How can I resolve this? If you propose a fix, please make it concise.
docs/analysis/risk-management.mdx
Outdated
| ) | ||
|
|
||
| # Create automated executor | ||
| executor = AutoExecutor(trading_client=TradingClient(), risk_manager=risk_manager) |
There was a problem hiding this comment.
syntax: missing risk_manager parameter in AutoExecutor initialization – executor won't receive risk events
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/analysis/risk-management.mdx
Line: 35:35
Comment:
**syntax:** missing `risk_manager` parameter in `AutoExecutor` initialization – executor won't receive risk events
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| executor = AutoExecutor( | ||
| trading_client=client, | ||
| config=ExecutionConfig( |
There was a problem hiding this comment.
logic: ExecutionConfig is imported/defined nowhere in the codebase – this code will fail at runtime
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/analysis/risk-management.mdx
Line: 156:156
Comment:
**logic:** `ExecutionConfig` is imported/defined nowhere in the codebase – this code will fail at runtime
How can I resolve this? If you propose a fix, please make it concise.
docs/analysis/risk-management.mdx
Outdated
|
|
||
| ```python | ||
| # Manual emergency stop | ||
| executor._emergency_stop({"reason": "manual_override"}) |
There was a problem hiding this comment.
style: calling a private method (_emergency_stop) in user-facing documentation is fragile – consider exposing a public API
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/analysis/risk-management.mdx
Line: 214:214
Comment:
**style:** calling a private method (`_emergency_stop`) in user-facing documentation is fragile – consider exposing a public API
How can I resolve this? If you propose a fix, please make it concise.
examples/risk_management_example.py
Outdated
| )() | ||
|
|
||
| # This should trigger position size limit | ||
| events = self.risk_manager._check_position_size_limit(large_position) |
There was a problem hiding this comment.
style: calling private method _check_position_size_limit from example code; use public API instead or remove underscore prefix from method name in risk_manager.py
Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/risk_management_example.py
Line: 175:175
Comment:
**style:** calling private method `_check_position_size_limit` from example code; use public API instead or remove underscore prefix from method name in risk_manager.py
How can I resolve this? If you propose a fix, please make it concise.
neural/analysis/risk/risk_manager.py
Outdated
| # Update trailing stop high if applicable | ||
| if position.stop_loss and position.stop_loss.type == StopLossType.TRAILING: | ||
| if current_price > position.trailing_high: | ||
| position.trailing_high = current_price |
There was a problem hiding this comment.
logic: Trailing stop logic only tracks upward price movement. For 'no' side (short positions), should track downward movement (trailing_low) instead. Should trailing stops for 'no' side positions track the lowest price instead of the highest?
Prompt To Fix With AI
This is a comment left during a code review.
Path: neural/analysis/risk/risk_manager.py
Line: 145:148
Comment:
**logic:** Trailing stop logic only tracks upward price movement. For 'no' side (short positions), should track downward movement (trailing_low) instead. Should trailing stops for 'no' side positions track the lowest price instead of the highest?
How can I resolve this? If you propose a fix, please make it concise.
neural/analysis/risk/risk_manager.py
Outdated
| elif stop_loss.type == StopLossType.TRAILING: | ||
| stop_price = position.trailing_high - stop_loss.value | ||
| if position.current_price <= stop_price: | ||
| _LOG.warning( | ||
| f"Trailing stop-loss triggered for {position.market_id}: " | ||
| f"price {position.current_price} <= {stop_price}" | ||
| ) | ||
| return True |
There was a problem hiding this comment.
logic: Trailing stop logic ignores position side. For 'no' positions, stop should trigger when price falls below trailing_low + value, not when it falls below trailing_high - value.
Prompt To Fix With AI
This is a comment left during a code review.
Path: neural/analysis/risk/risk_manager.py
Line: 215:222
Comment:
**logic:** Trailing stop logic ignores position side. For 'no' positions, stop should trigger when price falls below trailing_low + value, not when it falls below trailing_high - value.
How can I resolve this? If you propose a fix, please make it concise.
neural/analysis/risk/risk_manager.py
Outdated
| def _update_portfolio_value(self) -> None: | ||
| """Update total portfolio value from positions.""" | ||
| total_value = 0.0 | ||
| for position in self.positions.values(): | ||
| total_value += position.current_value | ||
|
|
||
| self.portfolio_value = total_value | ||
| self.peak_portfolio_value = max(self.peak_portfolio_value, self.portfolio_value) |
There was a problem hiding this comment.
logic: Portfolio value calculation excludes cash/unrealized positions. If portfolio_value represents total capital, it should include initial cash balance, not just sum of position values. Should portfolio_value include the initial cash balance or only the current value of open positions?
Prompt To Fix With AI
This is a comment left during a code review.
Path: neural/analysis/risk/risk_manager.py
Line: 269:276
Comment:
**logic:** Portfolio value calculation excludes cash/unrealized positions. If portfolio_value represents total capital, it should include initial cash balance, not just sum of position values. Should portfolio_value include the initial cash balance or only the current value of open positions?
How can I resolve this? If you propose a fix, please make it concise.| def _trailing_stop( | ||
| self, entry_price: float, current_price: float, side: str, trail_pct: float = 0.03 | ||
| ) -> float: | ||
| """Trailing stop-loss that follows favorable price movement.""" | ||
| if side == "yes": | ||
| # For long positions, trail below the highest price | ||
| trail_amount = current_price * trail_pct | ||
| return current_price - trail_amount | ||
| else: # "no" | ||
| # For short positions, trail above the lowest price | ||
| trail_amount = current_price * trail_pct | ||
| return current_price + trail_amount |
There was a problem hiding this comment.
logic: Trailing stop for 'no' side uses current_price + trail_amount, but should use the lowest price seen (trailing_low) + trail_amount for proper trailing behavior.
Prompt To Fix With AI
This is a comment left during a code review.
Path: neural/analysis/risk/risk_manager.py
Line: 349:360
Comment:
**logic:** Trailing stop for 'no' side uses current_price + trail_amount, but should use the lowest price seen (trailing_low) + trail_amount for proper trailing behavior.
How can I resolve this? If you propose a fix, please make it concise.| stop_price = self.calculate_stop_price( | ||
| position.entry_price, | ||
| position.current_price, | ||
| position.side, | ||
| strategy=self._map_stop_type_to_strategy(position.stop_loss.type), | ||
| stop_pct=position.stop_loss.value | ||
| if position.stop_loss.type == StopLossType.PERCENTAGE | ||
| else 0.05, | ||
| trail_pct=position.stop_loss.value | ||
| if position.stop_loss.type == StopLossType.TRAILING | ||
| else 0.03, | ||
| volatility=market_volatility, | ||
| time_held=current_time - position.entry_time, | ||
| ) |
There was a problem hiding this comment.
logic: stop_pct and trail_pct default values (0.05, 0.03) override position.stop_loss.value when type doesn't match. Should pass position.stop_loss.value directly or handle all types explicitly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: neural/analysis/risk/risk_manager.py
Line: 410:423
Comment:
**logic:** stop_pct and trail_pct default values (0.05, 0.03) override position.stop_loss.value when type doesn't match. Should pass position.stop_loss.value directly or handle all types explicitly.
How can I resolve this? If you propose a fix, please make it concise.- Integrate risk_manager into backtesting engine with stop-loss simulation - Fix AutoExecutor documentation examples and add missing imports - Correct trailing stop percentage calculation in RiskManager - Add public emergency_stop method to AutoExecutor - Fix websocket risk monitoring test implementation - Update documentation to use public APIs instead of private methods
- Fix typo in pr-docs.yml: validate_example_docs.py -> validate_examples.py - This resolves the failing Documentation Check in CI
- Auto-fix 10 linter issues with ruff --fix - Remove unused 'metrics' variable in risk management example - Rename unused 'order_info' loop variable to '_order_info' in auto_executor - Format 2 files with black: risk_manager.py and backtesting/engine.py - All ruff, black, and mypy checks now pass - Tests still pass after fixes
- Corrected quote consistency and operator spacing in f-strings - Adjusted line breaks for long expressions and function calls - Ensured compliance with black and ruff formatting rules
…des, portfolio calc, docs, tests
1 participant
Summary
Successfully implemented a comprehensive stop-loss and risk management system for Neural SDK v0.3.1. Key components delivered:
RiskManagerclass with real-time position monitoring, multiple stop-loss types (percentage, absolute, trailing), and configurable risk limits (max drawdown, position size, daily losses)StopLossEnginewith sophisticated strategies including volatility-adjusted and time-based stopsKalshiWebSocketClientwith real-time risk monitoring that processes price updates and triggers automated responsesAutoExecutorclass that handles risk events with immediate order generation, emergency stops, and rate limitingTradingClientand enhanced portfolio management🔄 Currently Being Worked On
📁 Files Modified/Created
Core Implementation:
neural/analysis/risk/risk_manager.py- Main risk management classesneural/analysis/risk/__init__.py- Module exportsneural/analysis/execution/auto_executor.py- Automated execution layerneural/analysis/execution/__init__.py- Module exportsneural/trading/websocket.py- WebSocket risk integrationneural/trading/client.py- Risk-aware trading clientneural/trading/paper_portfolio.py- Portfolio risk integrationneural/analysis/backtesting/engine.py- Backtesting risk simulationDocumentation & Examples:
docs/analysis/overview.mdx- Updated module overviewdocs/analysis/risk-management.mdx- Comprehensive risk management guideexamples/risk_management_example.py- Working demonstrationCHANGELOG.md- v0.3.1 release notesConfiguration & Tests:
pyproject.toml&neural/__init__.py- Version bump to 0.3.1tests/test_risk_management.py- Comprehensive test suite🎯 What Needs to Be Done Next
The implementation is functionally complete and ready for production, providing essential real-time risk control for websocket-based trading operations. All major components are tested and documented, with the system designed to be backward-compatible and non-disruptive to existing code.