Skip to content

Restore trading_calendar.py — universe_returns still imports it#48

Closed
cipher813 wants to merge 1 commit into
mainfrom
fix/restore-trading-calendar-for-universe-returns
Closed

Restore trading_calendar.py — universe_returns still imports it#48
cipher813 wants to merge 1 commit into
mainfrom
fix/restore-trading-calendar-for-universe-returns

Conversation

@cipher813
Copy link
Copy Markdown
Owner

Summary

Bug from the ae-dashboard de-bloat split: #43 moved `trading_calendar.py` to alpha-engine-dashboard on the incorrect assumption that the Step Function's `CheckTradingDay` was the sole consumer. Spot smoke test surfaced that `collectors/universe_returns.py:215` also imports it:

```python
from trading_calendar import is_trading_day as nyse_is_trading_day
```

Broke DataPhase1 on the spot with `ModuleNotFoundError: No module named 'trading_calendar'`.

Fix

Restore `trading_calendar.py` in this repo (byte-identical to the version now in alpha-engine-dashboard). Unblocks the spot migration.

Trade-off

Dual-copy across two repos. Drift risk. Architecturally correct fix: move to `alpha-engine-lib` (the PR I opened + closed earlier today — lib#3). That needs private-lib git auth wired into the spot, which is real scope. Added to ROADMAP.md as a P2 — do it in a calmer week.

Also surfaced in the smoke run (not addressed in this PR)

`fundamentals` collector reports `status=ok` in the summary but the inner log shows `Fundamentals fetched in 26.7s: 0 OK, 903 errors, 903 total`. Silent-failure bug — violates the "no silent fails" standard. Existing issue, predates this session. Separate follow-up needed. Likely FMP API key / auth / rate-limit problem on the spot's environment.

🤖 Generated with Claude Code

The ae-dashboard de-bloat split (#43) moved this file to
alpha-engine-dashboard on the (incorrect) assumption that the Step
Function's CheckTradingDay was the sole consumer. Spot smoke test
surfaced that collectors/universe_returns.py:215 also imports it:

    from trading_calendar import is_trading_day as nyse_is_trading_day

which broke the spot migration's DataPhase1 workflow
(ModuleNotFoundError in weekly_collector.py --phase 1).

This is a dual-copy smell (same file now lives in both repos) —
flagged in ROADMAP.md. The architecturally correct fix is to move
it into alpha-engine-lib so all consumers pull from one source of
truth, but that requires wiring up private-lib git auth on the spot
which is out of scope for the Saturday-unblock window.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cipher813
Copy link
Copy Markdown
Owner Author

Superseded by the lib-based approach (architecturally correct). Adding trading_calendar to alpha-engine-lib in cipher813/alpha-engine-lib#4; new data-repo PR will update universe_returns.py to import from lib and bump the lib pin, without restoring the local copy.

@cipher813 cipher813 closed this Apr 16, 2026
cipher813 added a commit that referenced this pull request Apr 16, 2026
Closes the loop on the de-bloat split regression. collectors/universe_returns.py
was importing `from trading_calendar import is_trading_day`, which broke
when #43 moved the file out of this repo. Rather than restore a local copy
(the patchwork approach in the now-closed #48), import from
alpha_engine_lib.trading_calendar — a single source of truth shared across
all Alpha Engine modules.

- collectors/universe_returns.py: `from alpha_engine_lib.trading_calendar ...`
- requirements.txt: bump alpha-engine-lib pin @v0.1.1 → @v0.1.3

Requires cipher813/alpha-engine-lib#4 merged + tagged v0.1.3 first.
Spots pip-install from the git tag, so the new module appears at
install time without any other plumbing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cipher813 added a commit that referenced this pull request Apr 16, 2026
Closes the loop on the de-bloat split regression. collectors/universe_returns.py
was importing `from trading_calendar import is_trading_day`, which broke
when #43 moved the file out of this repo. Rather than restore a local copy
(the patchwork approach in the now-closed #48), import from
alpha_engine_lib.trading_calendar — a single source of truth shared across
all Alpha Engine modules.

- collectors/universe_returns.py: `from alpha_engine_lib.trading_calendar ...`
- requirements.txt: bump alpha-engine-lib pin @v0.1.1 → @v0.1.3

Requires cipher813/alpha-engine-lib#4 merged + tagged v0.1.3 first.
Spots pip-install from the git tag, so the new module appears at
install time without any other plumbing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cipher813 added a commit that referenced this pull request Apr 16, 2026
#49)

Closes the loop on the de-bloat split regression. collectors/universe_returns.py
was importing `from trading_calendar import is_trading_day`, which broke
when #43 moved the file out of this repo. Rather than restore a local copy
(the patchwork approach in the now-closed #48), import from
alpha_engine_lib.trading_calendar — a single source of truth shared across
all Alpha Engine modules.

- collectors/universe_returns.py: `from alpha_engine_lib.trading_calendar ...`
- requirements.txt: bump alpha-engine-lib pin @v0.1.1 → @v0.1.3

Requires cipher813/alpha-engine-lib#4 merged + tagged v0.1.3 first.
Spots pip-install from the git tag, so the new module appears at
install time without any other plumbing.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cipher813 added a commit that referenced this pull request Apr 23, 2026
The deploy-drift preflight (alpha-engine-predictor, PR #48) enforces
"SF Comment stamp + CF stack git-sha tag == cipher813/alpha-engine-data@main
HEAD" and halts the pipeline on mismatch. But infrastructure/deploy-infrastructure.sh
had no GHA and was manual-only, so every main merge that touched anything
other than infrastructure/ silently accumulated drift. Caught on 2026-04-23
13:05 UTC — weekday SF halted 11 commits behind.

Adds .github/workflows/deploy-infrastructure.yml that runs the deploy script
on every push to main + workflow_dispatch. No path filter: every commit
restamps, ~30s of idempotent update-stack + update-state-machine calls per
merge. Cost is negligible, eliminates the whole drift class.

IAM: extends github-actions-lambda-deploy role with six new Sids scoped to
what CloudFormation actually calls during a tag-propagating stack update.
Verified via create-change-set — CF propagates the git-sha tag to all 22
resources in the template, so the role needs TagResource on cloudwatch,
events, sns, scheduler + SF UpdateStateMachine on both pipelines + Lambda
AddPermission/RemovePermission (Lambda::Permission resources show
Conditional replacement when their SourceArn reference is dynamic).

Template-content changes (new alarm, new rule) will still fail loudly with
AccessDenied until the specific resource action is added — intentional,
same discoverability as the existing Lambda deploy role.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cipher813 cipher813 deleted the fix/restore-trading-calendar-for-universe-returns branch May 18, 2026 15:33
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