feat(cross_exchange): add cross-exchange market management module

[guzus]

Summary

• Add type hierarchy: OutcomeRef, OutcomeToken, ExchangeOutcomeRef
• Add CrossExchangeManager for fetching and comparing markets across exchanges
• Add MarketMatcher skeleton with category-based and LLM matching strategies
• Add example script and documentation

[claude]

Pull Request Review: Cross-Exchange Market Management Module

Summary

This PR adds a well-structured cross-exchange market comparison system. The implementation is clean, modular, and follows good architectural principles with a clear separation between manual mapping (working) and automatic matching (skeleton for future).

---

Strengths

1. Excellent Architecture & Design ✅

• Clean type hierarchy: OutcomeRef → OutcomeToken and ExchangeOutcomeRef makes sense
• Clear separation of concerns: manager.py (fetching), matcher.py (matching), types.py (data structures)
• Exchange-agnostic design - no "primary" exchange bias
• The market_path concept is elegant: handles both simple IDs (["61"]) and hierarchical (["fetch_slug", "match_id"])

2. Good Documentation ✅

• Comprehensive CROSS_EXCHANGE.md explaining design choices
• Clear inline comments and docstrings
• Working example script demonstrates usage well

3. Smart Implementation Details ✅

• Lazy exchange initialization in CrossExchangeManager._get_exchange()
• Fallback logic in _fetch_market() for exchanges with/without fetch_markets_by_slug
• Proper handling of nested markets in Limitless (lines 364-446 in limitless.py)
• Case-insensitive market matching (types.py:19)

---

Issues & Recommendations

CRITICAL: Violation of CLAUDE.md Guidelines 🚨

1. Environment Variables (.env.example)

Location: .env.example:11-13

+# TODO: OpenRouter API (for LLM-based market matching)
+OPENROUTER_API_KEY=sk-or-...
+OPENROUTER_MODEL=xiaomi/mimo-v2-flash:free

Issue: CLAUDE.md Rule #4 states: "Single Source of Truth: DO NOT place many variables in .env file. Place them in the code instead."

Recommendation: Since LLM matching is not yet implemented (returns 0.0), either:

1. Remove these from .env.example until LLM matching is actually implemented
2. Add as class constants in matcher.py:LLMMatchStrategy with clear defaults

---

2. Documentation File Created

Location: dr_manhattan/cross_exchange/CROSS_EXCHANGE.md (223 lines)

Issue: CLAUDE.md Rule #2 states: "DO NOT create a new document. Purge unnecessary code and files."

Recommendation:

• While the documentation is excellent, consider moving key information to:
  • Module/class docstrings
  • README.md section
  • Inline comments
• If documentation must exist, consider a smaller focused doc or examples/README.md

---

HIGH PRIORITY: Code Quality Issues ⚠️

3. Silent Exception Handling

Location: manager.py:42-46, manager.py:70-74

# Line 42-46
if hasattr(exchange, "fetch_markets_by_slug"):
    try:
        return exchange.fetch_markets_by_slug(market_id)
    except Exception:  # Too broad\!
        pass

# Line 70-74  
try:
    fetched = self._fetch_market(exchange_id, fetch_id)
    markets[exchange_id].extend(fetched)
except Exception as e:  # Prints but continues
    print(f"[{exchange_id}] Error fetching {fetch_id}: {e}")

Issues:

• Bare except Exception without logging (line 45)
• print() statements instead of proper logging (line 74)
• Failures are silent - user may not realize data is incomplete

Recommendation:

import logging
logger = logging.getLogger(__name__)

# Better version:
if hasattr(exchange, "fetch_markets_by_slug"):
    try:
        return exchange.fetch_markets_by_slug(market_id)
    except (MarketNotFound, ValueError) as e:  # Specific exceptions
        logger.debug(f"fetch_markets_by_slug failed for {market_id}: {e}")
    except Exception as e:
        logger.warning(f"Unexpected error in fetch_markets_by_slug: {e}")

# Or raise if critical:
for fetch_id in ids:
    fetched = self._fetch_market(exchange_id, fetch_id)
    markets[exchange_id].extend(fetched)  # Let exceptions propagate

---

4. Incomplete Skeleton Code Exported in Public API

Location: cross_exchange/__init__.py:6-13

from .matcher import (
    CategoryMatchStrategy,
    CryptoHourlyMatcher,
    ElectionMatcher,
    FedDecisionMatcher,
    LLMMatchStrategy,
    # ... all return 0.0
)

Issue: All matcher strategies return 0.0 (non-functional), yet they're exported in __all__. This could mislead users into thinking they work.

Recommendation:

• Either implement at least one matcher OR
• Don't export them in __all__ until implemented
• Add clear raise NotImplementedError() instead of returning 0.0

---

5. Missing Test Coverage

Location: No test files for cross_exchange module

Issue: No tests found for:

• CrossExchangeManager.fetch()
• FetchedMarkets.get_matched_outcomes()
• Market matching logic
• Edge cases (missing prices, failed fetches, etc.)

Recommendation: Add tests, especially for:

def test_cross_exchange_manager_fetch():
    # Test successful fetch
    # Test partial failure (one exchange fails)
    # Test empty mapping
    
def test_fetched_markets_matching():
    # Test outcome matching
    # Test spread calculation
    # Test missing outcome handling
    
def test_market_matching_logic():
    # Test case-insensitive matching
    # Test match_id vs id fallback

---

MEDIUM PRIORITY: Design & Maintainability 💡

6. Type Confusion: Dict[str, set] vs Dict[str, Set[str]]

Location: manager.py:50

def _get_fetch_ids(self, slug: str) -> Dict[str, set]:  # lowercase 'set'
    return _extract_fetch_slugs(self.mapping, slug)  # returns Dict[str, Set[str]]

Issue: Type hint uses set (not Set[str]) - inconsistent with actual implementation

Fix:

def _get_fetch_ids(self, slug: str) -> Dict[str, Set[str]]:

---

7. Magic Number in Spread Condition

Location: types.py:68-71, examples/cross_exchange_simple.py:96-102

# types.py
values = [p.price for p in self.prices.values() if p.price > 0]  # Why > 0?

# example
if spread > 1:  # Magic numbers
    spread_str = f"[red]{spread:.1f}%[/red]"
elif spread > 0.5:

Issue:

• Filtering p.price > 0 might exclude valid 0% prices
• Magic numbers (1, 0.5) in example should be constants

Recommendation:

# types.py - prices can be 0
values = [p.price for p in self.prices.values()]

# example - use constants
SPREAD_HIGH_THRESHOLD = 0.01  # 1%
SPREAD_MEDIUM_THRESHOLD = 0.005  # 0.5%

---

8. Hardcoded Pagination Limit

Location: limitless.py:329, opinion.py:403

max_pages = 100  # Hardcoded limit

Issue: Could fail to fetch all markets if >2500 markets exist (100 pages × 25 per page)

Recommendation:

def _fetch_all_markets(self, params: Dict[str, Any], max_pages: int = 100) -> List[Market]:
    # Make it configurable

---

LOW PRIORITY: Polish & Best Practices 🔧

9. Inconsistent Return Type Documentation

Location: manager.py:127 (returns tuple but undocumented)

def _compute_score(self, market_a: Market, market_b: Market) -> tuple:
    # Should be: -> Tuple[float, Dict[str, float]]

---

10. Rich Dependency for Core Module

Location: pyproject.toml:24

Issue: rich is only used in example script but added as core dependency

Recommendation: Move to optional dependencies or dev dependencies since it's only for examples

---

Security & Performance

✅ Security: Good

• No SQL injection or XSS vulnerabilities
• No credential exposure
• Proper input validation in price ranges (market.py:74-76)

✅ Performance: Reasonable

• Exchange instances are cached
• Fetch operations could be parallelized (future enhancement)
• No obvious N+1 query issues

---

Testing Recommendations

Add these test scenarios:

1. Manager Tests:

   • Test with all 3 exchanges (Polymarket, Opinion, Limitless)
   • Test partial exchange failure
   • Test invalid slug handling

2. Matching Tests:

   • Test case-insensitive matching
   • Test nested market expansion (Limitless Fed example)
   • Test spread calculation edge cases

3. Integration Test:

   • Test the full example script flow
   • Mock exchange responses

---

Alignment with CLAUDE.md

Rule	Status	Notes
1. Focused & clean	⚠️ PARTIAL	Code is clean, but CROSS_EXCHANGE.md adds 223 lines
2. No new documents	❌ VIOLATION	CROSS_EXCHANGE.md created
3. Use UV	✅ PASS	Example uses uv run
4. Single source of truth	❌ VIOLATION	Added env vars for unimplemented feature
5. Run & Debug	⚠️ UNKNOWN	No evidence of testing

---

Verdict

Conditional Approval with Required Changes

Must Fix Before Merge:

1. ❌ Remove OPENROUTER_* from .env.example (unimplemented feature)
2. ❌ Remove or consolidate CROSS_EXCHANGE.md per CLAUDE.md
3. ⚠️ Add proper logging instead of print statements
4. ⚠️ Add basic test coverage (at least happy path)

Should Fix:

5. Fix type hints (set → Set[str])
6. Either implement one matcher OR don't export skeletons in __all__
7. Add raise NotImplementedError() to skeleton matchers

Nice to Have:

8. Move rich to dev dependencies
9. Make pagination limits configurable
10. Document return types properly

---

Positive Conclusion

Despite the issues above, this is high-quality work with excellent architecture. The type hierarchy, exchange-agnostic design, and separation of concerns are exemplary. The violations of CLAUDE.md are easily fixable, and the code quality issues are mostly about error handling and testing.

After addressing the critical violations and adding tests, this will be a solid addition to the codebase. 🚀

Great job on the core implementation! 👏

[bullish-lee]

wow cross-exchange market great!!