Skip to content

Commit e0cf83d

Browse files
committed
fix: resolve all remaining type issues in OrderManager
- Remove duplicate protective_side declarations in bracket_orders.py - Add _unlink_oco_orders to OrderManagerProtocol for proper type checking - Fix union type handling in position_orders.py with proper type narrowing - Clean up unused parameters and improve type annotations in tracking.py - Fix orderbook test to use synchronous get_memory_stats() (v3.3.0 compat) All 8 critical issues from PR #51 are now resolved: ✅ Race condition in bracket orders - Fixed with proper async synchronization ✅ Memory leaks in tracking - Fixed with TTL caches and cleanup tasks ✅ Deadlock potential - Fixed with single lock ordering ✅ Price precision - Fixed with Decimal arithmetic ✅ Order state validation - Fixed with comprehensive validation ✅ Tick size validation - Fixed with proper price alignment ✅ Error recovery - Fixed with complete recovery system ✅ Event handler robustness - Fixed with proper data validation Tests: All 33 OrderManager tests passing Type checking: Zero mypy errors Linting: All ruff checks pass IDE diagnostics: Zero issues
1 parent 20ccb80 commit e0cf83d

File tree

5 files changed

+30
-26
lines changed

5 files changed

+30
-26
lines changed

src/project_x_py/order_manager/bracket_orders.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ async def main():
8383
from typing import TYPE_CHECKING, Any
8484

8585
from project_x_py.exceptions import ProjectXOrderError
86-
from project_x_py.models import BracketOrderResponse
86+
from project_x_py.models import BracketOrderResponse, OrderPlaceResponse
8787
from project_x_py.utils.error_handler import retry_on_network_error
8888

8989
from .error_recovery import (
@@ -247,7 +247,7 @@ async def place_bracket_order(
247247
ProjectXOrderError: If bracket order validation or placement fails completely
248248
"""
249249
# Initialize recovery manager for this operation (if available)
250-
recovery_manager = self._get_recovery_manager()
250+
recovery_manager: OperationRecoveryManager | None = self._get_recovery_manager()
251251
operation: RecoveryOperation | None = None
252252

253253
if recovery_manager:
@@ -299,6 +299,10 @@ async def place_bracket_order(
299299
entry_ref: OrderReference | None = None
300300
stop_ref: OrderReference | None = None
301301
target_ref: OrderReference | None = None
302+
303+
# Determine protective order side (opposite of entry)
304+
protective_side: int = 1 if side == 0 else 0
305+
302306
if recovery_manager and operation:
303307
entry_ref = await recovery_manager.add_order_to_operation(
304308
operation,
@@ -309,9 +313,6 @@ async def place_bracket_order(
309313
entry_price if entry_type.lower() != "market" else None,
310314
)
311315

312-
# Determine protective order side (opposite of entry)
313-
protective_side = 1 if side == 0 else 0
314-
315316
stop_ref = await recovery_manager.add_order_to_operation(
316317
operation,
317318
contract_id,
@@ -329,11 +330,9 @@ async def place_bracket_order(
329330
"target",
330331
take_profit_price,
331332
)
332-
else:
333-
# Determine protective order side (opposite of entry)
334-
protective_side = 1 if side == 0 else 0
335333

336334
# Place entry order
335+
entry_response: OrderPlaceResponse
337336
if entry_type.lower() == "market":
338337
entry_response = await self.place_market_order(
339338
contract_id, side, size, account_id
@@ -460,7 +459,7 @@ async def place_bracket_order(
460459
try:
461460
# Place stop loss order
462461
logger.debug(f"Placing stop loss at {stop_loss_price}")
463-
stop_response = await self.place_stop_order(
462+
stop_response: OrderPlaceResponse = await self.place_stop_order(
464463
contract_id, protective_side, size, stop_loss_price, account_id
465464
)
466465

@@ -486,7 +485,7 @@ async def place_bracket_order(
486485

487486
# Place take profit order
488487
logger.debug(f"Placing take profit at {take_profit_price}")
489-
target_response = await self.place_limit_order(
488+
target_response: OrderPlaceResponse = await self.place_limit_order(
490489
contract_id, protective_side, size, take_profit_price, account_id
491490
)
492491

src/project_x_py/order_manager/position_orders.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,9 @@ async def cancel_position_orders(
409409
order_types = ["entry", "stop", "target"]
410410

411411
position_orders = self.get_position_orders(contract_id)
412-
results = {"entry": 0, "stop": 0, "target": 0, "failed": 0, "errors": []}
412+
# Track successful cancellations by type
413+
success_counts = {"entry": 0, "stop": 0, "target": 0, "failed": 0}
414+
error_messages: list[str] = []
413415

414416
# Track cancellation attempts and failures for better recovery
415417
failed_cancellations = []
@@ -421,7 +423,7 @@ async def cancel_position_orders(
421423
try:
422424
success = await self.cancel_order(order_id, account_id)
423425
if success:
424-
results[order_type] += 1
426+
success_counts[order_type] += 1
425427
self.untrack_order(order_id)
426428
logger.debug(
427429
f"Successfully cancelled {order_type} order {order_id}"
@@ -432,7 +434,7 @@ async def cancel_position_orders(
432434
f"Cancellation of {order_type} order {order_id} returned False - "
433435
f"order may be filled or already cancelled"
434436
)
435-
results["failed"] += 1
437+
success_counts["failed"] += 1
436438
failed_cancellations.append(
437439
{
438440
"order_id": order_id,
@@ -446,8 +448,8 @@ async def cancel_position_orders(
446448
f"Failed to cancel {order_type} order {order_id}: {e}"
447449
)
448450
logger.error(error_msg)
449-
results["failed"] += 1
450-
results["errors"].append(error_msg)
451+
success_counts["failed"] += 1
452+
error_messages.append(error_msg)
451453
failed_cancellations.append(
452454
{
453455
"order_id": order_id,
@@ -460,16 +462,12 @@ async def cancel_position_orders(
460462
total_attempted = sum(
461463
len(position_orders.get(f"{ot}_orders", [])) for ot in order_types
462464
)
463-
total_successful = sum(
464-
results[ot]
465-
for ot in order_types
466-
if ot in results and isinstance(results[ot], int)
467-
)
465+
total_successful = sum(success_counts[ot] for ot in order_types)
468466

469467
if total_attempted > 0:
470468
logger.info(
471469
f"Position order cancellation for {contract_id}: "
472-
f"{total_successful}/{total_attempted} successful, {results['failed']} failed"
470+
f"{total_successful}/{total_attempted} successful, {success_counts['failed']} failed"
473471
)
474472

475473
if failed_cancellations:
@@ -478,6 +476,11 @@ async def cancel_position_orders(
478476
f"Manual verification may be required."
479477
)
480478

479+
# Return results in expected format
480+
results: dict[str, int | list[str]] = {
481+
**success_counts,
482+
"errors": error_messages,
483+
}
481484
return results
482485

483486
async def update_position_order_sizes(

src/project_x_py/order_manager/tracking.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def on_order_fill(order_data):
5353
import time
5454
from collections import defaultdict, deque
5555
from collections.abc import Callable
56-
from typing import TYPE_CHECKING, Any
56+
from typing import TYPE_CHECKING, Any, cast
5757

5858
from cachetools import TTLCache # type: ignore
5959

@@ -502,12 +502,12 @@ def _extract_order_data(
502502
logger.error(f"Error extracting order data from {type(raw_data)}: {e}")
503503
return None
504504

505-
def _validate_order_data(self, order_data: dict[str, Any]) -> dict[str, Any] | None:
505+
def _validate_order_data(self, order_data: Any) -> dict[str, Any] | None:
506506
"""
507507
Validate and sanitize order data structure.
508508
509509
Args:
510-
order_data: Dictionary containing order information
510+
order_data: Raw order information (validated to be dict)
511511
512512
Returns:
513513
Validated order data or None if invalid
@@ -754,7 +754,8 @@ async def cancel_oco_order() -> None:
754754

755755
# Clean up OCO group using safe unlinking
756756
if hasattr(self, "_unlink_oco_orders"):
757-
linked_id = self._unlink_oco_orders(
757+
manager = cast("OrderManagerProtocol", self)
758+
linked_id = manager._unlink_oco_orders(
758759
order_id_int
759760
)
760761
if linked_id != other_order_id:

src/project_x_py/types/protocols.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ async def _wait_for_order_fill(
357357
self, order_id: int, timeout_seconds: int = 30
358358
) -> bool: ...
359359
def _link_oco_orders(self, order1_id: int, order2_id: int) -> None: ...
360+
def _unlink_oco_orders(self, order_id: int) -> int | None: ...
360361
async def _check_order_fill_status(
361362
self, order_id: int
362363
) -> tuple[bool, int, int]: ...

tests/orderbook/test_realtime_simplified.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ async def test_get_memory_stats(self, orderbook):
203203
ob = orderbook
204204

205205
# Get memory stats
206-
stats = await ob.get_memory_stats()
206+
stats = ob.get_memory_stats()
207207

208208
# Should return memory stats
209209
assert stats is not None

0 commit comments

Comments
 (0)