Skip to content

feat(limitless): implement WebSocket support for real-time market data#27

Merged
guzus merged 5 commits intomainfrom
feat/add-limitless-websocket
Dec 22, 2025
Merged

feat(limitless): implement WebSocket support for real-time market data#27
guzus merged 5 commits intomainfrom
feat/add-limitless-websocket

Conversation

@0xghXst
Copy link
Collaborator

@0xghXst 0xghXst commented Dec 21, 2025

  • Introduced LimitlessWebSocket and LimitlessUserWebSocket classes for handling real-time updates.
  • Added methods for subscribing to orderbook and price updates, as well as position tracking for authenticated users.
  • Enhanced the Limitless exchange to support WebSocket connections, allowing for real-time orderbook and price data retrieval.
  • Updated the strategy cleanup process to include cancellation of orders and liquidation of positions.
  • Added tests for WebSocket functionality, ensuring proper callback registration and data parsing.
  • Updated documentation to reflect new WebSocket capabilities and usage examples.

- Introduced LimitlessWebSocket and LimitlessUserWebSocket classes for handling real-time updates.
- Added methods for subscribing to orderbook and price updates, as well as position tracking for authenticated users.
- Enhanced the Limitless exchange to support WebSocket connections, allowing for real-time orderbook and price data retrieval.
- Updated the strategy cleanup process to include cancellation of orders and liquidation of positions.
- Added tests for WebSocket functionality, ensuring proper callback registration and data parsing.
- Updated documentation to reflect new WebSocket capabilities and usage examples.
…Socket and Limitless classes

- Consolidated list comprehensions for no bids and no asks in LimitlessWebSocket.
- Simplified inverted bids and asks creation in Limitless class for better readability.
- Improved code clarity by reducing line count while maintaining functionality.
@claude
Copy link

claude bot commented Dec 21, 2025

Code Review: WebSocket Support for Limitless Exchange

Summary

This PR introduces comprehensive WebSocket support for the Limitless exchange, enabling real-time market data updates. The implementation is well-structured and follows good practices, but there are several areas that need attention.


Critical Issues

1. Price Normalization Logic Inconsistency (limitless.py:374-393)

The price normalization logic attempts to handle both 0-100 and 0-1 ranges, but the implementation is flawed:

# Current implementation
yes_price = float(data.get("yesPrice", 0) or 0)
no_price = float(data.get("noPrice", 0) or 0)
prices["Yes"] = yes_price / 100 if yes_price > 1 else yes_price
prices["No"] = no_price / 100 if no_price > 1 else no_price

Problem: A price of exactly 1.0 would not be normalized, which could be either 100% (needs /100) or already normalized. This creates ambiguity.

Recommendation:

  • Determine the API's actual format through testing
  • Use a consistent threshold (e.g., >= 1 or > 1.5)
  • Add validation to ensure prices sum to ~1.0 after normalization
  • Add logging when normalization occurs to catch edge cases

2. Hardcoded Rounding Precision (limitless.py:508, limitless_ws.py:543-546)

{"price": str(round(1 - float(a["price"]), 3)), "size": a["size"]}

Problem: The 3-decimal rounding is hardcoded and may lose precision for markets with different tick sizes.

Recommendation: Use market-specific tick_size for rounding:

rounded_price = round(1 - price, -int(math.log10(tick_size)))

3. Unsafe Session Cookie Extraction (limitless.py:1585-1590)

session_cookie = None
for cookie in self._session.cookies:
    if cookie.name == "limitless_session":
        session_cookie = cookie.value
        break

Problem: No domain/path validation. Cookies from unrelated domains could be used.

Recommendation: Add domain validation:

for cookie in self._session.cookies:
    if cookie.name == "limitless_session" and cookie.domain in [".limitless.exchange", "limitless.exchange"]:
        session_cookie = cookie.value
        break

Security Concerns

4. Potential Race Condition in Event Loop Management (limitless_ws.py:475-493)

The start() method creates a new event loop without checking if one exists:

self.loop = asyncio.new_event_loop()

Problem: If called multiple times, this creates multiple event loops and threads, potentially leading to resource leaks.

Recommendation: Add state checking:

if self.state != WebSocketState.DISCONNECTED:
    raise RuntimeError("WebSocket already started")
if self._thread and self._thread.is_alive():
    return self._thread

5. Silent Error Handling in Callbacks (limitless_ws.py:226-230)

for callback in self._error_callbacks:
    try:
        callback(error_msg)
    except Exception:
        pass  # Silently ignores exceptions

Problem: Exceptions in error handlers are completely suppressed, making debugging difficult.

Recommendation: At minimum, log the exception:

except Exception as e:
    if self.verbose:
        logger.error(f"Error callback raised exception: {e}")

Code Quality Issues

6. Inconsistent API Limit Documentation (limitless.py:291-295)

# Comment says "API max is 25"
limit = min(query_params.get("limit", 25), 25)

But the docstring says:

- limit: Items per page (default 50)

Problem: Docstring and code don't match. Users will be confused.

Recommendation: Update docstring to reflect actual behavior (default 25, max 25).

7. Missing Type Hints in Critical Methods (limitless_ws.py:237-291)

The parsing methods lack return type hints:

def _parse_orderbook_update(self, data: Dict[str, Any]) -> Optional[OrderbookUpdate]:

Good, but callback parameters are missing:

def on_orderbook(self, callback: Callable[[OrderbookUpdate], None])  # Missing -> return type

Recommendation: Add complete type hints for better IDE support and type checking.

8. Duplicate Sorting Logic (limitless.py:500-502 and 508-518)

Orderbook sorting happens twice unnecessarily:

# First sort
bids.sort(key=lambda x: float(x["price"]), reverse=True)
asks.sort(key=lambda x: float(x["price"]))

# Then inverted orderbook gets re-sorted
inverted_bids.sort(key=lambda x: float(x["price"]), reverse=True)

Recommendation: Sort only after all processing (inversion, etc.) is complete.


Performance Considerations

9. Inefficient Token-to-Slug Mapping (limitless.py:428-434)

Every market parsing updates the global mapping:

for token_id in token_ids:
    if token_id:
        self._token_to_slug[token_id] = slug

Problem: For large market lists (hundreds of markets), this creates a growing dict that's never cleaned up.

Recommendation:

  • Add an LRU cache with size limit
  • Consider using functools.lru_cache on a method instead of instance dict

10. Blocking Sleep in Cleanup (strategy.py:577)

time.sleep(3)  # Wait for liquidation orders to fill

Problem: Blocks the entire thread for 3 seconds, which is arbitrary and may be too short or too long.

Recommendation: Poll order status with exponential backoff:

for _ in range(10):  # Try up to 10 times
    remaining_orders = self.client.fetch_open_orders(market_id=self.market_id)
    if not remaining_orders:
        break
    time.sleep(0.5)

Testing Gaps

11. Missing Edge Case Tests

The test suite is comprehensive but lacks:

  • WebSocket reconnection scenarios
  • Concurrent callback execution
  • Orderbook inversion edge cases (price = 0.5)
  • Price normalization boundary conditions (price = 1.0, 1.5, 99, 100)

Recommendation: Add integration tests for:

def test_price_normalization_boundary_cases():
    # Test price = 1.0, 0.99, 1.01, 50, 99, 100
    pass

def test_websocket_reconnection():
    # Test automatic resubscription after disconnect
    pass

12. No Test for WebSocket State Transitions

The WebSocketState enum is defined but state transitions aren't tested.

Recommendation: Add tests verifying state machine behavior:

def test_websocket_state_transitions():
    ws = LimitlessWebSocket()
    assert ws.state == WebSocketState.DISCONNECTED
    # Connect and verify state changes

Best Practice Violations

13. Magic Numbers Without Constants (limitless.py:508)

{"price": str(round(1 - float(a["price"]), 3)), "size": a["size"]}

Problem: The number 3 appears without explanation.

Recommendation: Define as constant:

DEFAULT_PRICE_PRECISION = 3  # Decimal places for price rounding

14. Violation of Single Source of Truth (CLAUDE.md #4)

The codebase guidelines state: "Single Source of Truth: DO NOT place many variables in .env file. Place them in the code instead."

While this PR doesn't add .env variables, the WebSocket URL is a constant that could benefit from being configurable:

WS_URL = "wss://ws.limitless.exchange"  # Hardcoded

Recommendation: While acceptable as a constant, consider if test environments need different URLs.


Documentation Issues

15. Incomplete WebSocket Documentation

The wiki/exchanges/limitless.md update mentions WebSocket support but lacks examples:

Recommendation: Add WebSocket usage examples:

### WebSocket Example

\`\`\`python
from dr_manhattan.exchanges.limitless import Limitless

exchange = Limitless()
ws = exchange.get_websocket()

def on_orderbook(update):
    print(f"Orderbook update for {update.slug}")
    print(f"Best bid: {update.bids[0] if update.bids else 'None'}")
    print(f"Best ask: {update.asks[0] if update.asks else 'None'}")

ws.on_orderbook(on_orderbook)
ws.subscribe_market("btc-above-100k")
ws.start()
\`\`\`

Positive Aspects

Well-structured WebSocket implementation using Socket.IO
Comprehensive test coverage (208 new test lines)
Proper separation of concerns (separate file for WebSocket logic)
Good use of dataclasses for WebSocket message types
Automatic reconnection with resubscription
Callback-based API is intuitive and flexible
OrderbookManager integration for compatibility
Proper cleanup logic in strategy base class


Recommendations Summary

Must Fix Before Merge:

  1. Fix price normalization logic (issue impl polymarket #1)
  2. Add session cookie domain validation (issue Adds Polymarket data fetching capabilities #3)
  3. Fix inconsistent API limit documentation (issue Better MX #6)
  4. Add WebSocket state checking in start() (issue fix: polymarket impl #4)

Should Fix:
5. Use market tick_size for rounding (issue #2)
6. Improve error callback handling (issue #5)
7. Add missing edge case tests (issue #11)
8. Optimize cleanup sleep logic (issue #10)

Nice to Have:
9. Add WebSocket usage examples to docs (issue #15)
10. Add LRU cache for token mapping (issue #9)
11. Define magic numbers as constants (issue #13)


Overall Assessment

This is a solid implementation that adds valuable real-time functionality to the Limitless exchange. The code is well-organized, follows Python best practices, and includes good test coverage. However, there are several critical issues around price normalization and security that should be addressed before merging.

Recommendation: Request changes to address critical issues #1, #3, #4, and #6 before merging.

@claude
Copy link

claude bot commented Dec 21, 2025

Code Review: WebSocket Support for Limitless Exchange

Summary

This PR adds comprehensive WebSocket support for the Limitless exchange, including real-time orderbook updates, price data, and position tracking. Overall, this is a well-structured implementation with good test coverage. Below are detailed findings organized by category.


Positive Highlights

  1. Excellent test coverage - 200+ lines of tests covering core functionality
  2. Good architectural design - Clean separation between public and authenticated WebSocket classes
  3. Polymarket compatibility - Thoughtful interface design for cross-exchange compatibility
  4. Proper error handling - Callbacks wrapped in try-except blocks
  5. Documentation - Clear docstrings and usage examples
  6. Orderbook inversion logic - Correctly handles "No" token orderbook transformation

Issues & Recommendations

1. CRITICAL: Race Condition in WebSocket Lifecycle (limitless_ws.py:483-492)

Issue: The start() method doesn't wait for connection to establish before returning. Subscribers may try to subscribe before the WebSocket is connected.

# Current implementation
def start(self) -> threading.Thread:
    self.loop = asyncio.new_event_loop()
    async def _run():
        await self.connect()  # No guarantee connection is ready
        while self.state != WebSocketState.CLOSED:
            await asyncio.sleep(1)
    # ...
    self._thread.start()
    return self._thread  # Returns immediately

Recommendation: Add a connection ready event:

def __init__(self, ...):
    # Add to initialization
    self._ready = threading.Event()

async def connect(self):
    # ... existing code ...
    await self.sio.connect(...)
    self._ready.set()  # Signal ready

def start(self):
    # ... existing code ...
    self._thread.start()
    self._ready.wait(timeout=5)  # Wait for connection
    return self._thread

2. Resource Leak: Event Loop Not Closed (limitless_ws.py:486)

Issue: asyncio.new_event_loop() is created but never closed, leading to resource leaks.

Fix:

def _thread_target():
    asyncio.set_event_loop(self.loop)
    try:
        self.loop.run_until_complete(_run())
    except Exception as e:
        if self.verbose:
            logger.error(f"WebSocket thread error: {e}")
    finally:
        self.loop.close()  # Add this

3. Bug: stop() Doesn't Wait for Cleanup (limitless_ws.py:561-564)

Issue: stop() fires async disconnect but doesn't wait, potentially leaving connections open.

def stop(self):
    if self.loop and self.sio.connected:
        asyncio.run_coroutine_threadsafe(self.disconnect(), self.loop)
        # Missing: wait for completion

Fix:

def stop(self):
    if self.loop and self.sio.connected:
        future = asyncio.run_coroutine_threadsafe(self.disconnect(), self.loop)
        future.result(timeout=5)  # Wait for completion
    if self._thread and self._thread.is_alive():
        self._thread.join(timeout=5)

4. Potential Bug: Price Normalization Inconsistency (limitless.py:374-393)

Issue: Price normalization assumes > 1 means percentage (0-100), but this breaks for markets with prices exactly 1.0.

prices["Yes"] = yes_price / 100 if yes_price > 1 else yes_price

Problem: If API returns 1.0 (100%), it won't be normalized. If API is inconsistent, this creates bugs.

Recommendation: Document the API's actual behavior or use a more robust detection:

# Option 1: Check if both prices sum to ~100 (percentage format)
if abs((yes_price + no_price) - 100) < 1:
    yes_price /= 100
    no_price /= 100

# Option 2: Add explicit bounds checking
if yes_price > 1 or no_price > 1:
    yes_price /= 100
    no_price /= 100

5. Code Quality: Hardcoded Sleep Duration (strategy.py:577)

Issue: time.sleep(3) is arbitrary and may be insufficient for slow networks or too long for fast ones.

Recommendation:

# Make it configurable
max_wait = 10
poll_interval = 0.5
waited = 0
while waited < max_wait:
    remaining_orders = self.client.fetch_open_orders(market_id=self.market_id)
    if not remaining_orders:
        break
    time.sleep(poll_interval)
    waited += poll_interval

6. Security: Potential Log Injection (limitless_ws.py:175, 193, etc.)

Issue: Errors are logged with string formatting, potentially exposing sensitive data or enabling log injection.

logger.error(f"Orderbook callback error: {e}")

Recommendation: Sanitize error messages:

logger.error("Orderbook callback error: %s", str(e)[:200])

7. Edge Case: Empty Orderbook Handling (limitless_ws.py:542-546)

Issue: Inverted orderbook for "No" tokens may produce empty arrays if original orderbook is one-sided.

no_bids = [(round(1 - price, 3), size) for price, size in update.asks]
# If update.asks is empty, no_bids = []

This is actually correct behavior, but should be documented since it may surprise users.

Recommendation: Add a comment explaining this is expected.


8. Type Safety: Missing Type Hints (limitless_ws.py:526)

Issue: Callback parameter types aren't enforced:

def on_orderbook_update(update: OrderbookUpdate):  # Good
    # ...
    if callback:
        callback(market_id, {"bids": update.bids, "asks": update.asks})  # Type unclear

Fix: Update signature in watch_orderbook_by_market:

callback: Optional[Callable[[str, Dict[str, List[tuple]]], None]] = None

9. Performance: Inefficient Sorting (limitless.py:500-503, 542-546)

Issue: Orderbook is sorted twice - once from API, then again after processing.

Optimization: Only sort once at the end:

# Remove sorting at line 500-501
# Keep only final sort at line 504-505

10. Documentation: Missing Error Scenarios (limitless.py:1566-1580)

Issue: get_user_websocket() doesn't document what happens if session expires.

Recommendation: Add to docstring:

"""
...
Raises:
    AuthenticationError: If not authenticated or session expired
    
Note:
    Session cookies may expire. Re-authenticate if connection fails.
"""

Best Practices Adherence

Following CLAUDE.md Guidelines:

  1. Focused and clean - Code is well-organized
  2. No unnecessary files - Only essential code added
  3. UV for dependencies - Correctly used python-socketio[asyncio_client]
  4. Single source of truth - WebSocket URL in code, not env

⚠️ Partially following:

  1. Run and debug proactively - Good unit tests, but missing integration tests for actual WebSocket connections

Suggestions for Future Improvements

  1. Add integration tests - Test against actual Limitless WebSocket API (if testnet available)
  2. Metrics/monitoring - Track connection uptime, message rates, reconnection attempts
  3. Backpressure handling - If messages arrive faster than callbacks can process, queue may grow unbounded
  4. Exponential backoff - Current reconnection is linear (1-30s), exponential backoff is better practice

Test Coverage Assessment

Strengths:

  • Unit tests for all data parsing functions ✅
  • Tests for callback registration ✅
  • Tests for subscription management ✅
  • Tests for orderbook inversion logic ✅

Gaps:

  • No tests for WebSocket connection/disconnection lifecycle
  • No tests for reconnection behavior
  • No tests for error recovery
  • No tests for stop() cleanup

Conclusion

This is a solid implementation that adds valuable real-time capabilities. The code is well-structured and tested. However, there are critical issues with resource management and lifecycle that should be addressed:

Must Fix (P0):

  1. Race condition in start() method
  2. Event loop not closed
  3. stop() doesn't wait for cleanup

Should Fix (P1):

  1. Price normalization edge case
  2. Hardcoded sleep duration
  3. Log injection risk

Nice to Have (P2):

7-10. Documentation and type safety improvements

Recommendation: Fix P0 issues before merging. Overall quality is high, and with these fixes, this will be an excellent addition to the codebase.

Great work on the implementation! 🚀

… handling

- Added a timeout parameter to the start() method to control connection establishment duration.
- Introduced a threading.Event to signal when the WebSocket connection is ready.
- Updated the stop() method to include a timeout for disconnecting and cleaning up the background thread.
- Improved error handling during connection setup and shutdown processes.
@claude
Copy link

claude bot commented Dec 21, 2025

Pull Request Review: WebSocket Support for Limitless Exchange

Overview

This PR adds comprehensive WebSocket support for real-time market data on the Limitless exchange. The implementation is well-structured and includes solid test coverage.

Code Quality & Best Practices

Strengths ✅

  1. Clean Architecture: The WebSocket implementation is properly separated into its own module (limitless_ws.py)
  2. Comprehensive Testing: Excellent test coverage with 208 lines of tests covering various scenarios
  3. Type Hints: Good use of type annotations throughout
  4. Error Handling: Proper try-except blocks with optional verbose logging
  5. Documentation: Clear docstrings and inline comments
  6. State Management: Well-defined WebSocket states using Enum
  7. Callback Pattern: Clean callback registration with method chaining support

Areas for Improvement ⚠️

1. Price Normalization Inconsistency (dr_manhattan/exchanges/limitless.py:374-395)

The price normalization logic has redundant checks:

yes_price = float(data.get("yesPrice", 0) or 0)
no_price = float(data.get("noPrice", 0) or 0)
# Normalize to 0-1 range if in percentage format
prices["Yes"] = yes_price / 100 if yes_price > 1 else yes_price

Issue: This assumes prices > 1 are percentages, but what if the API returns values like 1.5? This could lead to inconsistent behavior.

Recommendation: Add explicit field checking or API version detection to determine the format, rather than relying on value ranges.

2. Hardcoded Market Fetch Limit (dr_manhattan/exchanges/limitless.py:304)

limit = min(query_params.get("limit", 25), 25)  # API max is 25

Issue: The limit is hardcoded and changed from 50 to 25 without a constant.

Recommendation: Define as a class constant:

MAX_MARKETS_PER_PAGE = 25
limit = min(query_params.get("limit", self.MAX_MARKETS_PER_PAGE), self.MAX_MARKETS_PER_PAGE)

3. Thread Safety Concerns (limitless_ws.py:502-503)

self._thread = threading.Thread(target=_thread_target, daemon=True)
self._thread.start()

Issue: Using daemon threads means they'll be killed abruptly on program exit, potentially losing in-flight messages.

Recommendation: Consider proper cleanup in stop() method (which you have, good!) but also document the daemon behavior.

4. Missing Truncated Code (limitless_ws.py:518)

The file appears to be truncated in the diff:

return self.orderbook_manager

The get_orderbook_manager method seems incomplete in the diff output.

Note: This might just be a diff display issue, but verify the file is complete.

Potential Bugs 🐛

1. Orderbook Inversion Precision (limitless.py:508-516, limitless_ws.py:558-559)

inverted_bids = [
    {"price": str(round(1 - float(a["price"]), 3)), "size": a["size"]} for a in asks
]

Issue: Rounding to 3 decimal places might lose precision for markets with tick_size < 0.001.

Recommendation: Use the market's tick_size for rounding precision.

2. Race Condition in WebSocket State (limitless_ws.py:149-155)

async def on_connect():
    self.state = WebSocketState.CONNECTED
    if self.verbose:
        logger.info("Connected to Limitless WebSocket")
    await self._resubscribe()

Issue: State is set before resubscription completes. If resubscription fails, state shows CONNECTED but subscriptions failed.

Recommendation: Consider state management:

self.state = WebSocketState.CONNECTED
try:
    await self._resubscribe()
except Exception as e:
    logger.error(f"Resubscription failed: {e}")
    # Decide if state should remain CONNECTED or revert

3. Silent Callback Failures (limitless_ws.py:168-176)

Callback exceptions are caught and logged but don't affect WebSocket operation. This is generally good, but consider:

  • Adding an error counter
  • Exposing callback failures to users via error callbacks
  • Possibly disconnecting after too many failures

Performance Considerations 🚀

1. Orderbook Sorting on Every Update (limitless_ws.py:271-272)

bids.sort(reverse=True)
asks.sort()

Issue: Sorting happens on every update, even for incremental changes.

Recommendation: If the API already provides sorted data, add a flag to skip sorting. For incremental updates, use bisect for insertion.

2. Orderbook Duplication for No Tokens (limitless_ws.py:556-569)

The entire orderbook is duplicated and inverted for No tokens.

Recommendation: Consider lazy evaluation - compute inverted orderbook only when requested, not on every update.

3. Busy Loop in Receive (limitless_ws.py:405-406)

while self.state == WebSocketState.CONNECTED and self.sio.connected:
    await asyncio.sleep(0.1)

Issue: This sleep loop is unnecessary since Socket.IO handles events internally.

Recommendation: Replace with await asyncio.Event().wait() that gets set on disconnect.

Security Concerns 🔒

1. Session Cookie Handling (limitless.py:1578-1581)

for cookie in self._session.cookies:
    if cookie.name == "limitless_session":
        session_cookie = cookie.value
        break

Considerations:

  • Cookie value is stored in memory (acceptable)
  • Consider warning users not to log WebSocket objects (they contain sensitive cookies)
  • Add a __repr__ method that redacts the session cookie

2. No TLS Verification Configuration

The WebSocket uses WSS (good!), but there's no option to configure TLS verification for testing environments.

Recommendation: Add optional verify_ssl parameter for development/testing.

Test Coverage 📊

Excellent Coverage ✅

  • Basic WebSocket initialization
  • Callback registration and chaining
  • Data parsing (orderbook, price, position updates)
  • State management
  • Market subscription/unsubscription

Missing Coverage ⚠️

  1. Connection Failure Scenarios: What happens when WS connection fails?
  2. Reconnection Logic: Test that resubscription works after disconnect
  3. Concurrent Callbacks: Test multiple callbacks registered for same event
  4. Thread Cleanup: Verify stop() properly cleans up threads
  5. Integration Tests: End-to-end test with actual WebSocket (maybe mocked)

Additional Observations

Strategy Cleanup Enhancement (dr_manhattan/base/strategy.py:566-590)

Good addition! The enhanced cleanup with waiting and verification is practical.

Suggestions:

  • The 3-second sleep is arbitrary. Consider making it configurable or polling for completion
  • The bare except Exception: pass at line 588 is too broad. At minimum log the exception.

Documentation

The wiki update (wiki/exchanges/limitless.md) should be verified to ensure examples are accurate and complete.

Recommendations Summary

Must Fix Before Merge 🔴

  1. Fix the overly broad exception handler in strategy.py cleanup
  2. Verify limitless_ws.py is not actually truncated
  3. Document the price normalization assumptions

Should Fix 🟡

  1. Add constants for magic numbers (25, 3 seconds, etc.)
  2. Improve thread safety and state management
  3. Consider performance optimizations for orderbook handling
  4. Add missing test scenarios

Nice to Have 🟢

  1. Add __repr__ to redact sensitive data
  2. Lazy evaluation for No token orderbooks
  3. Use bisect for incremental orderbook updates
  4. Add integration tests

Conclusion

This is a solid implementation that adds significant value to the project. The code is well-structured, properly tested, and follows good Python practices. The main concerns are around edge cases, performance optimizations, and some minor code quality issues. With the recommended fixes, this will be production-ready.

Overall Assessment: ✅ Approve with minor changes recommended

Great work on this feature! 🎉

@claude
Copy link

claude bot commented Dec 22, 2025

Pull Request Review: Limitless WebSocket Implementation

Summary

This PR introduces WebSocket support for the Limitless exchange, adding real-time market data capabilities. The implementation is well-structured with good test coverage, but there are several areas that need attention before merging.


Critical Issues

1. Incomplete Trade Dataclass Implementation

Location: limitless_ws.py:71-80

The Trade dataclass has several required fields but many default to empty strings (outcome, taker, maker, transaction_hash). This suggests incomplete implementation or these fields should be Optional.

Recommendation: Make these fields Optional[str] = None if they are not always available, or ensure they are properly populated.

2. Thread Safety Concerns

Location: limitless_ws.py:396-402

The callback execution mixes sync/async without proper safety. Synchronous callbacks block the event loop.

Recommendation: Use asyncio.to_thread() or loop.run_in_executor() for sync callbacks to avoid blocking.

3. Price Normalization Inconsistency

Location: limitless.py:374-393

Price normalization logic is duplicated and complex. The heuristic (divide by 100 if > 1) could fail edge cases.

Recommendation: Add explicit price format metadata from API or use a more robust detection method.

4. Missing Error Handling in Cleanup

Location: strategy.py:578-588

The cleanup process has a bare except Exception: pass which silently fails.

Recommendation: At minimum log the error for debugging; consider re-raising critical errors.


Security Concerns

5. Session Cookie Handling

Location: limitless.py:1582-1586

Cookies are extracted manually. Consider secure cookie handling best practices.

Recommendation: Ensure cookies are handled securely; avoid logging them.

6. No Input Validation on WebSocket Messages

Location: limitless_ws.py:236-280

The parsing functions use float() and int() without validation. Malformed WebSocket messages could cause crashes.

Recommendation: Add try/except with proper error handling around type conversions.


Code Quality Issues

7. Orderbook Inversion Logic Complexity

Location: limitless.py:500-517

The No token orderbook inversion is clever but complex, hard to test and verify correctness. Rounding to 3 decimals may cause precision issues.

Recommendation: Extract to a separate well-tested method with comprehensive tests.

8. Magic Numbers

Location: limitless.py:301

Hard-coded value: limit = min(query_params.get("limit", 25), 25)

Recommendation: Define MAX_MARKETS_PER_PAGE = 25 as a class constant.

9. Hardcoded Sleep in Cleanup

Location: strategy.py:577

time.sleep(3) - Arbitrary 3-second wait may be too short or too long

Recommendation: Make configurable or poll for order completion.

10. Duplicate Token Mapping Logic

Token-to-slug mapping exists in both WebSocket and Exchange classes. Potential for inconsistency.

Recommendation: Consider centralizing this mapping logic.


Performance Considerations

11. Synchronous Request in Orderbook Fetch

Location: limitless.py:449-517

The get_orderbook method makes synchronous HTTP requests. Could benefit from async implementation.

Recommendation: Consider adding an async variant for better performance.

12. No Connection Pooling Configuration

WebSocket reconnection settings use infinite reconnection which could cause issues.

Recommendation: Add configurable max reconnection attempts with exponential backoff.


Test Coverage

Strengths:

  • Comprehensive unit tests for parsing logic
  • Good coverage of edge cases (status variants, datetime formats)
  • WebSocket callback registration tests
  • Dataclass validation tests

Gaps:

  1. No integration tests for actual WebSocket connections
  2. No tests for orderbook inversion logic (critical functionality)
  3. No tests for error scenarios in WebSocket message parsing
  4. No tests for cleanup process with actual open orders/positions
  5. Missing tests for thread safety and concurrent callback execution

Recommendation: Add integration tests (can use mock WebSocket server) and test orderbook inversion thoroughly.


Best Practices & Style (per CLAUDE.md)

Following Guidelines:

  • ✅ Code is clean and focused
  • ✅ No emojis or non-essential characters
  • ✅ Using UV for dependencies
  • ⚠️ Consider: Remove or populate the unused Trade dataclass if not needed yet
  • ⚠️ Missing: Self-testing - no evidence of proactive debugging mentioned in CLAUDE.md point 5

Documentation

Good Documentation:

  • Clear docstrings for public methods
  • Usage examples in docstrings
  • Wiki documentation updated

Could Improve:

  • Add architecture overview comment in limitless_ws.py
  • Document the orderbook inversion algorithm with examples
  • Add troubleshooting section to wiki for common WebSocket issues

Recommendations Summary

Must Fix Before Merge:

  1. Fix thread safety in callback execution (issue add logo #2)
  2. Add proper error handling in cleanup (issue fix: polymarket impl #4)
  3. Validate WebSocket message inputs (issue Better MX #6)
  4. Add tests for orderbook inversion (test gap add logo #2)

Should Fix:

  1. Make Trade dataclass fields optional or implement properly (issue impl polymarket #1)
  2. Extract orderbook inversion to separate method with tests (issue Add Claude Code GitHub Workflow #7)
  3. Add configurable reconnection limits (issue Fix spread strategy script and add polymarket fetch_markets_by_slug & add black to pre-commit hook and chores #12)

Nice to Have:

  1. Use constants instead of magic numbers (issue Add vpin strategies #8)
  2. Make cleanup wait time configurable (issue Add CI & black #9)
  3. Add integration tests for WebSocket (test gap impl polymarket #1)

Overall Assessment

Quality: Good
Test Coverage: 7/10
Security: Minor concerns
Performance: Acceptable

This is a solid implementation with good structure and testing. The main concerns are around thread safety, error handling, and edge case validation. Once the critical issues are addressed, this will be ready to merge.

Great work on the WebSocket implementation overall!

…cellation

- Enhanced the stop() method in ExchangeClient to ensure all pending tasks are canceled before stopping the event loop.
- Added logic to handle the cancellation of tasks in the market WebSocket loop, improving resource management during shutdown.
- Updated Limitless class to refine size determination from API responses, ensuring accurate order size calculations based on market conditions.
@claude
Copy link

claude bot commented Dec 22, 2025

Pull Request Review: WebSocket Support for Limitless Exchange

Overall Assessment

This is a solid implementation that adds real-time market data capabilities through WebSocket connections. The code is well-structured, includes comprehensive tests, and follows good practices. However, there are several areas that need attention before merging.

Critical Issues

1. Race Condition in Event Loop Shutdown (exchange_client.py:488)

  • Problem: asyncio.all_tasks() is not thread-safe when called from outside the event loop's thread
  • Fix: Move task cancellation logic into the event loop using run_coroutine_threadsafe

2. Hard-Coded Sleep in Cleanup (strategy.py:577)

  • Problem: Fixed 3-second sleep is fragile and violates CLAUDE.md principle
  • Recommendation: Replace with polling logic that checks for order completion with timeout

3. Missing Error Handling in WebSocket Callbacks (limitless_ws.py:164-179)

  • Problem: Only logs errors when verbose=True, silently swallowing exceptions otherwise
  • Fix: Always log errors at WARNING level or higher

Security Concerns

4. Session Cookie Handling (limitless.py:1601)

  • Issue: No validation or expiry checking on session cookies
  • Recommendation: Add cookie validation and expiry checks

5. Credential Logging

  • Ensure no sensitive data is ever logged, even at debug level

Code Quality Issues

6. Duplicate Code in Orderbook Inversion

  • Appears in limitless.py:496-516 and limitless_ws.py:558-562
  • Per CLAUDE.md, centralize in shared method

7. Inconsistent Rounding Precision (limitless.py:508)

  • Hardcoded 3 decimal places
  • Use market tick_size or make configurable

8. Silent Exception Handling (strategy.py:588)

  • Too broad, should at minimum log failures

Performance Considerations

9. Inefficient List Operations (limitless_ws.py:415-416)

  • Using lists for membership checks is O(n)
  • Use sets for _subscribed_slugs, _subscribed_addresses, _no_tokens

10. Busy-Wait Loop (limitless_ws.py:405-406)

  • Checks connection 10x/second
  • Increase sleep interval or use Event

Best Practices

11. Price Normalization Logic (limitless.py:376-387)

  • Assumes price > 1 is percentage format
  • Could break with basis points, handle explicitly per API docs

12. Magic Numbers (limitless.py:958-960)

  • 1_000_000 divisor should be named constant

Test Coverage

Positive: Comprehensive unit tests, good error case coverage
Missing: Integration tests, thread safety tests, cleanup tests, edge cases

Recommendations Summary

Must Fix Before Merge:

  1. Fix race condition in exchange_client.py:488 (CRITICAL)
  2. Replace hard-coded sleep with polling in strategy.py:577
  3. Add proper error logging in WebSocket callbacks
  4. Use sets instead of lists for subscriptions

Should Fix:
5. Centralize orderbook inversion logic
6. Add session cookie expiry validation
7. Remove magic numbers
8. Fix silent exception handling

Nice to Have:
9. Add integration tests
10. Improve price normalization logic
11. Standardize logging

Conclusion

This PR adds valuable real-time capabilities. Implementation is well-designed but critical threading issues and cleanup logic need addressing before merge.

Recommendation: Request changes for critical issues, then approve after fixes.

@guzus guzus merged commit d814b56 into main Dec 22, 2025
6 checks passed
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.

2 participants