Repository navigation
Sim-to-paper fill agreement: replay a retained Alpaca paper trial through NautilusTrader rc5 (audit gap #7) - #217
Closed
seathatflowsinourveins wants to merge 13 commits into
Closed
seathatflowsinourveins wants to merge 13 commits into
seathatflowsinourveins wants to merge 13 commits into
Conversation
Replay the retained adaptive-paper trial 20260923g-main-passed's five order decisions (four filled, one canceled) through NautilusTrader 2.0.0rc5's BacktestEngine, fed with real Alpaca SIP quotes for the trial window, using the accepted engine-nautilus/equity-replay fee/fill-model baseline (FixedFeeModel at $0, native default FillModel, L1_MBP matching). Result: 3/5 fill agreement; the 3 orders with unambiguous paper timestamps agree with sub-bp fill-price deltas (mean 0.375 bps) and sub-second fill-time deltas (mean 0.78 s). Both disagreements trace to one declared gap: broker-orders.json has no recorded cancel timestamp for the canceled GOOGL sell, so the replay holds it open longer than the paper trial did. One trial, five orders: this is an agreement measurement, not a fill-rate or slippage calibration. The retained receipt records input hashes, data provenance (SIP quote page ledger sha256), engine version and fill-model parameters. Seventeen offline tests cover the comparison/decision-building logic against synthetic fixtures and the retained trial fixture; no network or credential file is touched by the tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…cy sweep, harden replay Independent review of blueprints/us-equities/sim-paper-compare/ found the disagreement between order 4 (canceled by paper, filled by the zero-latency sim) and order 5 was misattributed to unknown cancel timing (H1). The real cause is that order 4's limit was marketable for only ~69ms after submit while the paper broker's own fill latency (648ms-1.09s) let Alpaca cancel it first. build_decisions() now uses the actual recorded cancel timestamp from paper-output.json's requests log (or submit + order_timeout_seconds from the matching adaptive-paper config) instead of inferring a cancel time from the next order's submit; using the real recorded cancel time still produces 3/5 agreement, so the old explanation is withdrawn. A declared latency-sensitivity sweep (0/5/50/70/100/250/650/ 1000ms via StaticLatencyModel) is now run and committed to the receipt/README: agreement flips from 3/5 to 5/5 between 50ms and 70ms, matching order 4's marketable window. Also fixes, per the review: - M1: decisions are scheduled with clock.set_time_alert_ns at their exact recorded timestamps instead of polling on the next quote of any symbol; applied-decision log now records the clock's actual application time alongside the scheduled time. - M2: report hashes are computed over fills/orders rows with the random per-order init_id stripped; README's reproduce section uses a scratch --receipt path and diffs against the committed one instead of overwriting it. - M3: --replay never opens or requires a credential file. - M4: receipt now carries sim order status/reject reason, explicit engine latency_model/queue_position, a measured (not hardcoded) alpaca-py version, evidence_class, redacted argv, start/end times, exit code and a stdout hash. - L1-L7: per-page network/cache source tracking with a corrected "identical" claim; crossed quotes dropped and counted (locked quotes kept); signed side-aware slippage; DAY-only/integral-qty enforcement instead of silent truncation, correct partial-fill scoring by quantity, and the recorded-cancel fix above; network_interfaces dropped; ts_last/status/reject reason read from strategy.cache.orders() instead of a pandas tz-column conversion; corrected reuse claim against equity-replay (market orders on daily bars, not limit orders vs. quotes); ledger writes open/close per record instead of holding a handle open; --out now refuses a non-empty directory; private cache dirs are chmod 0700 explicitly (mkdir's mode= is masked by umask). Adds PageFetcher tests (replay/hash-mismatch/pagination), a receipt-consistency test that recomputes aggregates and checks inputs_sha256/runner_sha256, crossed/locked/ quantization normalization tests, and a gated pinned-runtime suite (skips outside nautilus_trader==2.0.0rc5) covering a tiny fixture where an order is marketable only briefly -- the shape of case that would have caught H1 and M1 -- plus a --replay run of main() with no credential file argument at all. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… provenance, argv-redaction bypass Both independent reviews (Claude and Codex gpt-6-astra) found the prior fix round's H1 explanation, latency claims and several receipt/README details wrong or overstated. H1 (the disagreement mechanism was backwards): order 4 (SELL GOOGL @338.42) was already marketable at submit (bid 338.44 since 16:46:37), not "becomes marketable at submit+69ms" as previously written. It stops being marketable at submit+69.273ms (raw SIP quotes) / 69.216918-69.217529ms (bisected actual engine flip, now the cited number). The 16:51:06.696Z timestamp cited previously was ~8ms after submit, an artifact of the old quote-polling implementation, not a crossing point. The paper broker's own fill lag on other orders is now stated as a hypothesis consistent with the data, not a demonstrated broker mechanism. Latency semantics: disclosed that the pinned engine (StaticLatencyModel) processes a deferred command at the next event for that instrument at or after the modeled arrival, not exactly at submit+latency (cites crates/backtest/src/engine.rs L883 at nautilus_trader 1b0a49d2). The sweep now reports a bisected flip boundary (latency_sensitivity_sweep.flip_bisections, 1us resolution) instead of a quote-derived window boundary, and the README documents the sparse-quote artifacts (order 3's 266.9/466.4ms lag, order 2's 100ms price-delta outlier) this causes. Clock provenance: submit timestamps are broker-clock, cancel timestamps are adaptive-paper's host clock (captured pre-send, no client_order_id recorded); measured +27.994 to +40.769ms host-ahead-of-broker offset for this trial, now in a new clock_provenance receipt section and README section. Cancel-request-to-order pairing (no client_order_id available) is now validated for temporal sanity rather than trusted positionally, and falls back to submit+order_timeout_seconds for all canceled orders in the trial if any pairing fails validation. Also fixes, per both reviews: - partial-fill timestamps now come from the actual OrderFilled event(s) (sim_by_id["fill_ts_ns"]), not order.ts_last, which could surface a later cancel's timestamp as the fill time. - README's per-order table and headline numbers are now generated from, and a new test cross-checks them against, the committed receipt (was stale: -0.84s vs. the receipt's -1.093899s for order 3). - argv redaction closed structurally: allow_abbrev=False rejects --pag/--ou/--rec outright, and the recorded argv is built from the parsed namespace (redact_args), not scanned from raw argv text. - stdout_sha256 now hashes the exact bytes written (including print's trailing newline); exit_code (always a misleading constant 0, since a failed run raises before a receipt is written) is removed; alpaca_py_version moved out of data_provenance into a new runtime_environment section with a note that this script fetches via urllib directly, not alpaca-py. - os.umask(0o077) is now scoped and restored, not left changed for the rest of the process. - checksum-ledger and "the suite recomputes report hashes" claims corrected to match what is actually demonstrated (same-page reproducibility, not independent-request reproducibility; a new gated test actually reruns run_replay from the retained pages and compares report hashes to the receipt). - the credential-loader-never-called claim is now verified by pre-seeding sys.modules with a fake runner.credentials() that raises if called, not just by asserting main() returns 0. - the M1 fixture no longer submits exactly on a quote timestamp (which could not have distinguished exact-instant scheduling from quote-arrival polling); it now asserts the applied-decision's clock_ts_ns equals the scheduled ts_ns exactly. - reproduce snippet no longer interpolates $PRIVATE_CACHE_DIR inside a quoted heredoc (which raised FileNotFoundError); the scratch-receipt path is now passed as argv and read via sys.argv[1]. New tests: cancel-boundary (a canceled sim order must not fill on a later marketable quote), partial-fill-then-cancel (native and synthetic), BUY/SELL slippage sign assertions on compare_orders' output, ambiguous-cancel-match refusal, clock provenance, argv-redaction abbreviation/equals-form coverage, report-hash reproducibility, and README-vs-receipt consistency. Three independent in-memory mutations (omitted cancellation, halved latency magnitude, reversed slippage side at the compare_orders call sites) were applied to scratch copies of replay_compare.py and confirmed killed by their respective new tests (cancel-boundary, latency-flip-bisection, BUY/SELL slippage-sign), with no false failures against the unmutated source. 73 tests pass on both system python3 (8 skipped: pinned-runtime-only) and the pinned runtime (nautilus-trader==2.0.0rc5, all 73 run). validate.py, validate_catalogs.py, landscape.py, evidence_manifest.py --check, verdict_review_gate.py and trading_gates.py --check all pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…, partial-fill honesty, mutation gaps
Both reviews found the prior round's bisection labels backwards, the latency
disclosure incomplete, and several test/wording gaps that let real mutations escape.
H1/bisection direction: bisect_fill_agreement_flip always labeled the lower latency
endpoint "agrees" and the higher one "disagrees", regardless of which way the actual
flip ran. For this trial it is the opposite (disagrees below the boundary, agrees
above it -- a lower-latency sim fills an order paper did not). The function now
returns each endpoint's actual measured agreement explicitly (lo/hi, each
{latency_ns, agrees}) instead of assuming a direction; the receipt and README are
corrected to match, and order 5 is now documented (and receipt-verified, not
assumed: independent_flip/depends_on_client_order_ids, computed by re-running the
replay with order 4 removed) as a dependent consequence of order 4's flip, not an
independent second one.
Latency disclosure completed: the pinned engine's advance_time_impl settles ALL
instruments' due timers together at each processed event, not just "the next event
for that instrument" as previously stated -- reproduced exactly as the reviewer
described (order A, quotes at 0/100ms, submitted at 10ms with 20ms latency, fills at
50ms against its existing book when order B, a different instrument, is submitted at
50ms) and added as a native test.
Partial-fill fixture made honest: the current engine configuration
(liquidity_consumption=False, the declared no-partial-fill baseline) ignores quoted
size entirely and always fills in full, so it cannot natively produce a
partial-fill-then-cancel case -- documented as such (with a test asserting the
full-fill boundary directly) rather than silently passing on a fixture that never
exercised the case. The fill_ts_ns extraction that matters for that case is now
factored into a standalone summarize_sim_order() function and tested directly
against constructed OrderFilled/OrderCanceled/OrderRejected events, independent of
the native engine's current boundary -- this is the test that actually kills a
ts_last-revert mutation now.
Other fixes: cancel-request pairing validation now checks every order (not just
canceled ones), catching a cancel request that lost its race to a fill from being
silently absorbed by a different canceled order; clock_provenance's submit pairing
is now count-checked; the host-vs-broker clock offset is now stated as a lower bound
(host stamps before send, broker stamps after receipt); "decision-to-fill latency"
reworded to "broker submission-to-fill interval" (both timestamps are broker-
reported); the four-fill range corrected to 0.65-1.09s (was excluding order 5's
0.648s); the "processed the cancel before a fill" claim removed as unmeasured;
--trial/--receipt are now trimmed to repo-relative/basename in argv instead of
recorded verbatim; sweep points now carry full per-order rows, not just aggregates,
so every per-order figure cited in prose is traceable to receipt data; stdout is
now written with sys.stdout.write (works when stdout has no .buffer attribute, e.g.
under `python -m unittest -b`, which previously raised AttributeError after the
receipt was already saved).
Test-order-dependence fixed: the umask-restoration test previously compared against
whatever the ambient process umask happened to be, so it passed even without a
restore if an earlier test's main() call had already left the process umask
changed; it now sets a known, distinctive umask immediately before the call.
The five mutations from the reviewer's mutation harness (halved latency, ts_last
revert, umask not restored, stdout hash without the trailing newP, bisection labels
swapped) are now each killed by a specific behavioral test, not only by the
runner_sha256 hash-equality test -- verified by rebuilding each mutant in an
isolated mirror tree and running the full suite against it.
92 tests pass on system python3 (12 skipped: pinned-runtime-only) and on the pinned
runtime (nautilus-trader==2.0.0rc5, all 92 run, including under `python -m unittest
-b`). validate.py, validate_catalogs.py, landscape.py, evidence_manifest.py --check,
verdict_review_gate.py and trading_gates.py --check all pass.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… shifted-flip detection, exact stdout bytes
Both reviews confirmed round 3's core fixes reproduce; this round corrects remaining
precision issues and closes the last surviving mutations.
1. README wrongly blamed order 1 for the 100ms sweep-point price outlier; the
receipt's own per-order rows show order 1 is unchanged across 50-650ms while
order 2 changes specifically at 100ms (120.55 at neighboring 70/250ms vs 120.54
at 100ms) -- order 2 is what makes 100ms an outlier relative to its neighbors.
Corrected in both the README and the test comment/assertions that had the same
error.
2. Settlement-semantics wording was imprecise: "next event for that instrument"
understated that eligible settlement events are specifically due order-command
TIMERS (any order's submit/cancel being applied), not any event -- a plain
market-data quote for another instrument does not trigger cross-instrument
settlement. Verified and added as a new native quote-only control test
alongside the existing timer test; cited crates/backtest/src/engine.rs
L1559-1585/L1716-1747; updated the receipt's own `latency_model_semantics`
metadata string (previously stale) and regenerated.
3. Cancel-request pairing rewritten around a sound rule (a cancel request is
attributed to an order only if it is the *unique* open, not-yet-terminal order
across the whole trial at that instant, chronologically evaluated so an
earlier-resolved canceled order becomes terminal for later requests) --
replacing the previous heuristic, which both wrongly accepted a race-lost
cancel for an unrelated earlier-submitted filled order and wrongly refused a
valid timeout cancel merely because another symbol's unrelated order was
submitted in between. New behavioral tests reproduce both of the reviewer's
exact scenarios plus the input-order-independence (tie) case, a case where two
requests matching the same order leave another unmatched, and a case where a
long-open filled order could otherwise steal a pairing slot from a genuinely
canceled order -- all of which the code now refuses (never guesses, never
raises) rather than mis-accepting.
4. `clock_provenance()` no longer reports offsets from a truncated, misaligned
zip when submit-request and order counts disagree (a constructed case reached
~195 seconds of bogus "offset"); it now reports `null` with an explicit
`..._unavailable_reason`. Both the README and a new test (which forces
`counts_match=True` to confirm the offsets are genuinely gated on it) cover
this.
5. `classify_flip_dependence` now scans the entire declared latency sweep for the
counterfactual (other order removed) run, not just the original bisection
bracket's two endpoints, and retains every counterfactual observation in the
receipt (`counterfactual_sweep`). Probing only the original endpoints could
misread a flip that merely *shifted* elsewhere in the sweep as one that
disappeared; a new pure-Python test (via a controlled fake of the underlying
per-latency check, no native runtime needed) constructs exactly that shifted
case and confirms it is still correctly reported independent.
6. `stdout_sha256` now hashes the exact bytes written to `sys.stdout.buffer` when
one exists (the normal case for a real process); only when it doesn't (e.g.
unittest's `-b` StringIO capture) does it fall back to text-mode `.write()`,
and the receipt now records which basis applies (`stdout_sha256_basis`:
`exact_bytes_written` vs. `utf8_lf_normalized_text`). New tests cover both
paths, including one whose fake stdout deliberately mangles newlines in its
text `.write()` to prove the buffer path is what actually gets used.
7. Two previously-surviving mutations (halving the latency actually passed to
run_replay inside the sweep loop specifically, and dropping per-order rows
from sweep points) are now killed by new tests that call run_latency_sweep
live against the retained trial, rather than only reading the already-generated
committed receipt (which a code mutation in the generator can't affect).
8. "Moving the cancel instant later within the plausible clock-offset range"
was backwards: the host clock leads the broker clock, so the broker-clock
instant corresponding to a recorded cancel request is earlier, not later,
than the raw host-clock number used. Corrected in both the docstring and the
README.
Also: the cancel-pairing rewrite's two innermost per-request guards ("target must
be a canceled order", "target not already paired") were found to be provably
subsumed by the algorithm's final accumulated-pairing-equals-canceled-ids check
for every reachable single-request-window scenario; removed as redundant, with a
mutation-tested exception (a filled order open across multiple request instants
can still take a pairing slot meant for a genuinely canceled order) that only the
final check catches -- confirmed by a dedicated new test and the mutation run
below.
Every mutation from both reviewers' mutation harnesses (23 total, including this
round's new ones for items 1-7 above) is now killed by a specific behavioral
test, verified by rebuilding each mutant in an isolated mirror tree and running
the full suite against it.
103 tests pass on system python3 (16 skipped: pinned-runtime-only) and on the
pinned runtime (nautilus-trader==2.0.0rc5, all 103 run), with and without
`python -m unittest -b`. validate.py, validate_catalogs.py, landscape.py,
evidence_manifest.py --check, verdict_review_gate.py and trading_gates.py
--check all pass.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… honest test comment, pairing-rule assumptions Three small Codex fixes plus one Claude nit, all confirmed empirically before writing: 1. Settlement wording (replay_compare.py:1115, README:132, receipt latency_model_semantics) understated the eligible triggers as only order-command timers. There are two: (a) an order's own instrument's next quote (what produces order 3's 266.9ms/466.4ms lags when no timer intervenes), and (b) any due clock timer the engine processes at all, from any source, including one with no order effect whatsoever -- verified with a new native test that schedules a bare no-op timer at 50ms (via a new, test-only extra_timers_ns hook on run_replay, empty by default) and confirms it alone moves order A's fill from 100ms to 50ms, exactly like a real order's decision timer would. A foreign instrument's quote alone still does neither. 2. stdout_sha256_basis selection used hasattr(sys.stdout, "buffer") while the actual write used getattr(..., "buffer", None) is not None -- these disagree whenever a wrapper has a buffer attribute that is itself None. Both now use one condition, computed once. New test uses a fake stdout with buffer = None to confirm the recorded basis and the actual write path agree. 3. A test comment claimed its fixture could only be caught by the accumulated- pairing check; tracing it through shows the *second* request actually finds zero open orders and is refused by the primary per-request check first. Corrected the comment to say so (the fixture that genuinely requires the accumulated check, added last round, is a different one and its comment was already accurate). 4. The cancel-pairing README called the open-order-uniqueness rule "sound" without qualification. It is sound only under three assumptions (one cancel request per canceled order; no un-logged broker-side cancellations; host/broker clocks aligned within the pairing tolerance), none of which the code checks. Documented both, plus two constructed counterexamples: (a) a repeated DELETE for one order paired with a different, un-logged-cancel order submitted in between; (b) a DELETE that loses its race to a fill inside the measured >=41ms host-clock lead. Noted that logging client_order_id on cancel requests (adaptive-paper/transport.py:502 calls before_request(kind) with no order identifier) would remove the heuristic entirely. Confirmed (not assumed) that this retained receipt is unaffected: order 4 is the only open order at its cancel-request time in this trial. 105 tests pass on system python3 (18 skipped: pinned-runtime-only) and on the pinned runtime (nautilus-trader==2.0.0rc5, all 105 run), with and without `python -m unittest -b`. validate.py, validate_catalogs.py, landscape.py, evidence_manifest.py --check, verdict_review_gate.py and trading_gates.py --check all pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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. |
The host clock leads the broker's, so a race-lost cancel request appears after the fill, which excludes the targeted order and permits misattribution. The previous text described the opposite direction, which yields a refusal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gitleaks' generic-api-key rule flagged two 32-hex request-cache digests stored under a field named "key" (a false positive; they are sha256 prefixes of the request path and parameters). Renamed in the receipt-facing page events only; the private cache ledger format is unchanged. Receipt regenerated with --replay; only the field names, runner_sha256 and run-local times changed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1461732 Follows the repository's .gitleaksignore convention for reviewed historical false positives: exact commit:path:rule:line fingerprints only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Superseded by #218. It is the same final tree, carried as one commit on current main. Commit 1461732 on this branch stored two request-cache digests in the receipt under a field named 🤖 Generated with Claude Code |
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 24, 2026
…ough NautilusTrader rc5 (audit gap #7) (#218) Replays paper trial 20260923g-main-passed (5 orders) through NautilusTrader 2.0.0rc5's simulated exchange on the same SIP quotes. Agreement is 3/5 at 0-50 ms latency and 5/5 from 70 ms, with a bisected flip at 69.217 ms: order 4 was marketable for 69.273 ms after submit, and order 5 depends on it. Engine settlement semantics, clock provenance and the cancel-pairing assumptions are disclosed. This is one trial and an agreement measurement, not a calibration. Reviewed across five rounds by Claude and Codex (#217). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins
deleted the
claude/sim-paper-fill-compare-20260924
branch
September 25, 2026 18:47
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
1 participant
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.
What
Trading-lane audit gap #7 (sim-to-paper agreement), first measurement.
blueprints/us-equities/sim-paper-compare/replay_compare.pyreplays the retained Alpaca paper trial20260923g-main-passed(5 orders) through NautilusTrader 2.0.0rc5's simulated exchange. It uses the same SIP quotes, retained as a hash-ledgered page cache. It then scores sim-vs-paper fill agreement per order.This is one trial and an agreement measurement, not a calibration.
Result (
receipts/20260923g-main-passed.json)Bisected flip. The simulator disagrees with paper at 69.216918 ms and agrees at 69.217529 ms. Order 4 was already marketable at submit and stopped being marketable 69.273 ms later. With less latency than that, the simulator fills it; paper did not.
Disclosed engine semantics (pinned rc5
engine.rs). A delayed command settles on either of two events:A foreign instrument's quote alone does not settle it. So sweep times and prices are not a function of the latency L alone.
Clock provenance. Submit times come from the broker. Cancel times come from host stamps taken before send, and the host leads the broker by at least 27.99–40.77 ms. Cancel pairing uses an open-order uniqueness rule that refuses ambiguous cases. Its assumptions and two constructed counterexamples are stated in the README. Logging
client_order_idon cancel requests would remove the heuristic.Review
Five fix rounds with two model families.
--replay, and ran a 200k-trial differential fuzz of the pairing rule. They mutation-tested with about 25 mutants, each now killed by a behavioural test.Checks.
-b.manifests/evidence.json.--replayagainst the retained cache.Not covered
🤖 Generated with Claude Code