Feat/evening pnl equity backfill (daily-only P&L export) - #99
Conversation
- broker: get_full_portfolio_history() — full 1D equity history from Alpaca portfolio_history (5-year lookback, skips pre-funding rows) - notifier: send_document() — sends CSV file via Telegram sendDocument API - notifier: build_weekly_csv() — Date/NAV/Daily P&L/Return%/Drawdown%/SPY columns; SPY data from yfinance; degrades gracefully on failure - pipeline: run_weekly() — orchestrates fetch → CSV → Telegram send - main: --mode weekly added to choices Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fill; rename weekly→daily CSV - Evening TG notification no longer includes the P&L history text table - db.backfill_equity_close(): backfills NULL equity_close rows caused by Alpaca API lag; triggered each evening run for the prior N-day window - Renamed weekly → daily: P&L CSV export now runs Mon-Fri at 09:00 ET (run_weekly/build_weekly_csv → run_daily/build_daily_csv; --mode daily) - notifier: send_document() retained for daily CSV Telegram delivery Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
Resolved and pushed in commit |
|
@copilot I only want daily mode, I don't want weekly mode anymore |
Done in |
There was a problem hiding this comment.
Pull request overview
This PR shifts the P&L export workflow from a weekly-compatible flow to a daily-only CSV export, removes the evening message’s embedded P&L history table, and adds an automatic DB “self-heal” to backfill missing equity_close values caused by Alpaca portfolio_history lag. It also improves operational visibility by ensuring startup/config failures can still trigger a Telegram notification.
Changes:
- Replace legacy weekly export paths with a daily CSV export (
--mode daily,run_daily(),build_daily_csv()), and update session-result formatting accordingly. - Add
db.backfill_equity_close()and invoke it during the evening run to fill prior NULLequity_closerows once the API catches up. - Remove the evening Telegram P&L history text table and update tests to reflect the new daily-export behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_weekly_report.py | Updates tests to target build_daily_csv() / run_daily() and daily-mode session formatting. |
| tests/test_notifier.py | Removes coverage for the now-deleted evening P&L history text table helper. |
| tests/test_db.py | Adds test coverage for the new backfill_equity_close() gap-filling behavior. |
| src/storage/db.py | Introduces backfill_equity_close() to update only NULL equity_close rows. |
| src/pipeline.py | Adds evening-run backfill loop for prior dates and renames weekly export orchestration to run_daily(). |
| src/notifier.py | Switches session formatting from weekly to daily, deletes the evening P&L history table code, and renames CSV builder to build_daily_csv(). |
| main.py | Replaces weekly CLI mode with daily and restructures startup to instantiate notifier/try-finally earlier for crash visibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Daily P&L CSV export (PR #98). | ||
|
|
||
| Covers: build_weekly_csv settlement math (close-to-close, drawdown, return), | ||
| Covers: build_daily_csv settlement math (close-to-close, drawdown, return), | ||
| SPY column population + graceful degradation, broker.get_full_portfolio_history | ||
| ET-date mapping + pre-funding skip, send_document, run_weekly orchestration, | ||
| and the format_session_result weekly body. | ||
| ET-date mapping + pre-funding skip, send_document, run_daily orchestration, | ||
| and the format_session_result daily body. |
| elif mode == "daily": | ||
| rows = result.get("rows", "?") | ||
| filename = result.get("filename", "") | ||
| lines.append(f"📊 {rows} rows → {filename}") |
Review fixes on top of the daily-only P&L export:
- pipeline: guard the equity_close backfill against 0.0 / NaN / inf /
negative portfolio_history points. A backfilled value targets
NULL-only rows, so a corrupt write could never be corrected by a
later run; NaN additionally binds as SQL NULL (fake success log
every night). Mirrors the prev_close > 0 guard 16 lines above and
the eq == 0.0 pre-funding skip in get_full_portfolio_history.
- pipeline: reword the backfill comment — it claimed today's bar
'isn't settled' while the branch above trusts and stores it.
- scheduling: --mode daily previously had NO automated invocation
path (run_if_et_window.sh rejects it by design, no timer existed)
while this PR deletes the evening P&L table it replaces. Add
scripts/run_daily_export.sh (.env sourcing + 300s timeout) and
tracked units scripts/systemd/quant-agent-daily.{service,timer}
(Mon-Fri 09:00 America/New_York, Persistent=true).
- notifier: daily 'sent' is now silent (the CSV document push IS the
confirmation — was 2 pushes per weekday); 'error' now carries the
reason (a bare 'status: error' is undebuggable from a phone);
_status_emoji classifies 'sent' as success; docstring noise-policy
list gains the daily entry; drop stale history-table comment.
- main: honest crash-visibility comment (when .env was never sourced
the Telegram creds are missing too, so the FAILED push is dropped —
that scenario still needs an external dead-man's switch); missing
config file exits with the path in the message (str(SystemExit(1))
is just '1'); graceful live-scheduler exit pushes a clear
'scheduler_exited' status instead of 'returned non-dict result'.
- tests: rename test_weekly_report.py -> test_daily_report.py; +10
tests pinning the backfill loop (today excluded in both branches,
value guard, per-date error isolation), the early-crash FAILED
push, and the daily noise policy. 1236 passed.
- docs: CLAUDE.md + README updated for the daily mode, its timer,
and the removed evening P&L table.
Refs #99
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review summary (multi-agent review, 5 dimensions + adversarial verification)Core design is sound and merged with fixes (commit d337242). What the review found and the fix shipped on this branch: Data integrity (major): the backfill loop wrote any Operability (major): Notifications: daily Honesty fix: the main.py crash-visibility comment claimed to fix the missing-Saturday-report incident, but in that exact scenario (.env never sourced) the Telegram creds are missing too and the FAILED push is silently dropped. Comment now states the real coverage + the dead-man's-switch gap. Tests/docs: |
Summary
db.backfill_equity_close(): automatically backfills NULL equity_close rows caused by Alpaca API lag, triggered each evening rundailyflow, Mon–Fri at 09:00 ET)--mode weekly,run_weekly()alias, and weekly formatter/CSV aliases)Deploy steps (run on different server after merge)
1. Find your existing weekly CSV timer/service (name may vary by deployment)
2. Migrate to daily timer/service and remove weekly mode invocations
--mode daily