Skip to content

Paper engine: persist startup FillReports through the observation hook (closes #350's unverified startup-reconciliation boundary) - #356

Merged
seathatflowsinourveins merged 2 commits into
mainfrom
claude/paper-startup-fill-durability-20260926
Sep 26, 2026
Merged

seathatflowsinourveins merged 2 commits into
mainfrom
claude/paper-startup-fill-durability-20260926

Conversation

@seathatflowsinourveins

Copy link
Copy Markdown
Collaborator

What

This closes the boundary that #350 left unverified. Startup reconciliation reported every execution of an adopted open order to NautilusTrader as a FillReport, but it never persisted those executions to safety.Ledger. The fix sends each adopted execution through the transport's sink_observation hook before the reports are exposed. This is the same path FILL-activity recovery already uses (native_adapter.py, around lines 692-721). _adopted_executions already refuses an incomplete or non-contiguous execution prefix, so nothing partial is persisted.

Verification (test-first)

Seven regressions are in tests/test_adaptive_paper_native.py (StartupFillDurability). The fixture has two open orders (a buy and a losing sell) and four executions. It uses the real AlpacaPaperTransport observation hook wired to Controller.observe. Only the external activity read is stubbed, and the HTTP boundary raises if reached.

Boundary Test Before the fix After
(a) no later observation needed test_startup_reports_survive_reopen_without_later_observation FAIL: execution table empty after reopen pass
(b) restart does not double-book test_restart_does_not_double_book_previously_recorded_startup_fills pass pass
(c) reports equal durable executions test_startup_report_quantities_and_prices_match_durable_executions FAIL: 4 reports, 0 durable executions pass
(d) stream/REST redelivery is a no-op test_stream_and_rest_redelivery_after_startup_is_idempotent FAIL: redelivery re-derived provisional P&L (−0.579992 → −0.58) pass
fresh snapshot adoption test_fresh_snapshot_adoption_records_history_without_rebooking_positions FAIL: history absent pass
async sink awaited test_startup_awaits_async_observation_sink FAIL pass
sink failure aborts reports; retry idempotent test_startup_sink_failure_aborts_reports_and_retry_is_idempotent FAIL: hook bypassed pass
  • Revert check (coordinator): with only native_adapter.py reverted to dbf887e, 6 tests fail; (b) passes on both.
  • Paper suites (coordinator, pinned runtime adaptive-paper-20260921, private TMPDIR): safety, runner, native and transport ran 412 tests, all OK. tests.test_trading_gates: 26 OK. scripts/validate.py passed.
  • Pins: the native_adapter.py entry in adaptive-paper/source-hashes.json moves fd0aa91d… → fe544c95…, and manifests/evidence.json re-registers the 3 files.
  • Authorship and review: GPT-6 (gpt-6-astra, effort max, workspace-write sandbox) implemented it test-first. The coordinator (Claude) reviewed the diff and re-ran every check above. An independent read-only Claude review follows in a comment.

Relation to #355: this also covers the adopted-order part of #355's review (finding 2). Adopted orders now have execution records from zero, so #355's per-execution replay reaches them. The two PRs touch different files.

Limitations

  • Synthetic tests only. There is no broker connection, and paper keys currently return 401.
  • The NautilusTrader side rests on rc5's installed Python interfaces (ExecutionMassStatus.add_fill_reports, FillReport). The compiled Rust implementation was not read.

SOTA sources

🤖 Generated with Claude Code

…k before native reconciliation

#350 left one boundary unverified: the fills of adopted open orders that
startup reconciliation reports to NautilusTrader as FillReports. They reached
the native engine but not safety.Ledger's execution table, so a crash before
any later observation lost them, the reports disagreed with the durable
record, and a later stream or REST redelivery re-derived provisional P&L.
Startup now persists each adopted execution through the transport's
sink_observation hook, the same path as FILL-activity recovery, before the
reports are exposed (NautilusTrader 2.0.0rc5 FillReport and
ExecutionMassStatus.add_fill_reports contract). _adopted_executions already
requires a complete, contiguous execution prefix before anything persists.

Seven regressions in tests/test_adaptive_paper_native.py (StartupFillDurability),
using the real AlpacaPaperTransport hook wired to Controller.observe; with the
fix reverted, 6 fail (restart-without-double-booking passes on both).
Paper suites: 412 tests OK; trading gates 26 OK; validate.py passed.
Implemented by GPT-6 (gpt-6-astra, effort max) test-first; verified by the
coordinator. Pins: source-hashes.json native_adapter.py, manifests/evidence.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@seathatflowsinourveins seathatflowsinourveins added the lane:trading Trading lane: sim, paper and live north star label Sep 26, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

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

seathatflowsinourveins pushed a commit that referenced this pull request Sep 26, 2026
…on order, versioned marker

Independent review of c81a3d5 (needs_changes):
- Finding 1 (major): the replay booked fills in observation order. Stream fills
  now carry Alpaca's own trade_updates timestamp (event_at_ns; FILL recovery
  forwards transaction_time_ns), the ledger keeps it in a new optional
  executions.at_ns column (added on open for existing ledgers), and replay
  fills each symbol's timed execution slots in execution order, tie-broken by
  event id then cumulative quantity. Adoptions, untimed executions and
  uncovered provisional bookings keep their journal positions.
- Finding 3: the marker is execution_accounting:v2:<client_id>, so ledgers
  replayed under the observation-order rule re-derive once.
- Finding 4: a test for an uncovered REST-only booking beside another order's
  late executions.
- Finding 2 (adopted open orders) is covered by #356's startup persistence.

The A/B/C case gives cash 70, realized -15, gross loss 25, and 1 share at 115
under stream, REST-ahead, recovered, late-buy and nanosecond deliveries.
Reverting the three production files fails 10 cases (4 new tests, 2 strengthened).
Paper suites: 411 OK; trading gates 26 OK; validate.py passed.
Implemented by GPT-6 (gpt-6-astra, effort max) test-first; verified by the
coordinator. Pins: source-hashes.json (safety, transport, native_adapter) and
manifests/evidence.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
source-hashes.json also pins test files; CI's
test_adaptive_paper_source_hashes_manifest caught the stale entries. Pins only,
no code change: the changed tests' entries and their manifests/evidence.json
registration.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@seathatflowsinourveins

Copy link
Copy Markdown
Collaborator Author

Independent read-only Claude review of 2f8ed96 (evidence-reviewer, Opus): VERDICT: accept, with no blocker or major finding.

Confirmed:

  • The observation fields match the activity and stream paths.
  • Validation of the full execution prefix happens before anything is persisted, and prior is cached only after the loop.
  • Nothing is double-booked against adopt_broker_snapshot, and a retry is idempotent.
  • rc5 1b0a49d2 fails closed on a failed fill report: generate_mass_status uses try_join!, then abort_startup, which the runner maps to needs_attention. This is source review, not execution.

Minor findings, dispositions:

  1. The per-execution timestamp is never asserted in tests. Follow-up in Paper engine: book realized P&L and gross loss per exact execution (realized-loss delivery-order major; follow-up to #350) #355, which introduces the event_at_ns/at_ns contract.
  2. The startup call should send event_at_ns=execution["ts"] once Paper engine: book realized P&L and gross loss per exact execution (realized-loss delivery-order major; follow-up to #350) #355 lands. That follow-up goes in Paper engine: book realized P&L and gross loss per exact execution (realized-loss delivery-order major; follow-up to #350) #355, merged after this PR.
  3. Non-final executions' fields are inert in production, because the ledger is already at the row's filled quantity. The test gap is recorded.
  4. No rc5 node test runs the new hook path (ScriptedPort has no sink_observation). Recorded residual.
  5. Test 4 demonstrates a ledger property rather than a runner flow; test 2 is a regression guard. Recorded.
  6. README-native.md doesn't mention startup persistence. Doc follow-up in Paper engine: book realized P&L and gross loss per exact execution (realized-loss delivery-order major; follow-up to #350) #355.

Outside this change: orders that became terminal while the engine was down never get execution records, so their ledger P&L stays at the rounded cumulative average. Recorded as a residual.

d32cab8 re-pins the changed test file in adaptive-paper/source-hashes.json (the CI validate failure). Pins only.

@seathatflowsinourveins
seathatflowsinourveins merged commit 60c9f0a into main Sep 26, 2026
24 checks passed
@seathatflowsinourveins
seathatflowsinourveins deleted the claude/paper-startup-fill-durability-20260926 branch September 26, 2026 17:58
seathatflowsinourveins pushed a commit that referenced this pull request Sep 26, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

# Conflicts:
#	manifests/evidence.json
seathatflowsinourveins added a commit that referenced this pull request Sep 26, 2026
…ealized-loss delivery-order major; follow-up to #350) (#355)

* Paper engine: fix the retroactive cross-family review findings on #221, #242 and #284

GPT-6 (gpt-6-astra, effort max) reviewed three merged paper-engine PRs on main ca8e37f; #243 passed.
- #221: activity-based gap recovery booked fills natively without persisting them to the durable ledger (native 3,
  durable 1); a continuing LULD pause auto-cleared after twelve minutes and let cancel_expired cancel the exit it
  protected; cash and realized P&L depended on REST-snapshot versus stream delivery order.
- #242: replay_compare scheduled a named cancel stamped before its submit and silently skipped it; cancelling the
  transport task cleared the cancel identity before the HTTP worker sent the DELETE.
- #284: a clean exporter stop released the port before deregistering, so it could remove a replacement exporter's
  target; configure.py's initialization could overwrite a concurrent registration; malformed nested labels passed
  validation and would make Prometheus reject the whole target list.

Each fix was implemented test-first by a GPT-6 implementer in its own worktree; every new regression failed before
its fix. Coordinator integration: source-hashes.json and evidence pins refreshed; the 20260923g sim-paper-compare
receipt replayed from its retained pages with the new code (identical apart from run-local fields and runner_sha256,
updated as in #242); sim-capacity's frozen replay_compare source re-pinned with a dated limitation (AST check: the
8 definitions sim-capacity uses are unchanged).

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Paper engine repair round: persist recovered fills through the transport's real observation hook

Independent verification of cad7faf found #221 finding 1 unresolved in production: reconcile_fills persisted recovered
fills only through a port.controller attribute that no real Alpaca port has (AlpacaPaperTransport and the native-faults
FaultTransport are wired with sink_observation=controller.observe); only the test-only SimulatedPort carried it.
- native_adapter.py: persist through the port's sink_observation (the hook the transport already uses for stream rows),
  after validating that the recovered activities tile the fill contiguously (fail closed otherwise).
- simulation.py: SimulatedPort exposes the same sink_observation hook, so tests and production share one path.
- Tests: a real-shaped port (no .controller) regression that failed on cad7faf with Decimal('1') != Decimal('3'), restart
  persistence, incomplete tiling; the two accounting-replay guards in safety.py; fast build_decisions-level tests for
  the cancel-before-submit refusal and its equality boundary.
Implemented by a GPT-6 implementer test-first; 514 tests across the five affected modules pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Paper engine: book realized P&L and gross loss per exact execution in accounting replay

The accounting replay summed a status delta's executions into one notional
before computing P&L, so a sell filled at a gain and a loss inside one delta
netted the loss away, and the result depended on whether the stream or the
REST snapshot arrived first. Replay now books each execution that tiles the
delta separately (QuantConnect/Lean 985ef30: SecurityPortfolioModel.cs per-fill
closing P&L; TradeStatistics.cs accumulates losses separately), falling back to
the delta notional only while executions are incomplete.

Two regressions: mixed-profit gross loss across stream-first, REST-first,
duplicate and restart deliveries, and basis preservation at each booking.
With the fix reverted, 4 cases fail (rest_first, duplicates, restart, basis).
Paper suites (safety, runner, native, transport): 407 tests OK.
Implemented by GPT-6 (gpt-6-astra, effort max) test-first; verified by the
coordinator. Pins: source-hashes.json safety.py and manifests/evidence.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Paper engine repair round (#355 review): replay executions in execution order, versioned marker

Independent review of c81a3d5 (needs_changes):
- Finding 1 (major): the replay booked fills in observation order. Stream fills
  now carry Alpaca's own trade_updates timestamp (event_at_ns; FILL recovery
  forwards transaction_time_ns), the ledger keeps it in a new optional
  executions.at_ns column (added on open for existing ledgers), and replay
  fills each symbol's timed execution slots in execution order, tie-broken by
  event id then cumulative quantity. Adoptions, untimed executions and
  uncovered provisional bookings keep their journal positions.
- Finding 3: the marker is execution_accounting:v2:<client_id>, so ledgers
  replayed under the observation-order rule re-derive once.
- Finding 4: a test for an uncovered REST-only booking beside another order's
  late executions.
- Finding 2 (adopted open orders) is covered by #356's startup persistence.

The A/B/C case gives cash 70, realized -15, gross loss 25, and 1 share at 115
under stream, REST-ahead, recovered, late-buy and nanosecond deliveries.
Reverting the three production files fails 10 cases (4 new tests, 2 strengthened).
Paper suites: 411 OK; trading gates 26 OK; validate.py passed.
Implemented by GPT-6 (gpt-6-astra, effort max) test-first; verified by the
coordinator. Pins: source-hashes.json (safety, transport, native_adapter) and
manifests/evidence.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Re-pin the changed paper test files in adaptive-paper/source-hashes.json

source-hashes.json also pins test files; CI's
test_adaptive_paper_source_hashes_manifest caught the stale entries. Pins only,
no code change: the changed tests' entries and their manifests/evidence.json
registration.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Reduce #355 to the reviewed-safe subset; defer execution-order replay

The re-check of cc6d7cb found two new majors in the execution-order replay:
- Moving an earlier-executed fill into another order's slot can make an
  uncovered REST-only sell go short. The replay then raises, the fill rolls
  back and the transport freezes, although the real history is consistent.
- Removing the early exit let that failure block fills in every symbol and
  added a full replay to every fill.
Under the bounded review loop (one review, one repair, then land with
residuals), this commit lands the parts both reviews confirmed.

It keeps:
- c81a3d5's per-execution P&L and gross loss within a delta;
- the versioned execution_accounting:v2 marker (F3);
- the uncovered-fallback test (F4), with its oracle stated for
  observation-order replay.

It reverts:
- the slot-substitution replay and the early-exit removal;
- the executions.at_ns column, which also avoids the unversioned schema
  change (re-check F3);
- the stream and recovery timestamp plumbing.

Execution-time ordering is a recorded follow-up, with the reviewer's
constraints: never reorder into a short (fall back to observation order), a
targeted replay trigger, and a schema version bump.

Revert checks: the v1 marker fails the F3 test; skipping the uncovered
fallback errors the F4 test. Paper suites: 409 OK; source-hashes and trading
gates: 33 OK; validate.py passed. Pins: safety.py and its test file only.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

---------

Co-authored-by: Scout <scout@local>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins added a commit that referenced this pull request Sep 26, 2026
…351, #355, #356) (#361)

- paper lane: the cross-family engine fixes (#350), startup-fill durability
  (#356) and per-execution loss booking (#355) are merged. The 1x ladder
  stays blocked on the paper keys (HTTP 401). Flat.
- mover-early-entry-studies gate: v3 round 17 is GPT-6 ready_to_freeze
  (#351) but not frozen; the dry run needs data keys. evidence_ref now
  points at the v3 README.

Co-authored-by: Scout <scout@local>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lane:trading Trading lane: sim, paper and live north star

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant