fix(notifier): evening 4pm-to-4pm P&L from portfolio_history (supersedes #95)#96
Merged
Conversation
…des #95) #95 aimed to report the official 4pm close-to-close P&L instead of the close-to-8pm-after-hours broker diff — a good goal — but its implementation had a systematic off-by-one-DAY bug: it differenced consecutive stored `account.last_equity` values, and Alpaca's `last_equity` is the PRIOR trading day's close (it's one day stale at the 20:00 ET evening run). So row T stored T-1's close, and `last_equity[T] - last_equity[T-1]` = (T-1 close) - (T-2 close) = YESTERDAY's P&L, displayed as today's. The equity line and the whole history table inherited the same one-day shift. Root cause: today's official 4pm close simply isn't available from the 8pm account snapshot — `last_equity` is yesterday's close, `total_value` is the 8pm after-hours value, and today's close only lands in Alpaca's `last_equity` tomorrow. You have to fetch it. Corrected approach: - broker.get_recent_daily_closes(): pull Alpaca portfolio_history (1D, extended_hours=False) — its LAST point IS today's official regular-session close. ET-date mapping mirrors scripts/export_alpaca_trades.py. Best-effort, never raises. - run_evening computes pnl_4pm = today_close - prior_close (gated on the latest bar actually being today, else it logs + falls back), stores today's close in daily_pnl.equity_close (new column), and passes pnl_4pm/equity_close into the result dict. - notifier headline shows the true 4pm-to-4pm P&L ("· 4pm close") when present, else falls back to the real-time prior-close→now diff (with the existing n/a-on-nonpositive-baseline handling). No off-by-one — equity_close is TODAY's close, not last_equity. Deliberately scoped vs #95: - Builds on current main, so it PRESERVES the recently-added evening banners (deterministic loss escalation, missing-session, stop-coverage) that #95 would have clobbered. - History table left on its current consistent basis (switching it to the 4pm basis needs equity_close backfilled across displayed rows — accumulates going forward; future work). - Drops #95's timezone-default change (ET → America/Chicago) and the unused account_asof_utc plumbing — orthogonal to the P&L fix; ET is the codebase convention. Tests: the off-by-one regression (real-time says +$1,200 incl AH but today closed -$500 → headline shows -$500, not +$1,200), real-time fallback, portfolio_history ET-date mapping, equity_close round-trip. Full suite 1155. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yebof
added a commit
that referenced
this pull request
May 29, 2026
Follow-up audit of the evening P&L / settlement reporting after the 4pm-to-4pm change (#96). All confirmed by adversarial verification. 1. [P2] notifier headline gated the 4pm path on a truthy `equity_close`, so a liquidated account closing at exactly $0.00 fell back to the real-time diff (0.0 is a valid close). → `equity_close is not None`. 2. [P2] deterministic loss-escalation banner evaluated the real-time daily_pnl while the headline showed the 4pm close-to-close P&L — on an after-hours swing the alert could fire (or stay silent) on a different number than the operator sees. Now prefers the same 4pm basis (pnl_4pm / equity_close), falling back to real-time when the 4pm figures are absent. 3. [P2] P&L history table rendered the real-time daily_pnl basis while the headline was 4pm — the SAME day could show two different P&L figures in one push. Table now anchors NAV (and the per-row Net P&L = NAV increment) on the stored equity_close when present, stepping by daily_pnl only for legacy rows that predate the column. Today's row now matches the headline. 4. [P2] daily_pnl INSERT OR REPLACE wiped a stored equity_close to NULL on a documented same-day evening re-run whose portfolio_history fetch failed. Both writers now upsert with ON CONFLICT(date) DO UPDATE + COALESCE(excluded.equity_close, daily_pnl.equity_close) to preserve it. 5. [P3] run_evening computed pnl_4pm_pct without guarding prev_close > 0 — a negative prior close (corrupted/underwater) would flip the return sign. Now gated `prev_close > 0` (else falls back to the real-time path). Deliberately NOT changed: the position snapshot's cash% denominator. It reports CURRENT holdings (8pm) and its total_value denominator is internally consistent with its 8pm invested; forcing a 4pm denominator there would introduce a basis mismatch, not remove one. Tests: +5 (equity_close=0 headline path, escalation 4pm-basis both directions, table 4pm consistency, equity_close preserve-on-reinsert). Full suite 1160. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrected version of #95. Same goal — show the official 4pm close-to-close P&L in the evening push instead of the close-to-8pm-after-hours broker diff — but #95 had a systematic off-by-one-DAY bug.
The bug in #95
#95 differenced consecutive stored
account.last_equityvalues. But Alpaca'slast_equityis the prior trading day's close (it's one day stale at the 20:00 ET run). So the row for day T stored T-1's close, andlast_equity[T] − last_equity[T-1]=(T-1 close) − (T-2 close)= yesterday's P&L, shown as today's. The equity line and the whole history table inherited the same one-day shift.Concretely: Mon close $100k, Tue close $101k, Wed close $100.5k. On Wed evening #95 shows
101k − 100k = +\$1kas "Wed P&L" — but Wed actually lost $500.Why it needs a fetch
Today's official 4pm close isn't in the 8pm account snapshot at all:
last_equity= yesterday's close,total_value= 8pm after-hours, and today's close only lands inlast_equitytomorrow.Fix
broker.get_recent_daily_closes()— pulls Alpacaportfolio_history(1D,extended_hours=False); its last point is today's official regular-session close. ET-date mapping mirrorsscripts/export_alpaca_trades.py. Best-effort, never raises.run_eveningcomputespnl_4pm = today_close − prior_close(gated on the latest bar actually being today, else logs + falls back), stores today's close in the newdaily_pnl.equity_closecolumn, and passespnl_4pm/equity_closeinto the result dict.· 4pm close) when present, else falls back to the real-time diff. No off-by-one —equity_closeis today's close.Scope vs #95
main, so it preserves the recently-added evening banners (deterministic loss escalation / missing-session / stop-coverage) that feat(notifier): evening notification reports 4pm official snapshot #95 would have clobbered.equity_closebackfilled across displayed rows — accumulates going forward; future work).account_asof_utcplumbing — orthogonal; ET is the codebase convention.Tests
Off-by-one regression (real-time says +$1,200 incl. AH but today closed −$500 → headline shows −$500), real-time fallback, portfolio_history ET-date mapping,
equity_closeround-trip. Full suite 1155 passed.Closes #95 (superseded).
🤖 Generated with Claude Code