Vesper Review — fix/halt-reason-taxonomy-leak¶

To: Orion (he/him), Katja (Captain) From: Vesper (she/her) CC: Atlas (he/him) Date: 2026-04-21 Status: APPROVED — apply and merge

Verdict: APPROVED¶

2 commits, 4 new FLAG-041 tests, 251/251 regression green. Spec compliance confirmed on all points. No deviations. Apply instructions correct (defensive branch delete, Get-ChildItem form). No corrections required.

Spec Compliance¶

Both clobber sites fixed (Option A): main_loop.py:4282 and run_paper_session.py:415 both drop halt_reason=HALT_REASON_ENGINE_REQUESTED. _shutdown now falls through to existing_reason path in both cases. ✅

Unchanged paths correct: KeyboardInterrupt, unhandled exception, duration-elapsed, and signal handler paths all still pass an explicit halt_reason argument — these are the cases where the argument IS the authoritative source (no prior writer). Correctly untouched. ✅

Unused import removed: HALT_REASON_ENGINE_REQUESTED import dropped from run_paper_session.py. Constant retained in main_loop.py for external consumers. ✅

Comments: Both fixed sites carry FLAG-041-tagged comments explaining the halt_reason or existing_reason precedence and why the argument was dropped. The run_paper_session.py comment explicitly names S42 as the bug case — correct, since that was the actual path. ✅

Test 1 (canonical S42 regression): Seeds halt.reason=inventory_truth_halt + halt.detail=degraded_timeout_exceeded_300s, calls _shutdown with no kwarg, asserts both survive, AND explicitly asserts halt.reason != "engine_requested_halt". This is the right test — it guards exactly the S42 failure mode on exactly the code path S42 took. ✅

Tests 2–3 (risk and reconciler paths): Extend coverage to non-inventory-truth halt tokens. Same preservation invariant. ✅

Test 4 (fallback safety): No prior halt.reason, kwarg-less shutdown writes HALT_REASON_UNEXPECTED. Defensive net confirmed. ✅

Teardown: Tests use :memory: DB with no TemporaryDirectory — LIFO addCleanup pattern is not required here. setUp/tearDown with try/except on close() is correct for in-memory tests. ✅

Pre-existing failure noted: test_summarize_paper_run.py::test_summary_uses_existing_persisted_state KeyError: 'total_paper_orders_created' — reproduced on main independently. Pre-existing, unrelated to FLAG-041. Accepted.

Deviations¶

None. Combining fix commits 1 and 2 into a single commit was explicitly allowed by Vesper's ruling and was the right call — the diff is cohesive (same conceptual change in two files). 2 commits instead of 3.

Apply Instructions¶

cd C:\Users\Katja\Documents\NEO GitHub\neo-2026
git fetch
git checkout main
git pull
git branch -D fix/halt-reason-taxonomy-leak 2>$null
git checkout -b fix/halt-reason-taxonomy-leak
Get-ChildItem "C:\Users\Katja\Documents\Claude Homebase Neo\02 Projects\NEO Trading Engine\patches\fix-halt-reason-taxonomy-leak" -Filter "*.patch" | Sort-Object Name | ForEach-Object { git am $_.FullName }
python -m pytest tests/test_halt_reason_lifecycle.py -v

Expected: 8 passed.

Then merge to main and push.

What This Means¶

After this merge, a session that halts via DEGRADED timeout (the S42 pattern) will surface:

Field	Pre-fix	Post-fix
`halt.reason`	`engine_requested_halt`	`inventory_truth_halt`
`halt.detail`	`degraded_timeout_exceeded_300s`	`degraded_timeout_exceeded_300s` (unchanged)

S43 session summary will show the authentic halt attribution if the guards fire again. FLAG-041 closed on merge.

FLAG-042 (no DEGRADED recovery path for non-truth guards) remains deferred per Atlas ruling — out of scope for this branch.

— Vesper