Repository navigation
Fix three engine defects exposed by live SIP paper trials; first passed 1x trial recorded - #123
Merged
seathatflowsinourveins merged 6 commits intoSep 23, 2026
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2a0b7352a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 23, 2026
Independent review (2 lenses, each finding verified) confirmed 9 findings: - Safety: a halt-flagged quote with a zero or crossed price is no longer dropped; it fails closed, so its halt signal cannot be lost. A quote is dropped as untradable only when every other field is well-formed and its symbol is subscribed; malformed or unsubscribed ones still fail the transport. - Drops are counted per reason and per symbol in transport health and written to the run output (dropped_quotes); tests show a dropped quote leaves the stored quote and its age unchanged and that quote_stale still fires when only invalid quotes arrive. - Evidence wording: the recorded stop cause is callback_failure at quote_normalization; a crossed quote is the most likely cause, backed by the committed measurements; the unretained 9/56,855 sample is labelled an operator observation; the 10:11 explanation is hedged. The transport isolation check (restart log lines only 16-27 ms after stop()) and the frozen 1e2c96a hash list are now committed with their script. New defect found by the next runs (integration build 796e3d1, 11:42 and 11:5x ET): transport.mark_reconciled refused unless the whole event queue was empty, which on SIP it never is, so every run failed at the first 30 s reconciliation (traceback retained via an unchanged-runner wrapper). It now refuses only while an order event is queued. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 23, 2026
…s closed, SIP reconciliation) Adds #123's second commit to the 1e2c96a-based integration build: halt-flagged, malformed or unsubscribed invalid quotes fail closed; drops counted per reason and symbol and written to the run output; mark_reconciled refuses only while an order event is queued. Not for merge; engine suites pass on this tree. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…ailing the transport Both 2026-09-23 1x paper trials stopped early. The 11:27 ET rerun from Codex's stop-diagnostics build 1e2c96a recorded why: decision_exit transport_gap with callback_failure at quote_normalization. normalize_quote raised TransportError on a crossed SIP quote (bid above ask across exchanges), the consume loop froze the transport, and runner.py stopped on the non-stale reason. Crossed books are routine on SIP (one 60 s sample over the trial universe: 9 of 56,855 quotes; committed measurements: 1 of 51,116 and 1 of 50,633). normalize_quote now raises InvalidQuote('crossed'|'one_sided'), a TransportError subclass; the streaming consume loop drops those, counts them in health['dropped_quotes'] and keeps the last valid quote until it goes stale. Malformed quotes (bad timestamp, negative size or price, NaN) still fail the transport, and the REST snapshot path already skips invalid quotes per symbol. Also corrects the earlier diagnosis: the 'trading stream websocket error, restarting ... sent 1000' line is emitted when the engine closes its own streams at shutdown (the transport alone stayed healthy for 25 s). Evidence: adaptive-paper/trials/20260923b-needs-attention/ with a reproducible read-only quote-validity script. The adaptive-paper-broker-trial gate is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Independent review (2 lenses, each finding verified) confirmed 9 findings: - Safety: a halt-flagged quote with a zero or crossed price is no longer dropped; it fails closed, so its halt signal cannot be lost. A quote is dropped as untradable only when every other field is well-formed and its symbol is subscribed; malformed or unsubscribed ones still fail the transport. - Drops are counted per reason and per symbol in transport health and written to the run output (dropped_quotes); tests show a dropped quote leaves the stored quote and its age unchanged and that quote_stale still fires when only invalid quotes arrive. - Evidence wording: the recorded stop cause is callback_failure at quote_normalization; a crossed quote is the most likely cause, backed by the committed measurements; the unretained 9/56,855 sample is labelled an operator observation; the 10:11 explanation is hedged. The transport isolation check (restart log lines only 16-27 ms after stop()) and the frozen 1e2c96a hash list are now committed with their script. New defect found by the next runs (integration build 796e3d1, 11:42 and 11:5x ET): transport.mark_reconciled refused unless the whole event queue was empty, which on SIP it never is, so every run failed at the first 30 s reconciliation (traceback retained via an unchanged-runner wrapper). It now refuses only while an order event is queued. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
… order ids The 11:54 ET trial (integration build 885663a with both SIP fixes) made the first real engine round trip on the paper broker: INTC buy 1 filled 120.40, recovery sell 1 filled 120.39, flat, net -0.01 USD. It stopped at 43.3 s with adapter_error: the adapter built TradeId(broker_order_id + ':cum:' + qty), and a real Alpaca order id is a 36-character UUID, so the id was 42 characters against Nautilus's 36-character limit. The offline fake uses short ids. fill_trade_id() now derives a deterministic 36-character digest of broker order id and cumulative quantity. A native test with UUID-shaped ids fails on the old construction. Evidence, with a read-only broker order readback: adaptive-paper/trials/20260923e-first-roundtrip/. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…1d512; gate unchanged) Trial f, 2026-09-23 11:59 ET: 300.1 s duration_completed, 8 real paper orders and 8 native fills over four 1-share round trips, 13 crossed quotes dropped, reconciled flat with cash and positions matching, realized -0.29 USD; a read-only broker order readback matches. The build includes unmerged Codex commits (1e2c96a), so adaptive-paper-broker-trial stays not_established until those and this PR are on main and a main-built trial passes. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins
force-pushed
the
claude/crossed-quote-tolerance-20260923
branch
from
September 23, 2026 16:06
071851e to
90132ae
Compare
…pending order timers Delta review of #123 (2 new confirmed findings, 4 partials): - Major: the runner read transport health before the periodic snapshot await and then called mark_reconciled, so a serious gap that arose during the snapshot (for example orders_disconnected) could be cleared without stopping. Health is now re-read after snapshot and reconcile; any non-stale reason stops the run instead. The interval is a named constant (RECONCILE_EVERY_SECONDS) so a test can reach the branch; the test fails on the previous runner. - Minor: mark_reconciled cleared pending order-update timers on every acknowledgement; they now survive unless the orders stream actually dropped, so a silent trade_updates stream is still detected. - Partials: dropped-quote counts are also written when run_native raises; the 10:11 explanation is hedged to 'possible' with the retained-rate chance (about 15-45 percent) and the note that the isolation check ran on 1e2c96a's transport; the interrupted README no longer states the crossed quote as recorded. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
… chance to the measured range Addresses the GitHub Codex-connector threads on the measurement script: malformed nonzero prices are classified as 'other' instead of raising; the window starts at the first delivered quote; and the output records the config hash, symbol list, script hash and observed window. The -v2 re-measurements found 36 of 32,419 crossed (1e2c96a transport) and 0 of 38,251 (fixed transport) in later minutes, so the crossed-quote rate varies widely by minute, and the 10:11 explanation now states a range of about 15 percent to near certainty. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins
deleted the
claude/crossed-quote-tolerance-20260923
branch
September 23, 2026 16:46
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 23, 2026
…(paper rung ready) Trial adaptive-20260923g ran from a read-only git archive of main 843dc8f (with the three #123 engine fixes): 300.5 s at 1x on SIP, 5 real paper orders with one cancel-replace and 4 native fills over two 1-share round trips (INTC, GOOGL), 134 crossed quotes dropped, reconciled flat with cash and positions matching, realized -0.28 USD; the read-only broker order readback matches. The receipt's broker_readiness is now passed (previous block kept under broker_readiness.previous); the gate moves to established, native_proven; trading_gates.py reports rung_ready.paper true. Evidence of engine and broker order flow at 1x with 1-share orders, not of strategy edge. The leverage-ladder rungs keep their own receipts. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 23, 2026
…(paper rung ready) Trial adaptive-20260923g ran from a read-only git archive of main 843dc8f (with the three #123 engine fixes): 300.5 s at 1x on SIP, 5 real paper orders with one cancel-replace and 4 native fills over two 1-share round trips (INTC, GOOGL), 134 crossed quotes dropped, reconciled flat with cash and positions matching, realized -0.28 USD; the read-only broker order readback matches. The receipt's broker_readiness is now passed (previous block kept under broker_readiness.previous); the gate moves to established, native_proven; trading_gates.py reports rung_ready.paper true. Evidence of engine and broker order flow at 1x with 1-share orders, not of strategy edge. The leverage-ladder rungs keep their own receipts. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 23, 2026
…(paper rung ready) (#128) * Establish adaptive-paper-broker-trial on a main-built 1x paper trial (paper rung ready) Trial adaptive-20260923g ran from a read-only git archive of main 843dc8f (with the three #123 engine fixes): 300.5 s at 1x on SIP, 5 real paper orders with one cancel-replace and 4 native fills over two 1-share round trips (INTC, GOOGL), 134 crossed quotes dropped, reconciled flat with cash and positions matching, realized -0.28 USD; the read-only broker order readback matches. The receipt's broker_readiness is now passed (previous block kept under broker_readiness.previous); the gate moves to established, native_proven; trading_gates.py reports rung_ready.paper true. Evidence of engine and broker order flow at 1x with 1-share orders, not of strategy edge. The leverage-ladder rungs keep their own receipts. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * Record the replaced precondition in the gate note and date its history; fix the README hash wording Independent review of #128: the numbers and the full-duration run check out (147 decisions over 299.04 s, largest gap 2.11 s), but the note still stated the earlier 1e2c96a-on-main precondition and a pending receipt as current. The note now records the precondition as replaced by those direct checks, with the comparison that would overturn it, and labels the older text as history. The README names git archive 843dc8f as the hashed tree. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * Reflect the adaptive paper trial in runtime-target and the dashboard checkpoint GitHub review threads on #128: catalogs/us-equities/runtime-target.json broker_status and observability/grand-dashboard/state.json (paper lane and alpaca-paper-operational gate rows, recorded_at) still described the trial as not started. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * Keep the dashboard state values within the public-token alphabet progress.py accepts only [A-Za-z0-9_. /+()-] up to 160 characters; the previous values used ':' and ','. CI caught it (tests.test_grand_dashboard). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> --------- Co-authored-by: seathatflowsinourveins <[email protected]> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
9 of 22 tasks
5 of 8 tasks
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.
Summary
Six bounded 1x Alpaca paper trials on 2026-09-23 exposed three latent engine defects. None shows up offline: the fixture quotes and ids are too clean.
normalize_quoteraised on routine SIP crossed books, which causedcallback_failureand then a stop.mark_reconciledrequired a completely empty event queue, which SIP never has, so every run failed at the first 30 s reconciliation.TradeId(uuid + ':cum:' + qty)is 42 characters, and Nautilus allows 36, so every real Alpaca fill raised.Evidence (
adaptive-paper/trials/)20260923b-needs-attention/: Codex's diagnostics recordingcallback_failureatquote_normalization, the read-only quote-validity and transport-isolation scripts with their outputs, and the frozen hash list.20260923c-reconcile-refusal/: the traceback, captured with an unchanged-runner wrapper.20260923e-first-roundtrip/: the first real round trip (INTC) and the TradeId failure, with a broker readback.20260923f-first-passed/: the first passed 1x trial from integration build5e1d512(1e2c96aplus these fixes).Gate:
adaptive-paper-broker-trialstaysnot_establisheduntil Codex's1e2c96acommits and this PR are on main and a main-built trial passes.Review
7805fba, including the halt-signal safety regression.Checks
python3 -m unittest: 3073 OK.validate.ymlsteps pass, and guarded gitleaks overorigin/main..HEADis clean.🤖 Generated with Claude Code