Skip to content

fix: holiday check false positives on trading days#8

Merged
cipher813 merged 1 commit into
mainfrom
fix/holiday-check-false-positive
Apr 9, 2026
Merged

fix: holiday check false positives on trading days#8
cipher813 merged 1 commit into
mainfrom
fix/holiday-check-false-positive

Conversation

@cipher813
Copy link
Copy Markdown
Owner

Summary

  • Root cause: trading_calendar.py used sys.exit(1) for holidays, but SSM reports Failed for any non-zero exit — crashes, missing files, venv issues all looked like holidays to the Step Function
  • Script now always exits 0 with stdout markers (TRADING DAY / MARKET_CLOSED). Step Function checks StandardOutputContent instead of SSM status
  • Added TradingDayCheckFailed state: infrastructure errors send an alert but proceed as trading day (fail-open) instead of silently skipping
  • Added InProgress/Pending polling loop — was missing, so unfinished SSM commands defaulted to holiday skip

Test plan

  • All 13 trading calendar tests pass
  • Step Function JSON validates
  • After merge: deploy Step Function via deploy_step_function_daily.sh
  • After merge: git pull on EC2 micro instance
  • Verify next trading day pipeline runs through CheckTradingDay → DailyData

🤖 Generated with Claude Code

…it codes

Root cause: trading_calendar.py used sys.exit(1) for holidays, but SSM reports
"Failed" for ANY non-zero exit (holidays, crashes, missing files). The Step
Function treated all non-Success SSM status as holidays, causing false skips
on normal trading days when the script failed for infrastructure reasons.

Three fixes:
1. Script always exits 0, prints TRADING_DAY or MARKET_CLOSED marker
2. Step Function checks StandardOutputContent for markers, not SSM status
3. Added TradingDayCheckFailed state — infrastructure errors proceed as
   trading day (with alert) instead of silently skipping the pipeline
4. Added InProgress/Pending polling loop (was missing — immediate default
   to holiday skip if SSM hadn't finished yet)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cipher813 cipher813 merged commit 1d85e55 into main Apr 9, 2026
1 check passed
@cipher813 cipher813 deleted the fix/holiday-check-false-positive branch April 9, 2026 13:55
cipher813 added a commit that referenced this pull request Apr 25, 2026
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) <noreply@anthropic.com>
cipher813 added a commit that referenced this pull request Apr 25, 2026
…ndows (#96)

* fix(universe_returns): use NYSE trading-day arithmetic for forward windows

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) <noreply@anthropic.com>

* universe_returns: import _add_trading_days from alpha-engine-lib v0.2.1

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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant