ccp: stop reporting the OCA group as an order's parent (ibx#329) - #343
ccp: stop reporting the OCA group as an order's parent (ibx#329)#343userFRM wants to merge 3 commits into
Conversation
The engine writes the OCA group to tag 583 on the way out, in three places, and read the same tag back in as the parent order id, hashed into the `i64` the callback exposes. Both cannot be right. Tag 583 is the group. Two orders placed in one group, then one cancelled: the gateway cancelled the sibling, which is what an OCA group is for, and the inbound reports carry `583=PROBE-OCA-1` on both. The parent reported for those two orders was the same non-zero number — the hash of that string — for two orders that have no parent at all. Anything reconstructing order relationships from the callbacks reads that as a real link and builds the wrong tree. It never reads as missing data, which is what would have made it easy to notice; a non-OCA order has no 583, reports zero, and looks correct. Nothing on the execution report carries a parent order id, so the engine reports none. Tag 6107, which the bracket path sends a parent on, is a client id rather than an order id, and reading it back would be the same mistake in a new place. The client can answer where the engine cannot: it placed the order and was told the parent, so a locally placed child reports the parent it was given, on both clients. An order this client did not place keeps the engine's answer rather than borrowing someone else's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ba14668 to
9cd38f3
Compare
The parent was preferred only on the status branch, and a fill never reaches it — a fill emits its own `order_status` from a different branch, and the margin preview emits a third. Both read the engine's default of zero, so a bracket child still disowned its parent on the callback a caller is most likely to act on, on both clients. All three paths now prefer the parent this client recorded, and each is covered: reverting any one of them fails its own test by name. Also corrects a claim this change should not have made. The commit said nothing on the report can carry a parent and that tag 6107 is a client id rather than an order id. The encoder derives 6107 from the caller's `parent_id`, which contradicts the second half, and the first was asserted rather than shown. What is true is narrower and enough: the engine reads no parent from the report, so for an order this client placed its own record is the only source. The test that pins 6107 as unread now says what it is — a guard on a different invariant, which passes with or without this change — rather than implying it demonstrates this one. The status-loop test asserted a parent only for updates it happened to receive, so a case producing no update passed by never entering the loop. It now requires exactly one update per case first. `tracked_parent_id` was public on a publicly exported module and needed to be crate-internal, and its doc comment had displaced the one belonging to `is_order_tracked`.
|
Updated in A fill emits its own One claim corrected. The message said nothing on the report can carry a parent, and that tag 6107 is a client id rather than an order id. The encoder derives 6107 directly from the caller's The test pinning 6107 as unread stays, and now says what it is — a guard on a different invariant, which passes with or without this change — rather than being presented as evidence for it. Two smaller items: the status-loop test asserted a parent only for updates it happened to receive, so a case producing none passed by never entering the loop; it now requires exactly one update per case first. And 810 pass plus the two |
The change was applied to both clients and tested on one. Reverting either Python lookup — the fill or the margin preview — left every Rust test passing by name, so the claim that both clients were covered rested on nothing. Three tests against the Python client, using the existing `_test_*` harness. Reverting the fill lookup fails the first, the preview lookup the second, and the third is the positive control: an order this client never placed keeps the engine's answer, so the other two cannot pass against a client that simply reports the same parent for everything. `_test_track_order` gained an optional parent id, which is what made a tracked child expressible from Python at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Updated in The Python paths now have coverage. Reverting either Python lookup — the fill or the margin preview — left every Rust test passing, so "on both clients" rested on nothing. There are three Python tests now, using the existing On 810 pass plus the two |
Summary
The engine writes the OCA group to tag 583 on the way out, in three places, and read the same tag back in as the parent order id — hashed into the
i64the callback exposes. Both cannot be right.Closes #329.
Evidence
583 is the group. Two orders placed in group
PROBE-OCA-1, then one cancelled — the gateway cancelled the sibling, which is what an OCA group is for:The inbound reports carry the group on 583 for both, with distinct order ids:
and the parent reported for them was:
The same non-zero number for two orders that have no parent — the hash of the group name.
Impact
Anything reconstructing order relationships from the callbacks — a bracket tracker, a position attributor, a UI grouping children under parents — reads that as a real link and builds the wrong tree. It never reads as missing data, which is what would make it easy to notice: a non-OCA order has no 583, reports zero, and looks correct. The wrong value appears exactly when a group is in use.
Change
Nothing on the execution report carries a parent order id, so none is reported. An unknown parent is a worse answer than a correct one and a much better answer than a confident wrong one.
Tests
An order carrying an OCA group reports no parent, and its status is still delivered. Restoring the hash fails it.
Test plan
cargo test --offline --lib— 810 passed. The 2 failures areconfig::expiry_tests::{named_zone_converts_with_dst, instant_round_trips_to_wire}, which fail on the base commit too: the host has no legacy timezone files (fixed separately in config: resolve the legacy timezone names IB states its times in (ibx#335) #336).cargo check --offline --features python— clean.a_fill_reports_the_parent_the_child_was_given; reverting the preview path failsa_what_if_preview_reports_the_parent_the_child_was_given.connect.🤖 Generated with Claude Code