Conversation
- Implement `investigate_market_structure.py` to fetch and analyze non-crypto market data structures, focusing on price-related fields and outcomes. - Create `investigate_politics_prices.py` to compare data structures between sports and politics markets, investigating the absence of prices in politics/entertainment markets. - Introduce tests for momentum scanner filtering and integration tests for new momentum hunter data sources in `test_debug_filters.py` and `test_momentum_sources.py`. - Add end-to-end test for the momentum hunter functionality in `test_scan_e2e.py`, ensuring all strategies are functioning correctly. - Enhance unit tests in `tests/test_momentum_hunter.py` to cover market categorization, direction determination, middle zone filtering, and market deduplication logic.
Validation ResultsQuick Validation: Completed (64 tests) Full Validation: Completed (114 tests) NOTE: Check the "Checks" tab for detailed validation output. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Momentum Hunter scanner to use fixed probability thresholds (>75% for YES, <25% for NO) instead of configurable extremity filtering, adds investigation scripts for analyzing market data structures, and introduces comprehensive test coverage for the new multi-source aggregation approach.
Key Changes:
- Refactored
scan_pullback_markets()to use 6 market sourcing strategies (Liquidity, Default, Hot, Breaking, Events, Newest) with enhanced validation and filtering pipeline - Added 8 investigation scripts to analyze market data structures and debug price/expiry field issues
- Enhanced unit tests in
test_momentum_hunter.pyto cover fixed thresholds, middle zone filtering, and market deduplication - Reduced default volume filter from $500k to $250k and removed high-end volume options (1.5M, 2M)
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| app.py | Refactored scan_pullback_markets() with 4-stage pipeline (aggregation, validation, filtering, scoring), removed sidebar separator, updated CSS for compact spacing, changed volume slider defaults |
| tests/test_momentum_hunter.py | Replaced extremity-based tests with fixed threshold tests (75%/25%), added middle zone filtering tests, market categorization tests, and deduplication tests |
| test_scan_e2e.py | Added end-to-end test for complete momentum hunter scan with all strategies |
| test_momentum_sources.py | Added integration test to validate all 6 data source strategies |
| test_debug_filters.py | Added debug script to test momentum scanner filtering with relaxed parameters |
| investigate_market_structure.py | Investigation script to examine market data fields and price structures |
| investigate_politics_prices.py | Script to compare price data between Sports and Politics/Entertainment markets |
| investigate_grouped_markets.py | Script to analyze grouped/nested market structures and parent-child relationships |
| find_none_price_markets.py | Script to locate markets with missing price data |
| debug_expiry.py | Debug script to check market expiration date parsing |
| debug_api_format.py | Script to inspect actual API response format for markets |
| check_event_structure.py | Script to examine event market data structure |
| check_default_sort.py | Script to verify default sort market data |
| TEST_RESULTS.txt | Comprehensive test results summary documenting all passing tests and implementation details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| actual_passes = not in_middle and (direction := 'YES' if yes_price > 0.75 else 'NO') and \ | ||
| ((1.0 - yes_price) if direction == 'YES' else yes_price) >= min_distance - 1e-10 |
There was a problem hiding this comment.
This line uses the walrus operator inside a boolean expression in a way that relies on short-circuit evaluation to avoid errors. The expression will always be truthy because the assignment (direction := ...) returns the assigned value, and then the second part checks if the distance passes. This means actual_passes will be True even when in_middle is True (when it should be False). This logic error can cause the test assertion to fail. Restructure this to properly compute actual_passes without relying on short-circuit evaluation of the walrus operator.
| st.caption("Still seeing wrong data? [Force Refresh](?refresh=true)") | ||
|
|
||
| st.markdown('<div style="margin-top: 0.3rem;"></div>', unsafe_allow_html=True) | ||
| st.caption("💡 Qualifies if extreme (>75%/<25%) OR high momentum (≥30%) with >60%/<40% probability") |
There was a problem hiding this comment.
The caption describes outdated qualification logic that mentions "OR high momentum (≥30%) with >60%/<40% probability". However, based on the refactored code in scan_pullback_markets, the current logic uses fixed thresholds (>75%/<25%) without the high momentum alternative qualification path. Update this caption to reflect the actual filtering logic: markets qualify if they are >75% or <25%, have sufficient momentum, and pass volume/distance filters.
| st.caption("💡 Qualifies if extreme (>75%/<25%) OR high momentum (≥30%) with >60%/<40% probability") | |
| st.caption("💡 Markets qualify if they are >75% or <25%, have sufficient momentum, and pass volume/distance filters") |
| Pipeline Architecture: | ||
| ┌─────────────────────────────────────────────────────────┐ | ||
| │ STAGE 1: MULTI-SOURCE AGGREGATION │ | ||
| │ • 8 diverse API strategies for comprehensive coverage │ |
There was a problem hiding this comment.
The docstring claims "8 diverse API strategies" in the architecture description, but the implementation only includes 6 strategies (Liquidity, Default, Hot, Breaking, Events, Newest). Update the docstring to accurately reflect 6 strategies, or implement the missing strategies if they were intended.
| │ • 8 diverse API strategies for comprehensive coverage │ | |
| │ • 6 diverse API strategies for comprehensive coverage │ |
| # Run scan with relaxed filters | ||
| opportunities = scan_pullback_markets( | ||
| max_expiry_hours=720, # 30 days | ||
| min_extremity=0.20, # >=70% or <=30% |
There was a problem hiding this comment.
The comment ">=70% or <=30%" describes the expected behavior for min_extremity=0.20 (20%), but this is mathematically incorrect. With min_extremity=0.20, markets should qualify at >=80% or <=20%, not >=70% or <=30%. However, since min_extremity is not actually used in the implementation (it uses fixed 75%/25% thresholds), this comment is misleading. Either correct the comment to reflect the actual hardcoded thresholds (>75% or <25%), or implement the configurable min_extremity parameter.
| min_extremity=0.20, # >=70% or <=30% | |
| min_extremity=0.20, # uses hardcoded >75% or <25% extremes |
|
|
||
| opportunities = scan_pullback_markets( | ||
| max_expiry_hours=720, # 30 days | ||
| min_extremity=0.20, # >=70% or <=30% |
There was a problem hiding this comment.
The comment ">=70% or <=30%" describes the expected behavior for min_extremity=0.20 (20%), but this is mathematically incorrect. With min_extremity=0.20, markets should qualify at >=80% or <=20%, not >=70% or <=30%. However, since min_extremity is not actually used in the implementation (it uses fixed 75%/25% thresholds), this comment is misleading. Either correct the comment to reflect the actual hardcoded thresholds (>75% or <25%), or implement the configurable min_extremity parameter.
| min_extremity=0.20, # >=70% or <=30% | |
| min_extremity=0.20, # Note: current implementation uses fixed >75% or <25% thresholds; min_extremity is not yet applied |
| import asyncio | ||
| import logging | ||
| from clients.gamma_client import GammaClient | ||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| import json |
| if isinstance(outcomes, str): | ||
| try: | ||
| outcomes = json.loads(outcomes) | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| try: | ||
| # Try JSON first | ||
| outcome_prices = json.loads(raw_prices) | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| # Try comma-separated values | ||
| try: | ||
| outcome_prices = [float(p.strip()) for p in raw_prices.split(',') if p.strip()] | ||
| except: |
There was a problem hiding this comment.
Except block directly handles BaseException.
| except: | ||
| print(f"Outcomes (parse failed)") |
There was a problem hiding this comment.
Except block directly handles BaseException.
| except: | |
| print(f"Outcomes (parse failed)") | |
| except (json.JSONDecodeError, TypeError, ValueError) as e: | |
| print(f"Outcomes (parse failed): {e}") |
investigate_market_structure.pyto fetch and analyze non-crypto market data structures, focusing on price-related fields and outcomes.investigate_politics_prices.pyto compare data structures between sports and politics markets, investigating the absence of prices in politics/entertainment markets.test_debug_filters.pyandtest_momentum_sources.py.test_scan_e2e.py, ensuring all strategies are functioning correctly.tests/test_momentum_hunter.pyto cover market categorization, direction determination, middle zone filtering, and market deduplication logic.Description
Please provide a brief description of the changes made in this pull request.
Type of Change
Related Issues
Please link to any related issues or pull requests. Use
Fixes #123orCloses #123if this PR resolves an issue.Changes Made
What was changed?
Why was it changed?
How was it changed?
Testing
Screenshots (if applicable)
Add screenshots to demonstrate the changes, especially for UI changes.
Checklist
Additional Notes
Any additional information or context about this pull request.