Paper engine: replay fills in execution order per symbol, never into a short (#355 follow-up) — Claude review pending

[seathatflowsinourveins]

What

This is the #355 follow-up: book fills in execution order without creating a new failure mode. An earlier attempt, reverted in #355, was refuted because it could reorder a fill into a short position and freeze the transport, and because its replay was global. This design takes the #355 reviewers' constraints as requirements.

1. Exact execution time. Each path records the execution's own time:

   • stream fills use Alpaca's top-level trade_updates timestamp (not order.updated_at);
   • FILL recovery and startup persistence (Paper engine: persist startup FillReports through the observation hook (closes #350's unverified startup-reconciliation boundary) #356) use transaction_time.

   The time is stored losslessly as executions.execution_time_ns. The ledger schema_version moves to 2, so the previous release refuses to open a migrated ledger, and the execution insert names its columns.

2. Never reorder into a short. Replay is per symbol. Timed fills are substituted only into timed journal slots; adoptions, untimed fills and uncovered provisional bookings keep their positions. If the time-ordered pass would go short, that symbol falls back to observation order, which is always feasible. It raises only if the observation pass itself fails, which would mean corruption.

3. Targeted trigger. Replay runs only when the triggering order was provisional, or when it has an execution earlier than an already-booked timed fill of the same symbol. Money is tracked per symbol, so for a ledger written by this PR one symbol's journal cannot block another symbol's fills (see Limitations for ledgers written by main).

4. Re-derive at open. When the accounting-rule marker isn't current, the ledger re-derives at open, so _begin_trial sees the corrected loss before any new fill.

Tests (each fails on 460034e where the behaviour is new)

Test	Result on 460034e
A/B/C across five deliveries, expecting cash 70, realized −15, loss 25, 1 @ 115	5 late-delivery cases fail
B2, both arrival orders, expecting flat, cash 75, realized −25, loss 35, no rollback or freeze	both fail (loss 25)
An unreplayable journal cannot block another symbol's fill	raises the replay-short error
Partial coverage reorders only covered bookings; uncovered bookings anchor the timed replay	fail
Stream, recovery and startup paths carry exact ns to the ledger	fail
The previous open check refuses the migrated schema	fail
Old markers re-derive before any new fill (4 cases), including the pre-fix marker	fail

• Revert check (coordinator): with the three production files reverted, 18 failures and 1 error.
• Paper suites (coordinator, pinned runtime, private TMPDIR): safety, runner, native and transport ran 425 tests, all OK. Source-hashes plus trading gates: 33 OK. validate.py passed.
• Authorship: GPT-6 (gpt-6-astra, effort max, workspace-write) implemented this test-first, and the coordinator verified it.

Review status (2026-09-29). The repair (20a32965..1afd7522) got its cross-family Claude re-check from an Opus 5.5 evidence-reviewer (effort max): no code findings, and the body corrections below applied. An Opus 5.5 stack-verifier then ran a local merge preview of main cf3fb72e plus 1afd7522. The adaptive-paper suites, the trading gates and validate.py all passed (verdict approve).

Limitations

• Synthetic tests only; there are no broker orders. The 401 paper keys were replaced on 2026-09-28.
• Untimed execution rows are immutable. An execution recorded without a time keeps its row, journal slot and NULL time. A FILL transaction_time that arrives later, at startup or recovery, is ignored for such a row, so the fill keeps journal order. Its realized P&L, gross loss and loss-cap checks can therefore differ from execution-order accounting (repair fixture: loss 20 against 25; a cap of 25 does not halt).
• Marker cases. Five marker cases re-derive at open: no marker, the pre-fix marker, an out-of-date rule key present, and the two old-marker variants covered in the Tests table.
• Open-time isolation is limited. It applies only to symbols that already have a per-symbol money checkpoint, and only ledgers written by this PR have one. On the first open of a ledger written by main, one corrupt symbol still fails the whole open atomically. Isolation covers only the two replay SafetyErrors (accounting_replay_quantity_mismatch can also raise); other malformed-journal errors still fail the open.
• An isolated symbol is recorded, not contained (review nit, follow-up). Nothing reads the failure event, and buys on that symbol stay allowed until a later fix refuses entries on failed symbols.
• Test naming (review nit, follow-up). test_later_replay_cannot_reprice_untimed_history means untimed rows keep their row, slot and NULL time. A late timed fill can still move an earlier timed fill past an untimed one (20a3296's slot substitution), so the untimed fill's basis can change.
• Uncovered bookings stay provisional.
• Replay latency on a long-lived ledger is unmeasured; no fresh latency acceptance is claimed.

SOTA sources

• Alpaca trade_updates (execution timestamp, execution_id): https://docs.alpaca.markets/docs/websocket-streaming
• Alpaca account activities (FILL transaction_time): https://docs.alpaca.markets/docs/account-activities
• QuantConnect LEAN 985ef30: per-fill closing P&L in Common/Securities/SecurityPortfolioModel.cs; losses separate in Common/Statistics/TradeStatistics.cs

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[seathatflowsinourveins]

Independent Claude review at 20a32965: VERDICT: needs_changes

This was a headless claude -p session on Opus 5.5 at max effort: 54 turns and $10.01. It is a separate session identity from the GPT-6 author and from the coordinator.

[major] safety.py:985: a timestamp backfill onto an existing untimed fill changes the execution order but does not trigger the replay.

• What goes wrong: the branch at :985-988 writes execution_time_ns onto an untimed execution row, which moves that fill into the time-sorted set. The trigger at :1132-1141 fires only when an order was provisional, or when its earliest execution is earlier than an already-booked timed fill.
  • A newly timed fill that is later than every timed fill, but sits earlier in the journal, changes the order without firing the trigger.
  • The rule marker is already current, so reopening the ledger does not correct it.
• Reproduced through the real startup path (_generate_fill_reports, native_adapter.py:897-902):
  • Setup: 2@100 adopted, then untimed B 1@130 and A 1@110, with A executed first and B observed first; then a new sell C 1@90.
  • If the broker lists the sell first, the ledger ends with loss 20, realized −20, average 110.
  • A full replay gives loss 25, realized −15, average 115, which matches the hand-computed execution-order result.
  • With max_gross_loss_usd=25, one delivery order halts with gross_loss_cap_reached and the other keeps trading.
  • A later, unrelated replay moves loss from 20 to 25 silently.
• Also affected: it reproduces on a ledger written by the real 460034e0 release. The same gap applies to fill-gap recovery (:715-723) and to stream fills without a timestamp (transport.py:1296).
• Against the PR: it contradicts the stated limitation that legacy rows "stay untimed and keep their journal positions". The only test that reaches this branch checks for double booking, not for the resulting order.
• Fix directions: either make a backfill run _replay_symbol, or don't backfill legacy rows.

[minor] tests/test_adaptive_paper_safety.py:798: the four marker subtests all delete execution_accounting:rule, so they test one condition. No test covers a rule key that is present but holds an older value, so "4 cases, including the pre-fix marker" overstates the coverage.

[minor] safety.py:459/1094: after a rule-marker change, one unreplayable (corrupted) symbol makes Ledger() raise and stops the whole ledger from opening. "One symbol's journal cannot block another symbol's fills" therefore does not hold at open, and "raises only if the observation pass fails" leaves out accounting_replay_quantity_mismatch.

[minor] transport.py:331-335: more than 9 fractional digits silently drops the whole fraction (123456789 ns early). A lowercase z, which RFC 3339 allows, raises, and on the stream path that freezes the transport. The parser is pre-existing, so the impact is low.

Verified OK:

• The stream path uses the top-level timestamp; the test can tell it apart from updated_at.
• Recovery and startup use transaction_time_ns.
• Schema v2: the insert names its columns, and the 460034e0 Ledger refuses the migrated file.
• Migration keeps legacy times NULL, reopens byte-identical, and nothing is counted twice.
• reserve_intent refreshes risk after open.
• A hand-computed probe matches for both arrival orders and after full re-derivation.
• The B2 fallback and per-symbol isolation hold during fills.
• Replay latency grows linearly: about 26 ms per replaying fill at 2,000 bookings, and 55 ms at 4,000.

Checks run by the reviewer:

• The three paper suites: 246 tests, OK.
• validate.py: passed.
• The source-hash and gates-bindings tests: 19, OK.
• sha256 of all 7 changed files matched the manifests.
• 11 probes ran.

Coordinator reproduction on a merge preview (main + head):

• 425 paper-suite tests, OK.
• validate.py: passed.
• The base-revert check gives 18 failures and 1 error, as the PR states.

Next: one bounded repair round. GPT-6 writes the fix test-first in its own worktree, preferring "don't backfill legacy rows", which adds no replay cost. A headless Claude re-check of the repair diff and a GPT-6 read-only re-check follow. Merge waits for a main-free window in the token lane's queue.

[seathatflowsinourveins]

Repair round pushed as 1afd7522. The PR is now PARKED: on 2026-09-27 the user parked the paper lane to focus on the foundation.

GPT-6 (gpt-6-astra, effort max) implemented the repair test-first in its own worktree, and the coordinator verified it. What changed:

• [major], safety.py:974: untimed executions keep NULL times and their journal positions, and a timestamped re-delivery is a no-op (returns False). No replay is added, so there is no latency cost. Before and after, from the reviewer's own probes:
  • Before: startup order sell,buy gave live ('20','-20','110'), while a full replay gave ('25','-15','115'). Backfill order B,A halted with gross_loss_cap_reached.
  • After: both startup orders give ('20','-20','110') live and on full replay. Both backfill orders give loss 20 and no halt.
• [minor], test_adaptive_paper_safety.py:1642: a new marker subtest where the rule key is present but out of date.
• [minor], safety.py:1099: re-derivation at open isolates both replay SafetyErrors, keeps the failed symbol's booked money, and records the symbol and reason. Healthy symbols re-derive normally. Limit: a corrupted legacy symbol with no money checkpoint still fails atomically, because assigning zero would erase unknown accounting.
• [minor], transport.py:321: accepts a lowercase z and truncates fractions beyond 9 digits (RFC 3339 §5.6).

Checks

• 8 new regression methods fail on 20a32965 and pass at 1afd7522. With only the production files reverted: 12 failures and 6 errors.
• Coordinator run, with TMPDIR outside the worktree:
  • safety, runner, native and transport suites: 435 run, OK;
  • source-hashes plus trading gates and bindings: 45 run, OK;
  • validate.py: passed.
• GPT-6's own run of the full suite showed 2 failures and 4 errors in the runner's CredentialFilePermissions class. The cause was the brief's TMPDIR inside the worktree, which those fixtures refuse. The same failures reproduce on the baseline under that TMPDIR.

Before this merges, whenever the paper lane reopens:

1. A cross-family Claude re-check of the repair diff (20a32965..1afd7522).
2. PR-body corrections:
   • untimed rows are immutable;
   • five marker cases;
   • open-time isolation is limited to symbols with a checkpoint;
   • accounting_replay_quantity_mismatch can also raise;
   • no fresh latency acceptance is claimed.
3. A main-free window.