orders: route adaptive, algo and what-if through the shared encoder (ibx#318) - #374
Closed
userFRM wants to merge 2 commits into
Closed
orders: route adaptive, algo and what-if through the shared encoder (ibx#318)#374userFRM wants to merge 2 commits into
userFRM wants to merge 2 commits into
Conversation
`build_order_request` returned early into a standalone `SubmitAdjustableStop` request, which bypassed the extended-attributes path every other order type goes through. The dedicated encoder arm emitted neither tag 6107 nor 583 and hard-coded 59=0, so an adjustable stop used as a bracket child shipped unlinked from its parent, outside its OCA group and DAY. The bypass dropped the rest of `OrderAttrs` with them: outside-RTH, hidden, display size, trigger method, conditions and GTD expiry were all silently discarded on this path. The adjustable stop is now an `OrderKind::AdjustableStop` carried by `SubmitEx`, so it encodes through `send_order_ex` like every other kind and picks up the shared attribute block. The wire layout is unchanged: 40=3 and 99 sit with the other order-type tags, and the 6257/6261/6258/6259 group plus the conditional 6262 and 6260/6269 are appended after 204 and the attribute block, which is where the encoder being replaced put them. Tag order should not carry meaning, but this path had a shipped layout and there was no reason to move it as a side effect. `Context::submit_adjustable_stop` takes `tif` and `attrs` to match the other extended submitters. `Connection::for_test()` is new test-only plumbing: it hands back the peer socket so a test can assert on the bytes an encoder actually writes. Two regression tests use it: one pins 6107, 583 and 59 for a bracket child, the other pins the conditional 6262/6260/6269, the absence of 6107/583 when no parent or OCA is set, and the relative order of the whole group. The enum-level tests passed unchanged for the whole time the child was shipping naked, which is why these assert on the wire instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`build_order_request` computes the extended-attribute block partway down, and three order types returned before reaching it. Their request variants carried no attribute block at all, so `Order::attrs()` was never consulted on those paths and their encoders emitted none of it. Setting `outsideRth` on an adaptive, algo or what-if order was accepted by the API and silently ignored — the order went out regular-hours-only with nothing reporting the difference. The same early return bypassed the parent link and the OCA group, so an adaptive or algo order used as a bracket child was submitted unlinked and outside its group, which is what the attribute block exists to prevent. The tif was hard-coded to DAY on all three. Patching the missing tags into each dedicated encoder would leave the rest of the block — hidden, display size, min quantity, good-after, GTD expiry, discretionary amount, sweep-to-fill, all-or-none, trigger method, cash quantity, conditions — still dropped on those paths, and would put the emission in four places. These route instead, the way the adjustable stop does: `Adaptive`, `Algo` and `WhatIf` are now `OrderKind` variants carried by `SubmitEx`, and the three standalone encoder arms are gone. Their own tags keep their values and their position — after tag 204 and the attribute block, where the encoders this replaces put them: 18=e and the adaptive priority parameter, the algo strategy with 849 and its parameter pairs, and the what-if flag. A what-if is still tracked under its marker so the response is recognised as a preview. `OrderKind` is no longer `Copy`, because the algo parameters it now carries own their strings. `Context::submit_adaptive`, `submit_algo` and `submit_what_if` take a tif and an attribute block, as `submit_adjustable_stop` does. Closes deepentropy#318.
7 tasks
Author
|
Closing this. It's in #409 along with the rest of the fork, which is easier to take in one piece than sixty separate branches. |
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.
Problem
build_order_requestcomputes the extended-attribute block partway down, and three order types returned before reaching it. Their request variants carried no attribute block at all, soOrder::attrs()was never consulted on those paths and their encoders emitted none of it.Setting
outsideRthon an adaptive, algo or what-if order was accepted by the API and silently ignored — the order went out regular-hours-only with nothing reporting the difference. The same early return bypassed the parent link and the OCA group, so an adaptive or algo order used as a bracket child was submitted unlinked and outside its group, which is what the attribute block exists to prevent. The time-in-force was hard-coded toDAYon all three.What this changes
Patching the missing tags into each dedicated encoder would leave the rest of the block — hidden, display size, minimum quantity, good-after, GTD expiry, discretionary amount, sweep-to-fill, all-or-none, trigger method, cash quantity, conditions — still dropped on those paths, and would put the emission in four places.
These route instead, the way the adjustable stop does in #246:
Adaptive,AlgoandWhatIfareOrderKindvariants carried bySubmitEx, and the three standalone encoder arms are gone.Their own tags keep their values and their position — after tag 204 and the attribute block, where the encoders this replaces put them:
18=eand the adaptive priority parameter, the algo strategy with 849 and its parameter pairs, and the what-if flag. A what-if is still tracked under its marker so the response is recognised as a preview.Breaking change to the low-level Rust API
Pre-1.0, and the same shape #246 already applied to the adjustable stop:
OrderRequest::SubmitAdaptive,SubmitAlgoandSubmitWhatIfare removed — useSubmitEx { kind: OrderKind::Adaptive | Algo | WhatIf, tif, attrs }.Context::submit_adaptive,submit_algoandsubmit_what_iftake a time-in-force and anOrderAttrs, assubmit_adjustable_stopdoes.OrderKindis no longerCopy, because the algo parameters it now carries own their strings.The
Client/EClientand Python surfaces are unchanged.Tests
the_algo_order_types_carry_the_attributes_the_caller_set— at the API boundary, where the drop happened: all three types, outside-RTH, parent link, OCA group and a non-DAY tif.adaptive_wire_carries_the_attributes_and_keeps_its_algo_tagsalgo_wire_carries_the_attributes_and_keeps_its_algo_tagswhat_if_wire_carries_the_attributes_and_keeps_its_preview_flagThe three wire tests assert on the bytes, because the enum-level tests passed throughout the period the children shipped naked. Each fails by name against a compiling reversion of the production change it covers.
Closes #318.
Test plan
the_algo_order_types_carry_the_attributes_the_caller_setby name.what_if_wire_carries_the_attributes_and_keeps_its_preview_flag.18=efailsadaptive_wire_carries_the_attributes_and_keeps_its_algo_tags.cargo check --offlineclean on--lib,--lib --features python,--bins,--examples, and each integration target individually.tests/ib_paper_compatcompared against a clean checkout of the base commit — identical sorted diagnostic sets.cargo test --offline --lib— only the two knownconfig::expiry_testsfailures, which fail on the base commit for missing legacy tzdata (fixed separately in config: resolve the legacy timezone names IB states its times in (ibx#335) #336).