Repository navigation
Paper engine: fix the retroactive cross-family review findings on #221, #242 and #284 - #350
Merged
seathatflowsinourveins merged 2 commits intoSep 26, 2026
Conversation
#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) <noreply@anthropic.com>
…ort'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) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Collaborator
Author
|
Foundation ACK from native-agent-stack-9a for
The planned hold stands: merge after the stacked follow-up for the open realized-loss major is up. |
seathatflowsinourveins
deleted the
claude/paper-engine-crossfamily-fixes-20260926
branch
September 26, 2026 16:57
seathatflowsinourveins
pushed a commit
that referenced
this pull request
Sep 26, 2026
Conflicts came from #350's squash: this branch already held its commits. tests/test_adaptive_paper_safety.py keeps this branch's side. source-hashes.json and manifests/evidence.json take main's side, then re-pin only this PR's files. Git's automatic merge of safety.py re-inserted the pre-fix _reconcile_execution_accounting after the fixed one, and Python would bind that later definition. safety.py is restored from c81a3d5 (main's copy equals its parent c1539cd), so the delta against main is exactly the reviewed c1539cd..c81a3d5 change. Paper suites: 407 OK; validate.py passed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Sep 26, 2026
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 26, 2026
closes #350's unverified startup-reconciliation boundary) (#356) * Paper engine: persist startup FillReports through the observation hook 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Scout <scout@local> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Scout <scout@local> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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) <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
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.
Why
This PR fixes the defects from retroactive cross-family reviews of merged paper-engine PRs. GPT-6 (gpt-6-astra, effort max) re-examined main at ca8e37f because those PRs merged while Codex was rate-limited. #243 (margin financing) passed.
cancel_expiredthen cancelled the exit it protected.replay_comparesilently skipped a named cancel stamped before its submit.configure.py's initialization could overwrite a concurrent registration.What
cad7faf. GPT-6 implementers fixed each finding test-first in their own worktrees, and every new regression failed before its fix. The coordinator then integrated the fixes:
source-hashes.jsonandevidence.json;runner_sha256, which were updated as in Record which order each paper cancel request was for; pair sim-to-paper cancels exactly #242;replay_compare.pysource with a dated limitation. An AST check shows the 8 definitions sim-capacity uses are unchanged.c1539cd (repair round). Independent verification (8 Sonnet verifiers, one per finding, each mutation-checked) found 7 of 8 fully resolved. #221 finding 1 was unresolved in production: the fix persisted fills through
port.controller, which only the testSimulatedPorthas. The repair:sink_observation, the hook every real transport is built with (runner.pyfresh_port,mover_runner.py, the native-faults harness);SimulatedPortexpose the same hook, so tests and production share one path;build_decisions-level cancel-chronology tests, including the equality boundary.A second Sonnet mutation check of the repair confirmed the production wiring at every real construction site. Reverting the repair fails 3 of 45 native tests, including
Decimal('1') != Decimal('3')on the real-shaped port.Checks
TMPDIRoutside/tmp: runner, native, safety, transport, sessions, metrics, the source-hashes manifest, sim-paper-compare and sim-capacity. 761 tests OK, 2 skipped.tests.test_observability_backends_alerts, 19 OK.python3 -m unittestat cad7faf: 6,199 tests, 689 skipped. The one failure istest_pre_commit_gate, caused by gitleaks per-user lock contention from another session; it passes when re-run alone (3 OK).validate.py,evidence_manifest.py --checkandvalidate_convergence.py --all-recordedpass.Cross-family re-review of c1539cd: needs_changes (one pre-existing major, open)
GPT-6 confirmed that every finding above is resolved on the real production paths. It found one more major, which predates these commits: the gross realized loss that the loss caps read still depends on delivery order. For example, two shares at a 100.00 basis, sold as one at 110.00 and one at 90.00:
A GPT-6 implementer is fixing it test-first in a stacked follow-up PR, so this PR carries only verified fixes. It stays open until that follow-up is up.
Boundaries
FillReports is not verified here as durable./tmp/.giton the host. An empty/tmp/.git, probably left by a sandboxed Codex job, makes the credential guard reject temporary fixtures under/tmpin local runs. The checks above setTMPDIRoutside/tmp.SOTA sources
blueprints/us-equities/adaptive-paper/(safety.Ledger,native_adapter,transport) andobservability/backends/, at base 5008c30.file_sdtarget format, and its label-value rules as checked byprometheus/common/modelLabelSet. The Paper alert: scrape only registered adaptive-paper exporters, and alert only on a registered silent one #284 label validation follows them.alpaca/trading/client.pyatcc4cb3b7).🤖 Generated with Claude Code