Vesper Review — Branch #2: fix/summarize-paper-run-capital-overlay¶
To: Orion, Katja CC: Atlas From: Vesper
Verdict¶
APPROVED — merge when ready.
Both commits clean. One apply-command nit in the delivery memo (non-blocking, noted below).
Commit #1 — chore(halt-reason): use HALT_REASON_* constants¶
Pure refactor. All 7 literal → constant mappings verified correct:
| Literal (old) | Constant (new) |
|---|---|
"interrupted_sigbreak" |
HALT_REASON_INTERRUPTED_SIGBREAK |
"interrupted_sigint" |
HALT_REASON_INTERRUPTED_SIGINT |
"interrupted_sigterm" |
HALT_REASON_INTERRUPTED_SIGTERM |
"interrupted_other" |
HALT_REASON_INTERRUPTED_OTHER |
"engine_requested_halt" |
HALT_REASON_ENGINE_REQUESTED |
"duration_elapsed" |
HALT_REASON_DURATION_ELAPSED |
"unhandled_exception" (×2) |
HALT_REASON_UNHANDLED_EXCEPTION |
No behavior change. Branch #1 nit closed. ✅
Commit #2 — fix(summarize-paper-run): baseline-filter the capital-events overlay¶
Fix logic¶
_get_capital_delta_post_baseline(conn, asset) correctly mirrors StateManager.get_capital_delta_total:
- Baseline =
MIN(created_at) FROM inventory_ledger WHERE asset = ?✅ - No-baseline case returns
0.0(FLAG-030: pre-ledger capital assumed absorbed into seed) ✅ - SQL is structurally identical to the engine path ✅
- Read-only — no StateManager instantiation, no migrations ✅
- Lock-step contract documented in docstring ✅
_get_inventory_balance body change is minimal and correct — one inline SQL block replaced by one helper call. ✅
Test coverage — all 5 Atlas invariants verified¶
| # | Invariant | Test |
|---|---|---|
| 1 | Pre-baseline deposit excluded | test_pre_baseline_deposit_excluded |
| 2 | Post-baseline deposit included | test_post_baseline_deposit_included |
| 3 | Three-source equality (summarize == engine) | test_three_source_equality_mixed_history |
| 4 | basis_commit rows excluded from balance |
test_post_baseline_basis_commit_excluded |
| 5 | S39 fact-pattern closes (~$199, not ~$337) | test_s39_fact_pattern_matches_wallet |
Invariant 3 specifically verifies summarize._get_inventory_balance == StateManager.get_current_balance + StateManager.get_capital_delta_total. This is the correct three-source check for the summarize layer. ✅
Corrected test in test_flag_034_display_overlay.py¶
test_fallback_used_when_no_ledger_rows: 14.5 → 12.5 is correct. The old assertion was validating the bug (pre-baseline deposit included when no ledger baseline exists). New behavior: no ledger → no overlay → fallback only. FLAG-030 contract. ✅
Pre-existing failures — confirmed out of scope¶
test_run_paper_session.py(4):OrderSizeConfig.__init__()missingmax_size_pct_of_portfolio— upstream config schema drift, exists onmainindependently.test_summarize_paper_run.py(1):total_paper_orders_createdkey missing fromload_paper_run_summary— schema drift, exists onmainindependently.
Both sets confirmed pre-existing per Orion's disclosure. Candidates for FLAG-016 cleanup. ✅
Nit — Apply commands reference local-katja-main¶
The delivery memo's git checkout local-katja-main won't work — your branch is main. Use these instead:
git checkout main
git status # should be clean
git checkout -b fix/summarize-paper-run-capital-overlay
git am "C:\Users\Katja\Documents\Claude Homebase Neo\02 Projects\NEO Trading Engine\patches-summarize-overlay\0001-chore-halt-reason-use-HALT_REASON_-constants-in-run_.patch"
git am "C:\Users\Katja\Documents\Claude Homebase Neo\02 Projects\NEO Trading Engine\patches-summarize-overlay\0002-fix-summarize-paper-run-baseline-filter-the-capital-.patch"
python -m pytest tests/test_summarize_paper_run_overlay_baseline.py tests/test_flag_034_display_overlay.py tests/test_halt_reason_lifecycle.py -v
# Expected: 14 passed (5 new + 5 corrected + 4 Branch #1)
git checkout main
git merge --no-ff fix/summarize-paper-run-capital-overlay -m "Merge branch 'fix/summarize-paper-run-capital-overlay'"
git push
Approved. Katja can apply and merge.
— Vesper