From 943dc6db83ca512e0115d2c64e231d023ddc5a78 Mon Sep 17 00:00:00 2001 From: Brian McMahon Date: Sat, 25 Apr 2026 06:05:55 -0700 Subject: [PATCH 1/2] fix(universe_returns): use NYSE trading-day arithmetic for forward windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace _add_business_days (Mon-Fri, no holiday awareness) with _add_trading_days using alpha_engine_lib.trading_calendar.next_trading_day. The pre-fix helper silently mis-labeled forward returns when the window crossed a NYSE holiday — e.g. eval_date=2026-04-02 + 5 BD returned 2026-04-09 (counting Good Friday as a BD), so return_5d was actually a 4-trading-day return. Now lands on 2026-04-10 as intended. Also enumerates trading days only via existing is_trading_day check, so holidays never become eval_dates either. 14 new tests in test_universe_returns_trading_day.py covering the helper + _trading_days_to_process eligibility filter. 223/223 full suite pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- collectors/universe_returns.py | 31 +++--- tests/test_universe_returns_trading_day.py | 107 +++++++++++++++++++++ 2 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 tests/test_universe_returns_trading_day.py diff --git a/collectors/universe_returns.py b/collectors/universe_returns.py index f55c6a4..1d1329a 100644 --- a/collectors/universe_returns.py +++ b/collectors/universe_returns.py @@ -224,7 +224,7 @@ def _trading_days_to_process( if nyse_is_trading_day(d): trading_days_seen += 1 iso = d.isoformat() - fwd_5d = _add_business_days(d, 5) + fwd_5d = _add_trading_days(d, 5) if fwd_5d < today and iso not in existing: out.append(iso) d -= timedelta(days=1) @@ -328,9 +328,9 @@ def _build_rows_for_date( ) -> list[dict]: """Build universe_returns rows for a single eval_date.""" eval_dt = date.fromisoformat(eval_date) - fwd_5d = _add_business_days(eval_dt, 5) - fwd_10d = _add_business_days(eval_dt, 10) - fwd_30d = _add_business_days(eval_dt, 30) + fwd_5d = _add_trading_days(eval_dt, 5) + fwd_10d = _add_trading_days(eval_dt, 10) + fwd_30d = _add_trading_days(eval_dt, 30) # Check that forward dates are in the past (returns can be computed) today = date.today() @@ -350,7 +350,7 @@ def _build_rows_for_date( if not prices_t0: logger.warning("No prices for eval_date %s — may be a non-trading day", eval_date) # Try next business day - next_day = _add_business_days(eval_dt, 1) + next_day = _add_trading_days(eval_dt, 1) prices_t0 = polygon_client.get_grouped_daily(str(next_day)) if not prices_t0: return [] @@ -426,12 +426,19 @@ def _pct_return(price_start: float | None, price_end: float | None) -> float | N return (price_end / price_start) - 1.0 -def _add_business_days(start: date, n: int) -> date: - """Add n business days to a date (skipping weekends).""" +def _add_trading_days(start: date, n: int) -> date: + """Add n NYSE trading days to a date (skipping weekends + NYSE holidays). + + Calendar-business-day arithmetic (Mon-Fri only, no holiday awareness) + silently mis-labels forward returns when the window crosses a holiday — + e.g. eval_date=2026-04-02 with `_add_business_days(.,5)` returned 2026-04-09 + (counting Good Friday 2026-04-03 as a BD), so `return_5d` would actually + be a 4-trading-day return. Use NYSE-calendar-aware arithmetic so the + column label matches the column meaning. + """ + from alpha_engine_lib.trading_calendar import next_trading_day + current = start - added = 0 - while added < n: - current += timedelta(days=1) - if current.weekday() < 5: # Mon-Fri - added += 1 + for _ in range(n): + current = next_trading_day(current) return current diff --git a/tests/test_universe_returns_trading_day.py b/tests/test_universe_returns_trading_day.py new file mode 100644 index 0000000..3081b19 --- /dev/null +++ b/tests/test_universe_returns_trading_day.py @@ -0,0 +1,107 @@ +"""Trading-day arithmetic in collectors/universe_returns.py. + +Pre-fix `_add_business_days` skipped weekends but counted NYSE holidays +as business days, silently mis-labeling forward returns whose window +crossed a holiday. These tests pin the trading-day-aware behavior. +""" +from __future__ import annotations + +from datetime import date + +import pytest + +from collectors.universe_returns import ( + _add_trading_days, + _trading_days_to_process, +) + + +class TestAddTradingDays: + def test_skips_weekend(self): + assert _add_trading_days(date(2026, 4, 17), 1) == date(2026, 4, 20) + + def test_skips_good_friday_2026(self): + # 2026-04-02 (Thu) → 5 trading days = 2026-04-10 (Fri) + # Calendar BDs would have wrongly included 2026-04-03 (Good Friday) + # and returned 2026-04-09. + assert _add_trading_days(date(2026, 4, 2), 5) == date(2026, 4, 10) + + def test_skips_thanksgiving(self): + # 2025-11-26 (Wed) → 1 trading day; 11-27 is Thanksgiving (closed), + # 11-28 is Black Friday (open, half day but a trading day). + assert _add_trading_days(date(2025, 11, 26), 1) == date(2025, 11, 28) + + def test_horizons_5_10_30_no_holiday_window(self): + # 2026-04-13 (Mon): no NYSE holidays in 4/14-5/27 window; + # trading-day counts collapse to calendar BD counts. + assert _add_trading_days(date(2026, 4, 13), 5) == date(2026, 4, 20) + assert _add_trading_days(date(2026, 4, 13), 10) == date(2026, 4, 27) + + def test_zero_returns_start(self): + # Adding 0 trading days returns the input unchanged. + assert _add_trading_days(date(2026, 4, 17), 0) == date(2026, 4, 17) + + +class TestTradingDaysToProcess: + def test_only_trading_days_enumerated(self): + # Today = Sat 2026-04-25; lookback covers 4/3 (Good Friday) and + # 4/4-5 weekend. None should appear in the result. + out = _trading_days_to_process( + today=date(2026, 4, 25), + max_lookback=20, + existing=set(), + ) + forbidden = {"2026-04-03", "2026-04-04", "2026-04-05"} + assert forbidden.isdisjoint(set(out)) + + def test_eligibility_requires_fwd_5d_strictly_past(self): + # Today = 2026-04-25 (Sat). 4/20 (Mon) has fwd_5d = 4/27 (next Mon), + # which is NOT < today → ineligible. + out = _trading_days_to_process( + today=date(2026, 4, 25), + max_lookback=10, + existing=set(), + ) + assert "2026-04-20" not in out + + def test_existing_dates_filtered(self): + out = _trading_days_to_process( + today=date(2026, 4, 25), + max_lookback=20, + existing={"2026-04-17"}, + ) + assert "2026-04-17" not in out + + def test_4_17_eligible_today(self): + # Sanity: on 2026-04-25, 2026-04-17 (Fri) IS eligible because + # fwd_5d = 4/24 < 4/25 and it's a trading day. + out = _trading_days_to_process( + today=date(2026, 4, 25), + max_lookback=20, + existing=set(), + ) + assert "2026-04-17" in out + + def test_results_sorted_chronologically(self): + out = _trading_days_to_process( + today=date(2026, 4, 25), + max_lookback=30, + existing=set(), + ) + assert out == sorted(out) + + +@pytest.mark.parametrize( + "eval_date,horizon,expected", + [ + # Good Friday 2026 window: holiday must be skipped + (date(2026, 4, 2), 1, date(2026, 4, 6)), # Thu 4/2 → Mon 4/6 (skip Fri Good Friday) + (date(2026, 4, 2), 5, date(2026, 4, 10)), + # Thanksgiving 2025 + (date(2025, 11, 25), 5, date(2025, 12, 3)), # Tue → 1d holiday + weekends + # New Year 2026 — 2026-01-01 (Thu, holiday) and the long weekend + (date(2025, 12, 31), 1, date(2026, 1, 2)), # Wed → Fri (skip 1/1) + ], +) +def test_horizon_table(eval_date, horizon, expected): + assert _add_trading_days(eval_date, horizon) == expected From a6615f0beea36934d4a7de4e301d7f9d399d9cc1 Mon Sep 17 00:00:00 2001 From: Brian McMahon Date: Sat, 25 Apr 2026 06:22:26 -0700 Subject: [PATCH 2/2] universe_returns: import _add_trading_days from alpha-engine-lib v0.2.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace local helper with lib import — same logic, single source of truth across the three repos that need trading-day arithmetic. Trim duplicate arithmetic tests (those are locked in alpha-engine-lib's test_trading_calendar.py now); keep data-specific eligibility-filter tests. Requires alpha-engine-lib >= 0.2.1 (alpha-engine-data is pinned @main, so auto-picks up after lib PR #8 merges). Co-Authored-By: Claude Opus 4.7 (1M context) --- collectors/universe_returns.py | 19 +----- tests/test_universe_returns_trading_day.py | 74 ++++++---------------- 2 files changed, 21 insertions(+), 72 deletions(-) diff --git a/collectors/universe_returns.py b/collectors/universe_returns.py index 1d1329a..ff34fa0 100644 --- a/collectors/universe_returns.py +++ b/collectors/universe_returns.py @@ -23,6 +23,7 @@ import boto3 import pandas as pd +from alpha_engine_lib.trading_calendar import add_trading_days as _add_trading_days logger = logging.getLogger(__name__) @@ -424,21 +425,3 @@ def _pct_return(price_start: float | None, price_end: float | None) -> float | N if price_start is None or price_end is None or price_start <= 0: return None return (price_end / price_start) - 1.0 - - -def _add_trading_days(start: date, n: int) -> date: - """Add n NYSE trading days to a date (skipping weekends + NYSE holidays). - - Calendar-business-day arithmetic (Mon-Fri only, no holiday awareness) - silently mis-labels forward returns when the window crosses a holiday — - e.g. eval_date=2026-04-02 with `_add_business_days(.,5)` returned 2026-04-09 - (counting Good Friday 2026-04-03 as a BD), so `return_5d` would actually - be a 4-trading-day return. Use NYSE-calendar-aware arithmetic so the - column label matches the column meaning. - """ - from alpha_engine_lib.trading_calendar import next_trading_day - - current = start - for _ in range(n): - current = next_trading_day(current) - return current diff --git a/tests/test_universe_returns_trading_day.py b/tests/test_universe_returns_trading_day.py index 3081b19..d3f44c7 100644 --- a/tests/test_universe_returns_trading_day.py +++ b/tests/test_universe_returns_trading_day.py @@ -1,50 +1,21 @@ -"""Trading-day arithmetic in collectors/universe_returns.py. +"""Trading-day eligibility filter in collectors/universe_returns.py. -Pre-fix `_add_business_days` skipped weekends but counted NYSE holidays -as business days, silently mis-labeling forward returns whose window -crossed a holiday. These tests pin the trading-day-aware behavior. +The arithmetic itself (add_trading_days) is locked in alpha-engine-lib's +test_trading_calendar.py. These tests cover the data-module behavior: +that _trading_days_to_process correctly delegates to NYSE-aware arithmetic +and never enqueues weekends, holidays, or eval_dates whose 5d forward +window has not yet closed. """ from __future__ import annotations from datetime import date -import pytest - -from collectors.universe_returns import ( - _add_trading_days, - _trading_days_to_process, -) - - -class TestAddTradingDays: - def test_skips_weekend(self): - assert _add_trading_days(date(2026, 4, 17), 1) == date(2026, 4, 20) - - def test_skips_good_friday_2026(self): - # 2026-04-02 (Thu) → 5 trading days = 2026-04-10 (Fri) - # Calendar BDs would have wrongly included 2026-04-03 (Good Friday) - # and returned 2026-04-09. - assert _add_trading_days(date(2026, 4, 2), 5) == date(2026, 4, 10) - - def test_skips_thanksgiving(self): - # 2025-11-26 (Wed) → 1 trading day; 11-27 is Thanksgiving (closed), - # 11-28 is Black Friday (open, half day but a trading day). - assert _add_trading_days(date(2025, 11, 26), 1) == date(2025, 11, 28) - - def test_horizons_5_10_30_no_holiday_window(self): - # 2026-04-13 (Mon): no NYSE holidays in 4/14-5/27 window; - # trading-day counts collapse to calendar BD counts. - assert _add_trading_days(date(2026, 4, 13), 5) == date(2026, 4, 20) - assert _add_trading_days(date(2026, 4, 13), 10) == date(2026, 4, 27) - - def test_zero_returns_start(self): - # Adding 0 trading days returns the input unchanged. - assert _add_trading_days(date(2026, 4, 17), 0) == date(2026, 4, 17) +from collectors.universe_returns import _trading_days_to_process class TestTradingDaysToProcess: - def test_only_trading_days_enumerated(self): - # Today = Sat 2026-04-25; lookback covers 4/3 (Good Friday) and + def test_excludes_weekends_and_holidays(self): + # Today = Sat 2026-04-25; lookback covers Good Friday 4/3 and the # 4/4-5 weekend. None should appear in the result. out = _trading_days_to_process( today=date(2026, 4, 25), @@ -73,7 +44,7 @@ def test_existing_dates_filtered(self): assert "2026-04-17" not in out def test_4_17_eligible_today(self): - # Sanity: on 2026-04-25, 2026-04-17 (Fri) IS eligible because + # On 2026-04-25, 2026-04-17 (Fri) IS eligible because # fwd_5d = 4/24 < 4/25 and it's a trading day. out = _trading_days_to_process( today=date(2026, 4, 25), @@ -90,18 +61,13 @@ def test_results_sorted_chronologically(self): ) assert out == sorted(out) - -@pytest.mark.parametrize( - "eval_date,horizon,expected", - [ - # Good Friday 2026 window: holiday must be skipped - (date(2026, 4, 2), 1, date(2026, 4, 6)), # Thu 4/2 → Mon 4/6 (skip Fri Good Friday) - (date(2026, 4, 2), 5, date(2026, 4, 10)), - # Thanksgiving 2025 - (date(2025, 11, 25), 5, date(2025, 12, 3)), # Tue → 1d holiday + weekends - # New Year 2026 — 2026-01-01 (Thu, holiday) and the long weekend - (date(2025, 12, 31), 1, date(2026, 1, 2)), # Wed → Fri (skip 1/1) - ], -) -def test_horizon_table(eval_date, horizon, expected): - assert _add_trading_days(eval_date, horizon) == expected + def test_holiday_eval_date_skips_correctly(self): + # 2026-04-02 (Thu before Good Friday): trading day. fwd_5d via + # NYSE-aware arithmetic = 2026-04-10 (skip Good Friday). 4/10 < 4/25 + # → eligible, included. + out = _trading_days_to_process( + today=date(2026, 4, 25), + max_lookback=20, + existing=set(), + ) + assert "2026-04-02" in out