orders: distinguish Rejected from Inactive once the ibapi string collapses them (ibx#250) - #394
Open
userFRM wants to merge 1 commit into
Open
orders: distinguish Rejected from Inactive once the ibapi string collapses them (ibx#250)#394userFRM wants to merge 1 commit into
userFRM wants to merge 1 commit into
Conversation
…apses them (ibx#250) The wire distinguishes 39=8 (Rejected) from 39=I (Inactive) and the engine already models them as separate OrderStatus values, but order_status_str collapses both to the ibapi string "Inactive" — correctly, since ibapi has no Rejected string. Everything downstream of that string therefore saw one value for two different situations: a dead order and a parked one that can still reactivate. completed_status already carries the discriminator the two need: it is non-empty for every terminal status (Filled, Cancelled, Rejected) and stays empty for Inactive, since an Inactive order is not completed. That field is now read wherever the collapsed string is read back apart from OrderStatus itself — is_open_or_reactivatable checks it beside is_open_status, and TrackedOrder carries its own rejected flag for the path that only has a status string and no RichOrderInfo to consult. Widening is_open_status to admit "Inactive" outright was the trap to avoid, since that string is also every rejected order; the new function and flag exist so the open-order snapshot can tell the two apart without touching is_open_status. drain_open_orders and collect_open_orders both used to filter on is_open_status alone, so a genuinely-Inactive order vanished from req_open_orders along with the rejected ones it is indistinguishable from once stringified. Both now call is_open_or_reactivatable, and collect_open_orders's locally-tracked path sets TrackedOrder.rejected at the same place its status string is written, update_order_status, which now takes the pre-stringification OrderStatus for exactly that reason. The reason a 39=I transition carries — tags 58 and 103, already parsed alongside the reject-only warn log — had nowhere to go: completed_status stays empty for Inactive by design, so a caller reading OrderState never saw why the gateway parked the order. It now reaches Wrapper::error the same way a cancel/modify reject already does, through a small queue on OrderState that the engine populates only for a genuine Inactive transition, and both the Rust and Python dispatch loops drain it into an error callback alongside the existing order_status "Inactive". Closes deepentropy#250.
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.
Problem
The wire distinguishes
39=8(Rejected) from39=I(Inactive), and the engine models them as distinct states — butorder_status_strcollapses both to the string"Inactive", correctly, because ibapi has no Rejected string. From that point a caller cannot tell a dead order from a held one, and three consequences follow.The reason is dropped. Tags 58 and 103 are parsed and logged, but reach the API only for
39=8, throughOrderState.completed_status. A39=Itransition surfaces as a bare status change with the reason nowhere a caller can reach.The order disappears from the snapshot, and it has to.
is_open_statusomits"Inactive"— and cannot simply admit it, because after stringification that value is also every rejected order. Widening it would resurrect genuinely dead orders. So a reactivatable order is hidden by a filter defending itself against the conflation above, contradicting the engine one layer down, which deliberately models Inactive as non-terminal and reactivatable.A replace the gateway parks looks like success. Every client-visible surface asserts the modify succeeded while
req_open_ordersreturns nothing for that order.What this changes
order_status_stris untouched — both still map to"Inactive", which is the ibapi contract.completed_statusis the discriminator. It is already non-empty only for a terminal status and empty otherwise, and a rejection falls back to the literal"Rejected"when tag 58 is absent, so it is never empty for one.is_open_or_reactivatable(status, completed_status)admits"Inactive"only when it is empty.is_open_statusitself is not widened, which is the trap the issue names.TrackedOrdercarries whether it was rejected, set at its one write choke point. That function now takes theOrderStatusenum rather than the already-collapsed string, so the distinction is read before it is lost and the compiler enforces the call sites.A
39=Itransition delivers its reason through the wrapper'serror()callback — the same path a cancel reject already uses — alongside the status change.Note for merging
This changes
ClientCore::update_order_status's signature. #362 changes the same function for an unrelated reason (an order recovered from an earlier session has no local entry to update). The two are independent but will want resolving together.Tests
Eleven, covering the discriminator in all four quadrants, both
collect_open_ordersadmission sites, thedrain_open_ordersfilter, theTrackedOrderwrite, the ccp gating condition including the Rejected/Inactive conflation trap, the reason formatting, and the dispatch plumbing.Each production line touched was independently reverted and the intended test confirmed to fail by name.
Closes #250.